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

nixos/chrony: systemd service hardening #104944

Closed

Conversation

snicket2100
Copy link
Contributor

The service can successfully work with more systemd sandboxing
in place. Changing ProtectSystem from full to strict makes
the entire filesystem read-only, with the exception of two
directories listed under ReadWritePaths. Many other sandbox
options added as well, unfortunately I couldn't source them
from the project itself as they only provide a very basic
systemd service template.

The /var/run/chrony path is compiled in the chronyd binary,
as well as in the chronyc client. This is why I am not referencing it for anything else other than ReadWritePaths.

I've been running this config for a while and it seems stable.

Motivation for this change
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.

The service can successfully work with more systemd sandboxing
in place. Changing `ProtectSystem` from `full` to `strict` makes
the entire filesystem read-only, with the exception of two
directories listed under `ReadWritePaths`. Many other sandbox
options added as well, unfortunately I couldn't source them
from the project itself as they only provide a very basic
systemd service template.

The `/var/run/chrony` path is compiled in the `chronyd` binary,
as well as in the `chronyc` client.
@aanderse
Copy link
Member

Are you able to provide a patch to upstream with these changes? It would be nice to have upstream sign off on this unit. You get bonus points if upstream adopts your changes and we can stop providing our own systemd unit, instead including the upstream unit via systemd.packages.

@snicket2100
Copy link
Contributor Author

Are you able to provide a patch to upstream with these changes? It would be nice to have upstream sign off on this unit. You get bonus points if upstream adopts your changes and we can stop providing our own systemd unit, instead including the upstream unit via systemd.packages.

Yeah that would be lovely indeed, I will try doing it in a spare moment. Let's park this pr for now then.

@aanderse aanderse added the 2.status: wait-for-upstream Waiting for upstream fix (or their other action). label Nov 26, 2020
@bbigras
Copy link
Contributor

bbigras commented Jan 22, 2021

Any progress on this?

@stale
Copy link

stale bot commented Jul 21, 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 Jul 21, 2021
@mweinelt
Copy link
Member

Can we please unpark and rebase this? I'd be happy to review and test.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021
@stale
Copy link

stale bot commented Apr 19, 2022

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 Apr 19, 2022
@birlorg
Copy link
Contributor

birlorg commented Jul 4, 2022

Upstream has a systemd service file in git now: https://git.tuxfamily.org/chrony/chrony.git/tree/examples/chronyd.service

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 7, 2023
@Izorkin
Copy link
Contributor

Izorkin commented Jan 11, 2023

@snicket2100 implemented in this PR - #208751

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 11, 2023
@gkleen
Copy link
Contributor

gkleen commented Jan 14, 2023

This PR does not introduce the issues I described here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants