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/stage1: copy initrd secrets into place after special mounts #129449

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

ddz
Copy link
Contributor

@ddz ddz commented Jul 6, 2021

Motivation for this change

This modifies initialRamdiskSecretAppender to stage secrets in /initrd-secrets/ and stage-1-init to copy them into place after mounting special file systems. This allows secrets to be copied into ramfs mounts like /run/keys for use after stage-1 finishes without copying them to disk (which would not be very secure). This is useful for ZFS filesystem key files, which are mounted later, as well as when rebuilding system configurations to be able to pull secret from /run/keys to rebuild the initrd.

Things done

Tested on release-21.05 branch and master to preserve existing functionality as well as enable a destinations for secrets in /run/keys which were present after system had booted.

Updated test in tests/initrd-secrets.nix to verify new behavior.

  • 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/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ddz ddz force-pushed the copy-initrd-secrets-after-early-mount-script branch from 4ffbf45 to 5e9de99 Compare July 7, 2021 04:46
@ddz
Copy link
Contributor Author

ddz commented Jul 7, 2021

Thanks for the review! I responded to your suggestion re: find, fixed tabs, and squashed back into this PR.

@ddz ddz force-pushed the copy-initrd-secrets-after-early-mount-script branch from 5e9de99 to 142c6a4 Compare July 7, 2021 04:59
@ddz ddz requested a review from SuperSandro2000 July 9, 2021 14:13
@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/559

@ddz ddz force-pushed the copy-initrd-secrets-after-early-mount-script branch from 142c6a4 to b1bc6cc Compare July 11, 2021 20:21
Since /run/keys is a ramfs, it is not paged out and a good place to copy
secrets to. Test whether secrets with a path in /run/keys exist after initrd.
@ddz ddz force-pushed the copy-initrd-secrets-after-early-mount-script branch from b1bc6cc to 442dc55 Compare July 11, 2021 20:27
This modifies initialRamdiskSecretAppender to stage secrets in
/.initrd-secrets/ and stage-1-init to copy them into place after mounting
special file systems. This allows secrets to be copied into ramfs mounts
like /run/keys for use after stage-1 finishes without copying them to disk
(which would not be very secure).
@ddz ddz force-pushed the copy-initrd-secrets-after-early-mount-script branch from 442dc55 to 30b97d7 Compare July 18, 2021 18:34
@ddz
Copy link
Contributor Author

ddz commented Jul 19, 2021

cc: @Mic92 - you reviewed one of my PRs a few years back, would you mind taking a look at this small one?

@dasJ
Copy link
Member

dasJ commented Nov 27, 2021

How would that approach be secure in any way? Your initrd is usually unencrypted and so are the secrets inside. Since your secrets are already in your unencrypted initrd in your unencrypted /boot, why not store them in your regular filesystem as well?

@ddz
Copy link
Contributor Author

ddz commented Nov 27, 2021

How would that approach be secure in any way? Your initrd is usually unencrypted and so are the secrets inside. Since your secrets are already in your unencrypted initrd in your unencrypted /boot, why not store them in your regular filesystem as well?

@dasJ: Thank you for the review! This PR doesn't change how secrets are stored or where. It is only modifying them in a way to permit the system configuration to keep a copy on one of the special file systems after the initrd is done (to have, for example a clean contract between initrd and runtime where needed secrets are available in /run). In the case of my systems, I have a LUKS-encrypted /boot, which keeps the secrets initrd encrypted when powered off and I'm (slowly, sporadically) working on something which keeps them encrypted (e.g. TPM sealed) on the /boot filesystem as well.

# Secrets are named by their full destination pathname and stored
# under /.initrd-secrets/
#
for secret in $(cd "/.initrd-secrets"; find . -type f); do
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for secret in $(cd "/.initrd-secrets"; find . -type f); do
for secret in $(find /.initrd-secrets -not -type d); do

Copy link
Contributor Author

@ddz ddz Dec 8, 2021

Choose a reason for hiding this comment

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

Hello! @SuperSandro2000 suggested the same thing here, but that changes the output and paths assigned to secret (they would have /.initrd-secrets/ still prepended vs. being the full path specified as the attribute name to boot.initrd.secrets). I chose this approach rather than having to strip the first path component inside the loop, which would be less precise. In addition, -not -type d is not equivalent to explicitly including files (-type f), it could include links, sockets, device nodes, etc.

I tried to explain the above in the comment preceding the loop, but if it's still not clear to reviewers, I should try to clarify more.

@dasJ
Copy link
Member

dasJ commented Dec 8, 2021

I still don't see any point in having "secrets" that are in the plain initrd but there's really also no reason we shouldn't allow for this if this is functionality people want to have

@dasJ dasJ merged commit e36ceb6 into NixOS:master Dec 8, 2021
@ddz
Copy link
Contributor Author

ddz commented Dec 10, 2021

Thank you for the merge!

@ddz ddz deleted the copy-initrd-secrets-after-early-mount-script branch January 10, 2024 04:19
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

4 participants