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

aszlig · 2019-03-15T04:06:02Z

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.

@ocharles

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 NixOS#24317) and with the new systemd-confinement mode upcoming in NixOS#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 · 2019-03-15T05:45:23Z

👍 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 · 2019-03-15T09:06:39Z

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

aszlig · 2019-03-15T11:10:24Z

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

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

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 · 2019-03-16T02:07:34Z

@thoughtpolice, @danbst: Done.

danbst · 2019-03-16T11:04:46Z

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

aszlig · 2019-03-16T11:22:17Z

@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.

@danbst

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 · 2019-03-16T16:50:02Z

@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?

puckipedia · 2019-03-29T15:21:22Z

This PR seems to have broken the build of hydra, since it doesn't use the systemd service to launch postgresql and as such, it doesn't have a /run/postgresql to put the socket into...

flokli · 2019-03-29T15:50:47Z

can we tweak the hydra module to simply enable the postgresql module through services.postgresql in the localDB case?

flokli · 2019-03-29T15:58:15Z

wait, I don't get it - the hydra module already uses the postgresql module, and this one should set up /run/postgresql through RuntimeDirectory = "postgresql"…

puckipedia · 2019-03-29T15:58:39Z

no, that part's fine. It's during doCheck that it fails: https://hydra.nixos.org/build/91221682

flokli · 2019-03-29T16:17:10Z

@puckipedia we also can't just mkdir -p /run/postgresql in hydra's preCheck, as the sandbox doesn't allow to write to /run.

We however might be able to tweak hydra's tests/ to pass -k /tmp when instructing postgresql commands, didn't check.

danbst · 2019-03-29T18:31:23Z

Very unfortunate situation. I tend to revert this, and reapply when Nix does allow creating /run/postgresql. It can even be a build setup hook for postgresql, but Nix doesn't allow /run directory....

@aszlig what do you think?

aszlig · 2019-03-29T18:44:44Z

@danbst: Nix doesn't need to allow this, because you can simply set PGHOST during preCheck to use a different socket path.

aszlig · 2019-03-29T18:46:45Z

eg, similar to: https://github.com/nixcloud/nixcloud-webservices/blob/d8b966f8d25c08e9445fa9a28ba850afc2ffcc2d/modules/web/database/postgresql-hook.sh#L1

danbst · 2019-03-29T18:59:59Z

I just see it as regression - previously it was possible to do pg_ctl -D ... start and psql, but now you have specify socket directory both for server and client. It was useful...

Probably the best approach would be to patch postgres server to create socket in unix_socket_dir, and if it is not set, then create in /run/user/$(id -u), and if it doesn't exist, then use /run/postgresql. And implement same search logic for psql - first search in host argument/PGHOST, and if not specified, then in /run/user/$(id -u), if not found then search in /run/postgresql. This would still require Nix to allow /run directory and require setup hook for postgresql, to make things flawless.

danbst · 2019-03-29T21:39:20Z

Added a discussion topic here https://www.postgresql.org/message-id/CANZg%2Byd2OdqxM%3DHwnrkT-QecNMqLKNp%2BbGO_2SirJGznGK6h_w%40mail.gmail.com

Meanwhile, we just have to mention in release notes that pg_ctl should use custom socket dir (XDG_RUNTIME_DIR for users, /tmp for nix build environment) both for client and server.

@aszlig

In NixOS/nixpkgs#57677 default postgresql socket directory was changed to `/run/postgresql`, which doesn't exist (and can't be created) in Nix build environment. We'll use /tmp as socket dir explicitly then. Fixes build failure https://hydra.nixos.org/build/91221682 Cc @aszlig @edolstra

flokli · 2019-03-30T19:48:17Z

@danbst can we either revert this PR, or add a fetchpatch https://github.com/NixOS/hydra/pull/648/commits/4171ab4c4fd576c516dc03ba64d1c7945f769af0.patch to get Hydra to build again?

tilpner · 2019-05-12T11:15:11Z

I believe this might also fix a race condition between systemd-tmpfiles (caused by boot.cleanTmpDir) and postgres, that causes the socket to be deleted on boot.

flokli · 2019-05-12T12:35:48Z

@tilpner another reason to run systemd-tmpfiles as part of sysinit.target, before every other "regular" service ;-)

see #56265 (comment) and following.

cc @aanderse @erikarvstedt

LnL7 · 2019-07-02T22:43:03Z

This breaks postgres outside of NixOS, this shouldn't be patched but handled in the nixos modules instead IMHO.

danbst · 2019-07-03T07:07:46Z

@LnL7 true, but outside NixOS it is easy fixable with -h /tmp in client CLI arg, and -k /tmp server CLI arg. I agree this is less convenient, but sharing sockets through /tmp isn't generally safe in multiuser setup.

LnL7 · 2019-07-03T07:36:17Z

That doesn't matter, the default usage is broken because of non-standard behaviour. If this was a change in postgresql it would be another story.

edolstra · 2019-07-03T08:42:53Z

