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

Simultaneous passwordFile and secret.keyFile #415

Open
midirhee12 opened this issue Oct 10, 2023 · 12 comments
Open

Simultaneous passwordFile and secret.keyFile #415

midirhee12 opened this issue Oct 10, 2023 · 12 comments
Labels
contributions welcome There's nothing left to discuss, feel free to submit a PR for this! enhancement New feature or request

Comments

@midirhee12
Copy link
Contributor

0d39ae5

I think this commit has a bit of an error. This assumes that password files are an alternative to keyfiles. But in reality, many people use passwords as a backup in case they lose their file.

I'm not sure if this is intentional. So I don't know whether to state that this is a bug or a feature request. But here ya go :P

@Lassulus
Copy link
Collaborator

There is additionalKeyFiles? You can also set keyFile and askPassword at the same time? But then booting will fail if the keyFile is unavailable I guess.

@midirhee12
Copy link
Contributor Author

My intuition was that additionalKeyFiles were for when you wanted more than just one key file. So you cannot use secret.keyFile and passwordFile together. Also, it looks like the if askPassword part is ignored if you set a secret.keyFile since it is in an else branch.

@aos
Copy link
Contributor

aos commented Oct 15, 2023

I work around this by adding a postCreateHook on the volume I want to encrypt with a password and a key file simultaneously. Here's an example:

# part of the disko-config.nix
root = {
  size = "100%";
  content = {
    type = "luks";
    name = "cryptroot";
    extraOpenArgs = [ "--allow-discards" ];
    # Encrypt with the key first
    settings = {
      keyFile = "/dev/mapper/cryptkey";
      keyFileSize = 8192;
    };
    # Use a postCreateHook to add the passphrase from a file
    postCreateHook = ''
      cryptsetup luksAddKey --key-file /dev/mapper/cryptkey \
                            --keyfile-size 8192 \
                            /dev/disk/by-partlabel/dev-vda-root /tmp/disk.key
    '';
    content = {
    # ...
    };
};

There's probably a better way to do fill in the values in the hook, but this works for me.

The hooks are really powerful and you can do some fun stuff with them. I'll see if I can send a PR to document them better!

@aos
Copy link
Contributor

aos commented Oct 15, 2023

After digging into it further - seems like the only reason my solution works is because we call cryptsetup luksOpen during the creation of the LUKS drive.

Unfortunately, {pre,post}MountHooks aren't implemented and I think these would be pretty beneficial. One good example is to add a hostKey to initrd for LUKS remote unlock after mounting the boot partition.

If I have time I'll send a PR for it.

@phaer
Copy link
Member

phaer commented Oct 16, 2023

Unfortunately, {pre,post}MountHooks aren't implemented and I think these would be pretty beneficial. One good example is to add a hostKey to initrd for LUKS remote unlock after mounting the boot partition.

I think --extra-files should work this?
So you would put the directory tree & files you want to copy into a temp dir e.g. /tmp/extra_files and then use --extra-files /tmp/extra_files to copy them into your target system during installtion. So if you have /tmp/extra_files/etc/ssh/initrd_ssh_host_rsa_key and set

boot.initrd.network.ssh.hostKeys = ["/etc/ssh/initrd_ssh_host_rsa_key"];

in your nixos config, you should have the right host key in your initrd config.

The hooks are really powerful and you can do some fun stuff with them. I'll see if I can send a PR to document them better!

That would be very welcome!

@midirhee12
Copy link
Contributor Author

I see this more as a work around than actually fixing the issue.

@phaer
Copy link
Member

phaer commented Oct 16, 2023

I believe it might be easier to discuss the merits of this opinion if you could share with us why you regard it as a a workaround and what "actually fixing the issue" would look like from your perspective? :)

@midirhee12
Copy link
Contributor Author

The actual issue is that secret.keyFile and passwordFile cannot be used simulateously. Not that is is impossible to set a keyfile using some hack or work around. It would be best if we fixed the issue so secret.keyFile and passwordFile can be used intuitively as expected. For the user, even just using additionalKeyFiles would be nicer than having to do a hook.

@midirhee12
Copy link
Contributor Author

midirhee12 commented Oct 16, 2023

In the meantime, I've had a thought to make the disko options a bit more intuitive. You change additionalKeyFiles to secret.keyFiles (note the plurality b'c it's a list) and then remove secret.keyFile entirely. That way to add any additional let files, you just add more to the list. And by default, password files and key files are compatible.

Edit: I can start on a PR for this change if this behavior is desired.

@phaer
Copy link
Member

phaer commented Oct 16, 2023

The actual issue is that secret.keyFile and passwordFile cannot be used simulateously. Not that is is impossible to set a keyfile using some hack or work around.

Ah, without quotations or context it's rather hard to tell whether your posts refer to the one before you. So in my understanding that actually fixes the issue insofar as that you can simultaneously use a password an key file?

Getting the interface nicer would surely be appreciated, happy to review - and your proposal sounds like the right direction to me :)

@midirhee12 midirhee12 changed the title Simultaneous password and key file Simultaneous passwordFile and secret.keyFile Oct 16, 2023
@aos
Copy link
Contributor

aos commented Oct 17, 2023

So if you have /tmp/extra_files/etc/ssh/initrd_ssh_host_rsa_key and set boot.initrd.network.ssh.hostKeys = ["/etc/ssh/initrd_ssh_host_rsa_key"]; iin your nixos config, you should have the right host key in your initrd config.

Yeah I could do that - but that would mean I generate the host key outside of the host that uses it. I wanted to generate the host key on the host that would be using it, hence the need for a postMount hook.

I've worked around this by writing my own subpar implementation of nixos-anywhere that runs the disko script on the target host, generates the key and places it into the defined location under boot.initrd.network.ssh.hostKeys and does the final nixos-install.

I think having it all in the disko config would make it much cleaner :-)

@iFreilicht iFreilicht added enhancement New feature or request contributions welcome There's nothing left to discuss, feel free to submit a PR for this! labels Oct 14, 2024
@iFreilicht
Copy link
Contributor

@midirhee12 I agree with @phaer, your proposal sounds good, and a PR for this would be welcome.

If you (or anyone else) decide to implement this, please use mkRenamedOptionModule and mkRemovedOptionModule from the nixpkgs lib to ensure backwards compatibility. We can then remove the old options in the 2.x release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome There's nothing left to discuss, feel free to submit a PR for this! enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants