Unix-domain socket support on Windows

On 07/08/2019 16:56, Peter Eisentraut wrote:

It works!

Cool!

Am I reading the patches correctly, that getpeereid() still doesn't work
on Windows? That means that peer authentication doesn't work, right?
That's a bit sad. One of the big advantages of unix domain sockets over
TCP is peer authentication.

- Heikki

On 2019-08-07 16:06, Heikki Linnakangas wrote:

Am I reading the patches correctly, that getpeereid() still doesn't work
on Windows? That means that peer authentication doesn't work, right?
That's a bit sad. One of the big advantages of unix domain sockets over
TCP is peer authentication.

Correct, it's not supported. I think it's plausible that they will add
this in the future.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Aug 7, 2019 at 4:59 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-08-07 16:06, Heikki Linnakangas wrote:

Am I reading the patches correctly, that getpeereid() still doesn't work
on Windows? That means that peer authentication doesn't work, right?
That's a bit sad. One of the big advantages of unix domain sockets over
TCP is peer authentication.

Correct, it's not supported. I think it's plausible that they will add
this in the future.

Does it work well enough that SSPI auth can run over it? SSPI auth with the
local provider gives you more or less the same results as peer, doesn't it?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On 2019-08-07 15:56, Peter Eisentraut wrote:

Depending on your Windows environment, there might not be a suitable
/tmp directory, so you'll need to specify a directory explicitly using
postgres -k or similar. This leads to the question what the default for
DEFAULT_PGSOCKET_DIR should be on Windows. I think it's probably best,
at least for now, to set it so that by default, neither server nor libpq
use Unix sockets unless explicitly selected. This can be done easily on
the server side by defining DEFAULT_PGSOCKET_DIR as "". But in libpq, I
don't think the code would handle that correctly everywhere, so it would
need some more analysis and restructuring possibly.

Updated patches, which now also address that issue: There is no default
socket dir on Windows and it's disabled by default on both client and
server.

Some comments on the patches:

v2-0001-Enable-Unix-domain-sockets-support-on-Windows.patch

This is pretty straightforward, apart from maybe some comments, but it
would need to be committed last, because it would enable all the Unix
socket related code on Windows, which needs to be fixed up by the
subsequent patches first.

v2-0002-Sort-out-getpeereid-and-struct-passwd-handling-on.patch

Maybe a more elegant way with fewer #ifdef WIN32 can be found?

v2-0003-psql-Remove-one-use-of-HAVE_UNIX_SOCKETS.patch

This could be committed independently.

v2-0004-libpq-Remove-unnecessary-uses-of-HAVE_UNIX_SOCKET.patch

This one as well.

v2-0005-initdb-Detect-Unix-domain-socket-support-dynamica.patch

I think this patch contains some nice improvements in general. How much
of that ends up being useful depends on how the subsequent patches (esp.
0007) end up, since with Unix-domain sockets disabled by default on
Windows, we won't need initdb doing any detection.

v2-0006-Fix-handling-of-Unix-domain-sockets-on-Windows-in.patch

This is a fairly independent and isolated change.

v2-0007-Disable-Unix-sockets-by-default-on-Windows.patch

This one is a bit complicated. Since there is no good default location
for Unix sockets on Windows, and many systems won't support them for
some time, the default implemented here is to not use them by default on
the server or client. This needs a fair amount of restructuring in the
to support the case of "supports Unix sockets but don't use them by
default", while maintaining the existing cases of "doesn't support Unix
sockets" and "use Unix sockets by default". There is some room for
discussion here.

This patch set needs testers with various Windows versions to test
different configurations, combinations, and versions.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Aug 14, 2019 at 6:27 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

This patch set needs testers with various Windows versions to test
different configurations, combinations, and versions.

It's failing to build on cfbot's AppVeyor setup[1]https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.55034?fullLog=true. That's currently
using Windows SDK 7.1, so too old for the new AF_UNIX sockets, but
presumably something is wrong because it shouldn't fail to compile and
link.

src/interfaces/libpq/fe-connect.c(2682): warning C4101: 'pwdbuf' :
unreferenced local variable [C:\projects\postgresql\libpq.vcxproj]
src/interfaces/libpq/fe-connect.c(2687): warning C4101: 'passerr' :
unreferenced local variable [C:\projects\postgresql\libpq.vcxproj]

fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
referenced in function PQconnectPoll
[C:\projects\postgresql\libpq.vcxproj]

[1]: https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.55034?fullLog=true

--
Thomas Munro
https://enterprisedb.com

On 2019-09-03 01:45, Thomas Munro wrote:

fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
referenced in function PQconnectPoll
[C:\projects\postgresql\libpq.vcxproj]

This should be fixed in the attached patches.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-Sep-03, Peter Eisentraut wrote:

On 2019-09-03 01:45, Thomas Munro wrote:

fe-connect.obj : error LNK2019: unresolved external symbol getpeereid
referenced in function PQconnectPoll
[C:\projects\postgresql\libpq.vcxproj]

This should be fixed in the attached patches.

Minor bitrot in MSVC script; probably trivial to resolve.

I think you should get 0001 (+0002?) pushed and see what the buildfarm
thinks; move forward from there. 0003+0004 sound like they should just
be pushed shortly afterwards, while the three remaining ones might need
some more careful review.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-Sep-06, Alvaro Herrera from 2ndQuadrant wrote:

I think you should get 0001 (+0002?) pushed and see what the buildfarm
thinks; move forward from there.

... but of course this goes counter to what you said earlier about 0001
needing to be pushed last.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

To move this topic a long, I'll submit some preparatory patches in a
committable order.

First is the patch to deal with getpeereid() that was already included
in the previous patch series. This is just some refactoring that
reduces the difference between Windows and other platforms and prepares
the Unix-domain socket specific code to compile cleanly on Windows.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Oct 30, 2019 at 10:32 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

To move this topic a long, I'll submit some preparatory patches in a
committable order.

First is the patch to deal with getpeereid() that was already included
in the previous patch series. This is just some refactoring that
reduces the difference between Windows and other platforms and prepares
the Unix-domain socket specific code to compile cleanly on Windows.

This looks fairly sane and straightforward. Let's give it an outing on
the buildfarm ASAP so we can keep moving forward on this.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-12-16 05:39, Andrew Dunstan wrote:

On Wed, Oct 30, 2019 at 10:32 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

To move this topic a long, I'll submit some preparatory patches in a
committable order.

First is the patch to deal with getpeereid() that was already included
in the previous patch series. This is just some refactoring that
reduces the difference between Windows and other platforms and prepares
the Unix-domain socket specific code to compile cleanly on Windows.

This looks fairly sane and straightforward. Let's give it an outing on
the buildfarm ASAP so we can keep moving forward on this.

pushed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Next patch: This allows building *with* Unix-domain socket support but
*without* a default Unix socket path. This is needed because on Windows
we don't have a good default location like "/tmp" and we probably don't
want Unix sockets by default at run time so that older Windows versions
continue to work out of the box with the same binaries.

We have code paths for Unix socket support and no Unix socket support.
Now add a third variant: Unix socket support but do not use a Unix
socket by default in the client or the server, only if you explicitly
specify one.

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Dec 18, 2019 at 02:52:15PM +0100, Peter Eisentraut wrote:

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

Would it make sense to support abstract sockets in PostgreSQL?

I know it's bit unrelated. I haven't read all the code here I just was
thinking about it because of the code checking the leading \0 byte of the dir.

Garick

On 2019-12-18 15:24, Hamlin, Garick L wrote:

On Wed, Dec 18, 2019 at 02:52:15PM +0100, Peter Eisentraut wrote:

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

Would it make sense to support abstract sockets in PostgreSQL?

Maybe, I'm not sure.

I know it's bit unrelated. I haven't read all the code here I just was
thinking about it because of the code checking the leading \0 byte of the dir.

We would probably represent abstract sockets with a leading '@' in the
user-facing components and only translate it to the internal format at
the last moment, probably in that very same UNIXSOCK_PATH() function.
So I think that wouldn't be a problem.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-12-18 14:52, Peter Eisentraut wrote:

Next patch: This allows building *with* Unix-domain socket support but
*without* a default Unix socket path. This is needed because on Windows
we don't have a good default location like "/tmp" and we probably don't
want Unix sockets by default at run time so that older Windows versions
continue to work out of the box with the same binaries.

We have code paths for Unix socket support and no Unix socket support.
Now add a third variant: Unix socket support but do not use a Unix
socket by default in the client or the server, only if you explicitly
specify one.

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

Perhaps this patch is too boring to be reviewed. If there are no
objections, I'll commit it soon and then submit the final patches with
the real functionality for the next commit fest.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-12-18 14:52, Peter Eisentraut wrote:

We have code paths for Unix socket support and no Unix socket support.
Now add a third variant: Unix socket support but do not use a Unix
socket by default in the client or the server, only if you explicitly
specify one.

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

Perhaps this patch is too boring to be reviewed. If there are no
objections, I'll commit it soon and then submit the final patches with
the real functionality for the next commit fest.

Sorry, I'd paid no particular attention to this thread because
I figured it'd take a Windows-competent person to review. But
the patch as it stands isn't that.

The code looks fine (and a big +1 for not having knowledge of
DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH). I wonder though
whether any user-facing documentation needs to be adjusted.

regards, tom lane

On Fri, 31 Jan 2020 at 02:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-12-18 14:52, Peter Eisentraut wrote:

We have code paths for Unix socket support and no Unix socket support.
Now add a third variant: Unix socket support but do not use a Unix
socket by default in the client or the server, only if you explicitly
specify one.

To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
to "" has the desired effect. This mostly already worked like that;
only a few places needed to be adjusted. Notably, the reference to
DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
callers already resolve an empty socket directory setting with a
default if appropriate.

Perhaps this patch is too boring to be reviewed. If there are no
objections, I'll commit it soon and then submit the final patches with
the real functionality for the next commit fest.

Sorry, I'd paid no particular attention to this thread because
I figured it'd take a Windows-competent person to review. But
the patch as it stands isn't that.

The code looks fine (and a big +1 for not having knowledge of
DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH). I wonder though
whether any user-facing documentation needs to be adjusted.

Probably, since it won't work with 'peer' auth from what was said upthread.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

On 2020-01-30 19:41, Tom Lane wrote:

The code looks fine (and a big +1 for not having knowledge of
DEFAULT_PGSOCKET_DIR wired into UNIXSOCK_PATH). I wonder though
whether any user-facing documentation needs to be adjusted.

There are no user-facing changes in this patch yet. That will come with
subsequent patches.

This patch has now been committed.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Here is another patch set to enable this functionality.

0001 enables Unix-domain sockets on Windows, but leaves them turned off
by default at run time, using the mechanism introduced by a9cff89f7e.
This is relatively straightforward, except perhaps some aesthetic
questions about how these different configuration bits are distributed
around the various files.

0002 deals with pg_upgrade. It preserves the existing behavior of not
using Unix-domain sockets on Windows. This could perhaps be enhanced
later by either adding a command-line option or a run-time test. It's
too complicated right now.

0003 deals with how initdb should initialize postgresql.conf and
pg_hba.conf. It introduces a run-time test similar to how we detect
presence of IPv6. After I wrote this patch, I have come to think that
this is overkill and we should just always leave the "local" line in
pg_hba.conf even if there is no run-time support in the OS. (I think
the reason we do the run-time test for IPv6 is that we need it to parse
the IPv6 addresses in pg_hba.conf, but there is no analogous requirement
for Unix-domain sockets.) This patch is optional in any case.

0004 fixes a bug in the pg_upgrade test.sh script that was exposed by
these changes.

0005 fixes up some issues in the test suites. Right now, the TAP tests
are hardcoded to not use Unix-domain sockets on Windows, where as
pg_regress keys off HAVE_UNIX_SOCKETS, which is no longer a useful
distinguisher. The change is to not use Unix-domain sockets for all the
tests by default on Windows (the previous behavior) but give an option
to use them. At the moment, I would consider running the test suites
with Unix-domain sockets enabled as experimental, but that's only
because of various issues in the test setups. For instance, there is an
issue in the comment of pg_regress.c remove_temp() that I'm not sure how
to address. Also, the TAP tests don't seem to work because of some path
issues. I figured I'd call time on fiddling with this for now and ship
the patches.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services