While I think getting rid of sockets in /tmp is great (including /tmp/.X11-unix and /tmp/.ICE-unix while we're at it), it would be better if this were done upstream rather than in NixOS-specific patches.

tilpner · 2019-07-03T09:15:03Z

So revert this PR and pass -k /run/... from the module?

aanderse · 2019-07-03T09:45:02Z

Is anyone in favor of a socket option (potentially readOnly) so we can stop using so many magic strings all over NixOS modules every time we want to refer to a socket? Other modules like mysql would benefit from this as well. If so, let me know and I can whip something up.

danbst · 2019-07-03T11:55:13Z

@LnL7 unfortunately, it was already "non-standard", depending on what is "standard". Ubuntu (and Debian afaik) uses /var/run/postgresql as a de-factor standard socket path (same as we do now).

@edolstra Upstream doesn't work to improve this. https://www.postgresql.org/message-id/25758.1553906401%40sss.pgh.pa.us

@tilpner no, because PG consists of "server" and "client". And PG wants both to have same config parameter. So either both use same location, or they don't communicate each other.

edolstra · 2019-07-03T12:13:11Z

@danbst Well, Tom Lane isn't wrong about "a Babel of servers and clients that can't talk to each other", as this issue shows...

Having said that, the fact that Ubuntu/Debian already use /run/postgresql is probably more relevant to us than what upstream does.

LnL7 · 2019-07-03T18:09:51Z

Unlike the apt packages, nix won't ensure these defaults are functional however.

This seems to be mostly a pre - NixOS#57677 relict. As postgresql sockets now are not in /tmp anymore, isolate /tmp.

aszlig requested review from thoughtpolice, danbst and edolstra March 15, 2019 04:06

aszlig requested a review from infinisil as a code owner March 15, 2019 04:06

aszlig added the 9.needs: changelog label Mar 15, 2019

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) labels Mar 15, 2019

GrahamcOfBorg requested a review from ocharles March 15, 2019 04:19

GrahamcOfBorg added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 501-1000 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500 labels Mar 15, 2019

thoughtpolice approved these changes Mar 15, 2019

View reviewed changes

nixos/manual: Document PostgreSQL socket change

116bdc9

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 removed the 9.needs: changelog label Mar 16, 2019

GrahamcOfBorg added 8.has: changelog 8.has: documentation labels Mar 16, 2019

GrahamcOfBorg requested a review from thoughtpolice March 16, 2019 02:19

postgresql: Only use /run/postgresql on Linux

f510601

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>

GrahamcOfBorg added 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 501+ 10.rebuild-darwin: 501-1000 labels Mar 16, 2019

danbst mentioned this pull request Mar 29, 2019

tests: change postgresql socket dir to /tmp NixOS/hydra#648

Merged

flokli mentioned this pull request Mar 31, 2019

miniflux: add service #56719

Merged

10 tasks

flokli mentioned this pull request May 12, 2019

replace deprecated usage of PermissionsStartOnly (part 2) #56265

Merged

10 tasks

flokli added a commit to flokli/nixpkgs that referenced this pull request Nov 21, 2019

nixos/phpfpm: enable PrivateTmp=true

4321a88

This seems to be mostly a pre - NixOS#57677 relict. As postgresql sockets now are not in /tmp anymore, isolate /tmp.

flokli mentioned this pull request Nov 21, 2019

nixos/phpfpm: enable PrivateTmp=true #73886

Merged

10 tasks

danbst mentioned this pull request Apr 2, 2020

Starting postgresql demands that /run/postgresql exists #83770

Open

aszlig mentioned this pull request Feb 15, 2021

nixos/postgresql: enable sandbox mode #113100

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

aszlig commented Mar 15, 2019

danbst commented Mar 15, 2019

edolstra commented Mar 15, 2019

aszlig commented Mar 15, 2019

thoughtpolice left a comment

aszlig commented Mar 16, 2019

danbst commented Mar 16, 2019

aszlig commented Mar 16, 2019

flokli commented Mar 16, 2019

puckipedia commented Mar 29, 2019

flokli commented Mar 29, 2019 •

edited

Loading

flokli commented Mar 29, 2019

puckipedia commented Mar 29, 2019

flokli commented Mar 29, 2019

danbst commented Mar 29, 2019

aszlig commented Mar 29, 2019 •

edited

Loading

aszlig commented Mar 29, 2019

danbst commented Mar 29, 2019

danbst commented Mar 29, 2019

flokli commented Mar 30, 2019

tilpner commented May 12, 2019

flokli commented May 12, 2019

LnL7 commented Jul 2, 2019

danbst commented Jul 3, 2019

LnL7 commented Jul 3, 2019

edolstra commented Jul 3, 2019

tilpner commented Jul 3, 2019

aanderse commented Jul 3, 2019

danbst commented Jul 3, 2019

edolstra commented Jul 3, 2019

LnL7 commented Jul 3, 2019

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

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

Conversation

aszlig commented Mar 15, 2019

danbst commented Mar 15, 2019

edolstra commented Mar 15, 2019

aszlig commented Mar 15, 2019

thoughtpolice left a comment

Choose a reason for hiding this comment

aszlig commented Mar 16, 2019

danbst commented Mar 16, 2019

aszlig commented Mar 16, 2019

flokli commented Mar 16, 2019

puckipedia commented Mar 29, 2019

flokli commented Mar 29, 2019 • edited Loading

flokli commented Mar 29, 2019

puckipedia commented Mar 29, 2019

flokli commented Mar 29, 2019

danbst commented Mar 29, 2019

aszlig commented Mar 29, 2019 • edited Loading

aszlig commented Mar 29, 2019

danbst commented Mar 29, 2019

danbst commented Mar 29, 2019

flokli commented Mar 30, 2019

tilpner commented May 12, 2019

flokli commented May 12, 2019

LnL7 commented Jul 2, 2019

danbst commented Jul 3, 2019

LnL7 commented Jul 3, 2019

edolstra commented Jul 3, 2019

tilpner commented Jul 3, 2019

aanderse commented Jul 3, 2019

danbst commented Jul 3, 2019

edolstra commented Jul 3, 2019

LnL7 commented Jul 3, 2019

flokli commented Mar 29, 2019 •

edited

Loading

aszlig commented Mar 29, 2019 •

edited

Loading