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/luksroot.nix: fallback to interactive password entry when no keyfile found #30416

Merged
merged 3 commits into from Mar 5, 2018

Conversation

symphorien
Copy link
Member

Motivation for this change

being able to boot if the keyfile is missing but I know the passphrase

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@Moredread
Copy link
Contributor

Nice, that was something I wanted for my Fileserver.

The only issue I see is, that I can imagine use cases where I want to unlock a device at boot if a keyfile is available, but still continue booting when it is missing. I'm not sure if this is possible at the moment with luksroot but I have this issue with the filesystems.*.encrypted module atm so it would be nice if that'd (still) work.

@symphorien
Copy link
Member Author

That's a good point -- I was mostly thinking of the case of unlocking / : if it is not available, the system just doesn't boot.
What should I do ? adding an option required guarding this fallback mechanism ? Adding a timeout to the cryptsetup prompt ?

@fpletz fpletz requested a review from abbradar October 14, 2017 20:25
@fpletz fpletz added 0.kind: enhancement 6.topic: nixos 9.needs: port to stable A PR needs a backport to the stable release. labels Oct 14, 2017
@Moredread
Copy link
Contributor

maybe a enablePasswordFallback oder disablePasswordFallback Option?

If a timeout is chosen there should be a option to configure it IMO, with the possibility of waiting forever. Actually that might be the most flexible way.

@abbradar
Copy link
Member

abbradar commented Oct 15, 2017

Timeout feels somewhat wrong to me (what if you have a long password?). Something better may be to set a number of retries before booting continues. Usual three seems to be enough. What do you think?

EDIT: I need to test that, but won't ^C kill the password prompt?

@symphorien
Copy link
Member Author

@abbradar not sure I understand your point about retries. The main problem is that if the luks device is not so important, and that the key goes missing, the system can't boot unattended anymore.
That the user can ^C is (from that point of view) irrelevant.
By the way, if a key is missing with the current bahavior for a "non-essential" drive, what happens ?

@abbradar
Copy link
Member

@symphorien Hm, indeed, ^C doesn't help the usecase when you want to continue unattended boot with the key missing but the partition not strictly required.

Maybe then use timeout not for the prompt itself, but for the fallback? In lines of "Key for ${foo} is not found. Press any key in N seconds to enter password phrase instead". Then one can type the password at their own leisure.

@abbradar
Copy link
Member

@symphorien I don't remember the code well now, but I think the system will just continue to boot. The drive will stay encrypted.

This option, if set to true, enables fallbacking to an interactive
passphrase prompt when the specified keyFile is not found.

The default is false, which is compatible with previous behavior and
doesn't prevent unattended boot.
@symphorien
Copy link
Member Author

I pushed a commit implementing an option fallback guarding the interactive prompt. It is set to false by default and thus backward compatible.
I am open to bikeshedding about the name of the option.
I tested all the 4 combination of {key file is present}×{fallback==true} on my computer. It works for me™

@Moredread
Copy link
Contributor

I think it would be good to name the option, so it is clear what it does. But I don't have a good idea sadly.

A few ideas, but they might be too verbose

  • fallbackToPasswordIfKeyFileIsMissing (long, and doesn't have 'keyfile' at the start)
  • passwordFallback (unclear that it only applies when keyfile is set)
  • keyFileFallback (unclear what that means. E.g. is it another file?)
  • keyFilePasswordFallback (Could be interpreted, that is is a string with the password)

I think I'd prefer the last one, if no one has a better idea.

@symphorien
Copy link
Member Author

I had also thought of interactiveFallback

@symphorien
Copy link
Member Author

Sorry for the delay.
I finally went for fallbackToPassword.
If nobody objects, I think this is ready to merge.

@symphorien
Copy link
Member Author

hum should I add something to make this mergeable ?

@fpletz fpletz removed the 9.needs: port to stable A PR needs a backport to the stable release. label Jan 28, 2018
@fpletz fpletz added this to the 18.03 milestone Jan 28, 2018
@Ekleog
Copy link
Member

Ekleog commented Mar 3, 2018

I know there's the 18.03 milestone, but… is there anything missing for this PR to be merged?

@fpletz fpletz merged commit 17ba8bb into NixOS:master Mar 5, 2018
@symphorien symphorien deleted the luksnokey branch May 18, 2019 16:02
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

6 participants