Using AF_UNIX sockets always for tests on Windows

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Using AF_UNIX sockets always for tests on Windows
Date: 2022-12-02 00:02:36
Message-ID: CA+hUKGK30uLx9dpgkYwomgH0WVLUHytkChDgf3iUM2zp0pf_nA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Commit f5580882 established that all supported computers have AF_UNIX.
One of the follow-up consequences that was left unfinished is that we
could simplify our test harness code to make it the same on all
platforms. Currently we have hundreds of lines of C and perl to use
secure TCP connections instead for the benefit of defunct Windows
versions. Here's a patch set for that. These patches and some
discussion of them were buried in the recent
clean-up-after-recently-dropped-obsolete-systems thread[1], and I
didn't want to lose track of them. I think they would need review and
testing from a Windows-based hacker to make progress. The patches
are:

1. Teach mkdtemp() to make a non-world-accessible directory. This is
required to be able to make a socket that other processes can't
connect to, to match the paranoia level used on Unix. This was
written just by reading documentation, because I am not a Windows
user, so I would be grateful for a second opinion and/or testing from
a Windows hacker, which would involve testing with two different
users. The idea is that Windows' mkdir() is completely ignoring the
permissions (we can see in the mingw headers that it literally throws
away the mode argument), so we shouldn't use that, but native
CreateDirectory() when given a pointer to a SECURITY_ATTRIBUTES with
lpSecurityDesciptor set to NULL should only allow the current user to
access the object (directory). Does this really work, and would it be
better to create some more explicit private-keep-out
SECURITY_ATTRIBUTE, and how would that look?

I'm fairly sure that filesystem permissions must be enough to stop
another OS user from connecting, because it's clear from documentation
that AF_UNIX works on Windows by opening the file and reading some
magic "reparse" data from inside it, so if you can't see into the
containing directory, you can't do it. This patch is the one the rest
are standing on, because the tests should match Unix in their level of
security.

2. Always use AF_UNIX for pg_regress. Remove a bunch of
no-longer-needed sspi auth stuff. Remove comments that worried about
signal handler safety (referring here to real Windows signals, not
fake PostgreSQL signals that are a backend-only concept). By my
reading of the Windows documentation and our code, there is no real
concern here, so the remove_temp() stuff should be fine, as I have
explained in a new comment. But I have not tested this signal safety
claim, not being a Windows user. I added an assertion that should
hold if I am right. If you run this on Windows and interrupt
pg_regress with ^C, does it hold?

3. Use AF_UNIX for TAP tests too.

4. In passing, remove our documentation's claim that Linux's
"abstract" AF_UNIX namespace is available on Windows. It does not
work at all, according to all reports (IMHO it seems like an
inherently insecure interface that other OSes would be unlikely to
adopt).

Note that this thread is not about libpq, which differs from Unix by
defaulting to host=localhost rather than AF_UNIX IIRC. That's a
user-facing policy decision I'm not touching; this thread is just
about cleaning up old test infrastructure of interest to hackers.

[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJ3LHeP9w5Fgzdr4G8AnEtJ%3Dz%3Dp6hGDEm4qYGEUX5B6fQ%40mail.gmail.com

Attachment Content-Type Size
0001-WIP-Make-mkdtemp-more-secure-on-Windows.patch text/x-patch 1.0 KB
0002-WIP-Always-use-Unix-domain-sockets-in-pg_regress-on-.patch text/x-patch 10.7 KB
0003-WIP-Stop-using-TCP-in-TAP-tests-on-Windows.patch text/x-patch 10.7 KB
0004-Doc-Abstract-AF_UNIX-sockets-don-t-work-on-Windows-a.patch text/x-patch 1.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-12-02 00:13:21 Re: HOT chain validation in verify_heapam()
Previous Message Kohei KaiGai 2022-12-01 23:35:13 Re: Asynchronous execution support for Custom Scan