Skip to content

boot.initrd.systemd: make TPM2 modules optional#207969

Closed
elohmeier wants to merge 1 commit intoNixOS:masterfrom
elohmeier:tpmfix
Closed

boot.initrd.systemd: make TPM2 modules optional#207969
elohmeier wants to merge 1 commit intoNixOS:masterfrom
elohmeier:tpmfix

Conversation

@elohmeier
Copy link
Contributor

Description of changes

This is a follow-up to a discussion reg. #189676. On some kernel configs the TPM2 modules are not available, e.g. the Raspberry Pi kernels. This change implements a proposed fix to only pull in the TPM2 modules if they are available, fixing the "module not found" issues for these kernels and re-enabling the use of boot.initrd.systemd.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@elohmeier elohmeier requested a review from a team as a code owner December 27, 2022 13:26
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 8.has: module (update) This PR changes an existing module in `nixos/` labels Dec 27, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 27, 2022
Copy link
Contributor

@oxalica oxalica left a comment

Choose a reason for hiding this comment

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

Could you move the guard to variables and also add conditions cfg.package.withTpm2Tss && cfg.package.withCryptsetup? If systemd for initrd is built without cryptsetup or TPM2 support, the module is also unnecessary. So does withFIDO2 and libfido2.

This part also need a clean up to lower the condition granularity.
https://github.com/NixOS/nixpkgs/blob/8a7499d659107387703f9090d672e1e997b311d9/nixos/modules/system/boot/systemd/initrd.nix#L412-L423

@Majiir Majiir mentioned this pull request Jan 10, 2023
13 tasks
@ajs124 ajs124 mentioned this pull request Jan 23, 2023
13 tasks
@RaitoBezarius
Copy link
Member

Can we do something to get this to the finish line?

@ajs124
Copy link
Member

ajs124 commented Mar 23, 2023

@RaitoBezarius someone could respond to the question in my review comment

@RaitoBezarius
Copy link
Member

@RaitoBezarius someone could respond to the question in my review comment

Answered. :)

@elohmeier
Copy link
Contributor Author

Thanks for your feedback. The previous attempt didn't actually work. I've added a new approach with a option boot.initrd.systemd.enableTpm2 as suggested in the comments.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jul 3, 2023
Copy link
Contributor

@ElvishJerricco ElvishJerricco left a comment

Choose a reason for hiding this comment

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

I'd like to see this also exclude the TPM libraries, because they add a considerable amount of size to the initrd:

] ++ optionals cfg.package.withCryptsetup [
# tpm2 support
"${cfg.package}/lib/cryptsetup/libcryptsetup-token-systemd-tpm2.so"
pkgs.tpm2-tss

FIDO2 is fine though, since it barely affects the size at all.

See this test I ran: https://gist.github.com/ElvishJerricco/ca36d26fd30b78b211aea10604b9885e#file-results-txt

@elohmeier
Copy link
Contributor Author

I'd like to see this also exclude the TPM libraries, because they add a considerable amount of size to the initrd:

] ++ optionals cfg.package.withCryptsetup [
# tpm2 support
"${cfg.package}/lib/cryptsetup/libcryptsetup-token-systemd-tpm2.so"
pkgs.tpm2-tss

FIDO2 is fine though, since it barely affects the size at all.

See this test I ran: https://gist.github.com/ElvishJerricco/ca36d26fd30b78b211aea10604b9885e#file-results-txt

Thanks, updated!

Copy link
Contributor

@Majiir Majiir left a comment

Choose a reason for hiding this comment

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

Tested on an armv7l system which can't build with tpm-crb. Code changes LGTM.

Is this config option discoverable for anyone running into the problem? Should we include a note in the enableTpm2 description? Is it possible to detect the missing module and print a NixOS-specific error? (I don't think any of that should hold up the PR, though.)

@elohmeier
Copy link
Contributor Author

@ElvishJerricco can you somehow resolve/close your requested change blocking the merge? I don't know how I could do that since I addressed it by force-pushing an update. Maybe you have an option in GitHub to do that?

Copy link
Contributor

@lopsided98 lopsided98 left a comment

Choose a reason for hiding this comment

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

Confirmed that this allows systemd in initrd to build on my armv7l and armv6l machines.

@elohmeier
Copy link
Contributor Author

Please see follow-up PR here: #253498

@elohmeier elohmeier deleted the tpmfix branch September 6, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 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 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants