-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
systemd: Fix systemd-{cryptenroll,cryptsetup} TPM2 and FIDO2 support (attempt #3) #189676
Conversation
Do you think you can add a NixOS test? |
@RaitoBezarius I was already writing one when I looked back at this PR to check something and saw your comment… should be done in an hour or so |
Here we go: import ./make-test-python.nix ({ lib, pkgs, ... }: {
name = "systemd-initrd-luks-tpm";
nodes.machine = { pkgs, ... }: {
# Use systemd-boot
virtualisation = {
emptyDiskImages = [ 512 ];
useBootLoader = true;
useEFIBoot = true;
qemu.options = ["-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-tis,tpmdev=tpm0"];
};
boot.loader.systemd-boot.enable = true;
boot.initrd.availableKernelModules = [ "tpm_tis" ];
environment.systemPackages = with pkgs; [ cryptsetup ];
boot.initrd.systemd = {
enable = true;
};
specialisation.boot-luks.configuration = {
boot.initrd.luks.devices = lib.mkVMOverride {
cryptroot = {
device = "/dev/vdc";
crypttabExtraOpts = [ "tpm2-device=auto" ];
};
};
virtualisation.bootDevice = "/dev/mapper/cryptroot";
};
};
testScript = ''
import subprocess
import os
import time
class Tpm:
def __init__(self):
os.mkdir("/tmp/mytpm1")
self.start()
def start(self):
self.proc = subprocess.Popen(["${pkgs.swtpm}/bin/swtpm", "socket", "--tpmstate", "dir=/tmp/mytpm1", "--ctrl", "type=unixio,path=/tmp/mytpm1/swtpm-sock", "--log", "level=20", "--tpm2"])
def wait_for_death_then_restart(self):
while self.proc.poll() is None:
print("waiting for tpm to die")
time.sleep(1)
assert self.proc.returncode == 0
self.start()
tpm = Tpm()
# Create encrypted volume
machine.wait_for_unit("multi-user.target")
machine.succeed("echo -n supersecret | cryptsetup luksFormat -q --iter-time=1 /dev/vdc -")
machine.succeed("PASSWORD=supersecret SYSTEMD_LOG_LEVEL=debug systemd-cryptenroll --tpm2-pcrs= --tpm2-device=auto /dev/vdc |& systemd-cat")
# Boot from the encrypted disk
machine.succeed("bootctl set-default nixos-generation-1-specialisation-boot-luks.conf")
machine.succeed("sync")
machine.crash()
tpm.wait_for_death_then_restart()
# Boot and decrypt the disk
machine.wait_for_unit("multi-user.target")
assert "/dev/mapper/cryptroot on / type ext4" in machine.succeed("mount")
'';
}) Based on the existing systemd-initrd-luks-* family of tests – feel free to add to the PR. |
(it passes on master with this PR added, not much rebuilding required for such a simple config, took me about 10 minutes) |
a0f1ad4
to
64a7625
Compare
Can we add to |
Here's another minor patch that will make the additional store paths optional. diff --git a/nixos/modules/system/boot/systemd/initrd.nix b/nixos/modules/system/boot/systemd/initrd.nix
index f10a8c1313f..f0ec3db3b21 100644
--- a/nixos/modules/system/boot/systemd/initrd.nix
+++ b/nixos/modules/system/boot/systemd/initrd.nix
@@ -394,6 +394,14 @@ in {
"${cfg.package}/lib/systemd/system-generators/systemd-hibernate-resume-generator"
"${cfg.package}/lib/systemd/system-generators/systemd-run-generator"
+ # utilities needed by systemd
+ "${cfg.package.util-linux}/bin/mount"
+ "${cfg.package.util-linux}/bin/umount"
+ "${cfg.package.util-linux}/bin/sulogin"
+
+ # so NSS can look up usernames
+ "${pkgs.glibc}/lib/libnss_files.so.2"
+ ] ++ optionals cfg.package.withCryptsetup [
# tpm2 and fido2 support
"${cfg.package}/lib/cryptsetup/libcryptsetup-token-systemd-tpm2.so"
"${cfg.package}/lib/cryptsetup/libcryptsetup-token-systemd-fido2.so"
@@ -402,14 +410,6 @@ in {
# the unwrapped systemd-cryptsetup executable
"${cfg.package}/lib/systemd/.systemd-cryptsetup-wrapped"
-
- # utilities needed by systemd
- "${cfg.package.util-linux}/bin/mount"
- "${cfg.package.util-linux}/bin/umount"
- "${cfg.package.util-linux}/bin/sulogin"
-
- # so NSS can look up usernames
- "${pkgs.glibc}/lib/libnss_files.so.2"
] ++ jobScripts;
targets.initrd.aliases = ["default.target"]; |
64a7625
to
8084e43
Compare
We could, but I'd prefer not to do that to make it less fragile and reduce work in the future in case tpm2-tss/libfido2 change their dependencies.
Thanks, applied. |
Can that not be said about the rest of the stuff in |
8084e43
to
852fe4d
Compare
No, because the dependencies are listed in the ELF (that make-initrd-ng picks up automatically), unlike tpm2-tss which dlopen's libraries dynamically depending on the interface. I went ahead and did it anyways and looks like it saved us 1MB, but at what cost? |
You're probably right, maybe just keep libfido's single library and still use pkgs.tpm2-tss? |
852fe4d
to
451587a
Compare
To measure the impact on the initrd size, let's use the following simple expression: let
pkgs = import ./. {};
nixos = pkgs.nixos {
boot.initrd.systemd.enable = true;
};
in nixos.config.system.build.initialRamdisk
Note that the sizes here are after zstd compression. Including the percentage increases in the table would be misleading, because the baseline sizes in actual configs are larger due to the bundled kernel modules. |
Should we remove these assertions now? Seems there are no more concerns? nixpkgs/nixos/modules/system/boot/luksroot.nix Lines 908 to 911 in 74d81b4
|
~3MB of initrd growth for fairly large quality of life improvements seems pretty reasonable to me 👍 |
For anyone interested, I wrote a FIDO2 test using QEMU with CanoKey support in oxalica@d75bc7b, which depends on #194254. |
Done! The test won't work in One weird thing though, if I add |
Seems the private key of the virtual card is hard-coded. |
Small notes:
Otherwise it worked great, thanks! (EDIT: sorry, this can be ignored, I missed the context that @oxalica clarifies in the next reply.) |
I believe that's unrelated and deserves an individual issue about systemd-initrd. You could try to shuffle around the loading order of your graphics driver, between This PR is not just about systemd-initrd. Currently on master, even full systemd package doesn't support FIDO2 enrolling. |
@GrahamcOfBorg build cryptsetup systemd |
@GrahamcOfBorg test systemd cryptsetup |
@GrahamcOfBorg build cryptsetup.passthru.tests For some reason it failed on x86? maybe flakey? just rerunning before merging |
Co-authored-by: Janne Heß <janne@hess.ooo> Co-authored-by: Ilan Joselevich <personal@ilanjoselevich.com>
Update pkgs/os-specific/linux/systemd/default.nix Co-authored-by: Janne Heß <janne@hess.ooo> Co-authored-by: Ilan Joselevich <personal@ilanjoselevich.com> Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
This improves the out-of-box experience of TPM2 unlocking at a small (50K) overhead.
4fe76c2
to
78f929c
Compare
Rebased to resolve conflicts. The failure seems to be unrelated, and the tests (
|
Just in case anyone is having a similar problem I created an issue on upstream: systemd/systemd#25128 |
boot.initrd.availableKernelModules = [ "autofs4" ]; # systemd needs this for some features | ||
boot.initrd.availableKernelModules = [ | ||
"autofs4" # systemd needs this for some features | ||
"tpm-tis" "tpm-crb" # systemd-cryptenroll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maybe make this optional? Or conditional on if the kernel has this option set?
I have some custom kernel configs that really don't need this module and also use the new initrd and now the initrd build fails with modprobe: FATAL: Module tpm-tis not found in directory […]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can have a new option under boot.initrd.luks.devices.<name>
that indicates whether a LUKS makes use of cryptenroll-based TPM2/FIDO2 unlocking, and only insert the modules and extra files when a volume with those options enabled exists. However, this can be confusing for non-systemd-stage1 users, especially when boot.initrd.luks.devices.<name>.fido2
already exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if disabling these kernel modules would an opt-in config option.
We can't determine at evaluation time if there's a fido luks slot configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough. iirc we can determine at eval time if the kernel even has these modules enabled. maybe using system.requiredKernelConfig
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the kernel config, these options/modules don't even seem to be available on non-x86 systems or systems without ACPI, so I'm really not sure they should be enabled by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmh, that'd be a problem. Can we check if the kernel has these modules configured, and only include them in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think something like this might work:
--- a/nixos/modules/system/boot/systemd/initrd.nix
+++ b/nixos/modules/system/boot/systemd/initrd.nix
@@ -334,8 +334,11 @@ in {
boot.initrd.availableKernelModules = [
"autofs4" # systemd needs this for some features
- "tpm-tis" "tpm-crb" # systemd-cryptenroll
- ];
+ ]
+ # systemd-cryptenroll
+ ++ lib.optional (config.boot.kernelPackages.kernel.config.isEnabled "TCG_TIS") "tpm-tis"
+ ++ lib.optional (config.boot.kernelPackages.kernel.config.isEnabled "TCG_CRB") "tpm-crb"
+ ;
boot.initrd.systemd = {
initrdBin = [pkgs.bash pkgs.coreutils cfg.package.kmod cfg.package] ++ config.system.fsPackages;
but only if we actually set these options and don't rely on kernel defaults. Maybe.
For some reason nixosTests.systemd-initrd-luks-fido2
and nixosTests.systemd-initrd-luks-tpm2
build even with:
boot.initrd.availableKernelModules = [
"autofs4" # systemd needs this for some features
];
so I'm not sure how to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VM test passes without adding any extra modules but on (some?) actual hardware they are required.
Description of changes
This PR makes it possible to use
systemd-cryptenroll
(and its integration insystemd-cryptsetup
) to automatically unlock LUKS volumes using TPM2 or FIDO2.Based on #171242 (attempt #1) and #179823 (attempt #2).
Relavent discussions about potential attack vectors: #179823 (comment)
How to test
You can test this PR with the minimal amount of rebuilds by pulling in the updated packages and NixOS module on my
cryptenroll-master
branch:Reboot into the new configuration. After rebooting, add a new LUKS key slot with the key sealed to the current PCR values:
Note: The PCR list here is an example only! You need to understand what's being measured and when. One bootloader-specific thing is initrd: systemd/systemd#12716
Then, reboot again. This time the volume should be decrypted without prompting you for a password.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes