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/modules: Add security.pki.caBundle option and make all services use it for CA bundles #352244

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shelvacu
Copy link
Contributor

@shelvacu shelvacu commented Oct 30, 2024

Previously some modules used config.environment.etc."ssl/certs/ca-certificates.crt".source, some used "/etc/ssl/certs/ca-certificates.crt", and some used "${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt". These were all bad in one way or another:

  • config.environment.etc."ssl/certs/ca-certificates.crt".source relies on source being set; if text is set instead this breaks, introducing a weird undocumented requirement
  • "/etc/ssl/certs/ca-certificates.crt" is probably okay but very un-nix. It's a magic string, and the path doesn't change when the file changes (and so you can't trigger service reloads, for example, when the contents change in a new system activation)
  • "${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt" silently doesn't include the options from security.pki

Things done

  • Added a release notes entry
  • nixpkgs-review

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 30, 2024
@nix-owners nix-owners bot requested a review from peti October 30, 2024 01:25
@shelvacu shelvacu marked this pull request as draft October 30, 2024 01:33
… use it for CA bundles

Previously some modules used `config.environment.etc."ssl/certs/ca-certificates.crt".source`, some used `"/etc/ssl/certs/ca-certificates.crt"`, and some used `"${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"`. These were all bad in one way or another:

- `config.environment.etc."ssl/certs/ca-certificates.crt".source` relies on `source` being set; if `text` is set instead this breaks, introducing a weird undocumented requirement
- `"/etc/ssl/certs/ca-certificates.crt"` is probably okay but very un-nix. It's a magic string, and the path doesn't change when the file changes (and so you can't trigger service reloads, for example, when the contents change in a new system activation)
- `"${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"` silently doesn't include the options from `security.pki`
@shelvacu
Copy link
Contributor Author

@ofborg test gonic navidrome gocd-agent gocd-server postfix db-rest gitlab portunus radicle tandoor-recipes ocsinventory-agent parsedmarc uptime-kuma biboumi gateone privoxy stunnel unbound hound nix-daemon transmission cryptpad dex nextcloud peertube sogo

@shelvacu shelvacu marked this pull request as ready for review October 30, 2024 05:40
@shelvacu
Copy link
Contributor Author

shelvacu commented Nov 1, 2024

I don't know what command to run to reproduce the ofborg/CI fail.

HOME=/homeless-shelter NIX_PATH=nixpkgs=$(pwd) nix-instantiate --arg nixpkgs '{ outPath=./.; revCount=999999; shortRev="d37c608"; rev="d37c6088c292178843c2bb0dc1ccfdc92d5d8249"; }' ./pkgs/top-level/release.nix -A manual --option restrict-eval true --show-trace --argstr system x86_64-linux works fine for me.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4842

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 2, 2024
@shelvacu shelvacu force-pushed the unify-caBundle branch 2 times, most recently from 64b8011 to 484786a Compare December 3, 2024 01:48
@shelvacu shelvacu marked this pull request as draft December 3, 2024 01:49
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 3, 2024
@shelvacu shelvacu marked this pull request as ready for review December 3, 2024 20:56
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation This PR adds or changes documentation 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: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants