Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

postgresql: Move socket dir to /run/postgresql #57677

Open
wants to merge 3 commits into
base: master
from

Conversation

@aszlig
Copy link
Member

aszlig commented Mar 15, 2019

The default, which is /tmp, has a few issues associated with it:

One being that it makes it easy for users on the system to spoof a PostgreSQL server if it's not running, causing applications to connect to their provided sockets instead of just failing to connect.

Another one is that it makes sandboxing of PostgreSQL and other services unnecessarily difficult. This is already the case if only PrivateTmp is used in a systemd service, so in order for such a service to be able to connect to PostgreSQL, a bind mount needs to be done from /tmp to some other path, so the service can access it. This pretty much defeats the whole purpose of PrivateTmp.

We regularly run into issues with this in the past already (one example would be #24317) and with the new systemd-confinement mode upcoming in #57519, it makes it even more tedious to sandbox services.

I've tested this change against all the postgresql NixOS VM tests and they still succeed and I also grepped through the source tree to replace other occasions where we might have /tmp hardcoded. Luckily there were very few occasions.

postgresql: Move socket dir to /run/postgresql
The default, which is /tmp, has a few issues associated with it:

One being that it makes it easy for users on the system to spoof a
PostgreSQL server if it's not running, causing applications to connect
to their provided sockets instead of just failing to connect.

Another one is that it makes sandboxing of PostgreSQL and other services
unnecessarily difficult. This is already the case if only PrivateTmp is
used in a systemd service, so in order for such a service to be able to
connect to PostgreSQL, a bind mount needs to be done from /tmp to some
other path, so the service can access it. This pretty much defeats the
whole purpose of PrivateTmp.

We regularily run into issues with this in the past already (one example
would be #24317) and with the new
systemd-confinement mode upcoming in
#57519, it makes it even more
tedious to sandbox services.

I've tested this change against all the postgresql NixOS VM tests and
they still succeed and I also grepped through the source tree to replace
other occasions where we might have /tmp hardcoded. Luckily there were
very few occasions.

Signed-off-by: aszlig <aszlig@nix.build>
Cc: @ocharles, @thoughtpolice, @danbst
@danbst

This comment has been minimized.

Copy link
Contributor

danbst commented Mar 15, 2019

👍 would be awesome if it was changed to /var/run/postgresql, as Debian/Ubuntu use that path by default. See https://sources.debian.org/src/postgresql-11/11.2-2/debian/patches/51-default-sockets-in-var.patch/ , but I guess /run and /var/run are mounted to each other in every distribution, so not a big deal.

Also, add release notes, because it can make old psql not to connect to new postgresql via UNIX socket.

@edolstra

This comment has been minimized.

Copy link
Member

edolstra commented Mar 15, 2019

I would stick to /run instead of /var/run, since the latter is obsolete.

@aszlig

This comment has been minimized.

Copy link
Member Author

aszlig commented Mar 15, 2019

Also, add release notes, because it can make old psql not to connect to new postgresql via UNIX socket.

Yep, that's why I added that label, just wanted to see first whether there is any opposition.

@thoughtpolice
Copy link
Member

thoughtpolice left a comment

LGTM, pending a release note addition as @danbst asked.

nixos/manual: Document PostgreSQL socket change
This is a backwards-incompatible change and while it won't probably
affect a whole lot of users, it makes sense to give them a heads-up
anyway.

Signed-off-by: aszlig <aszlig@nix.build>
@aszlig

This comment has been minimized.

Copy link
Member Author

aszlig commented Mar 16, 2019

@danbst

This comment has been minimized.

Copy link
Contributor

danbst commented Mar 16, 2019

BTW, this should be only for Linux system, right? Does darwin platform have /run? cc @basvandijk

@aszlig

This comment has been minimized.

Copy link
Member Author

aszlig commented Mar 16, 2019

@danbst: Hm, good catch, but even if OSX would have /run, it probably won't have /run/postgresql. So I'm going to make this Linux-only.

postgresql: Only use /run/postgresql on Linux
We only have /run on modern GNU/Linux systems and it's not necessarily
the case for Mac OS X or *BSD, so let's add the patch only if
stdenv.isLinux.

Thanks to @danbst for catching this.

Signed-off-by: aszlig <aszlig@nix.build>
@flokli

This comment has been minimized.

Copy link
Contributor

flokli commented Mar 16, 2019

@aszlig what's the socket path that PHP defaults to when connecting to a local postgresql?

Is there a way to tweak this one, similar to how I did for mysql in 5bb85cd?

@aszlig

This comment has been minimized.

Copy link
Member Author

aszlig commented Mar 16, 2019

@flokli: I haven't checked PHP in particular but I'd assume that it's using the default one in libpq like most other software.

@flokli

This comment has been minimized.

Copy link
Contributor

flokli commented Mar 16, 2019

@aszlig if that's the case, shouldn't overriding DEFAULT_PGSOCKET_DIR in this PR also make PHP find the proper socket path automatically?

@aszlig

This comment has been minimized.

Copy link
Member Author

aszlig commented Mar 17, 2019

@flokli: Exactly that is the idea. Just checked the PHP source and they rely fully on libpq and there is no NIH implementation. So this also covers PHP unless of course someone uses /tmp as the hostname in PDO/pg_connect, but see the changelog entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.