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.

Sign up for GitHub

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

Jump to bottom

nixos/postgresql: enable sandbox mode #113100

Closed
wants to merge 1 commit into from
Closed

nixos/postgresql: enable sandbox mode #113100

wants to merge 1 commit into from

Conversation

Izorkin
Copy link
Contributor

@Izorkin Izorkin commented Feb 14, 2021

Motivation for this change

Running postgresql service in sandbox mode.
Minimal testing.

cc @aanderse @flokli

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 14, 2021
@aanderse aanderse requested review from srhb, danbst and aszlig February 14, 2021 15:28
aszlig
aszlig reviewed Feb 15, 2021
View reviewed changes
Copy link
Member

@aszlig aszlig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimal testing.

I think we should especially test this with a few extensions (eg. postgis) and involve a few more folks using PostgreSQL before merging this, because reverting this on a per-config basis will be a bit more involved with all the options set.

nixos/modules/services/databases/postgresql.nix
# Security
NoNewPrivileges = true;
# Sandboxing
ProtectSystem = "strict";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try the systemd.services.*.confinement options instead? I'm asking because one of the main targets of that module was PostgreSQL and it's also more strict that ProtectSystem because it only contains runtime closure necessary for running PostgreSQL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have not tried. Don't know how to use systemd.services.*.confinement.

nixos/modules/services/databases/postgresql.nix
# Sandboxing
ProtectSystem = "strict";
ProtectHome = true;
PrivateTmp = false; # Fixme, nixos/tests/postgresql-wal-receiver uses the /tmp directory for test.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, we should fix the test and not the other way around. One of the reasons why we've switched to /run/postgresql is because it makes sandboxing more difficult as outlined in #57677.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now there is no idea how to fix the test.

nixos/modules/services/databases/postgresql.nix Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Aug 15, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 15, 2021
@wolfgangwalther wolfgangwalther mentioned this pull request Sep 27, 2024
Merged
13 tasks
@wolfgangwalther
Copy link
Contributor

Closing, because #344925 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aszlig aszlig aszlig left review comments

@aanderse aanderse aanderse requested changes

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@srhb srhb Awaiting requested review from srhb

@danbst danbst Awaiting requested review from danbst

Assignees
No one assigned
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Izorkin @wolfgangwalther @aszlig @aanderse