Allow fallback to interactive key entry if TPM2 fails

[aaronjamt]

This PR contains 2 commits that, together, will allow the stock interactive password/key entry to take over if the TPM2 (or keyfile) fails to unlock the disk. If, for instance, you have configured the TPM2 to require that the Linux kernel hasn't been modified but you update it, TPM will not automatically unlock the disk anymore. Currently, it is technically possible to unlock using a regular LUKS key, but only if you're using regular text mode and if you type your password (in plain text without any masking) without being prompted for it. Now, with these commits, it will fall back to whatever default password entry screen is provided by the OS and, if the base OS supports it, allow you to enter your password into a graphical interface or, at the very least, properly prompt you for it and mask what you type in.

[wmcelderry]

Password fallback expectation - it does/should work (untested), but I'm not a fan of the code - executing the attempt to unlock inside the if statement chain with '&&' to make it conditional.

I can see it is less code than the alternative, but it's not "easy to read".

To be fair, I've got patches in the repo, so looking at a diff of those patches is also not easy to read - so I know I'm hypocritical... sorry.

I like the general changes in the other commit - but I feel I should also sanity check it before accepting it and I cannot do that right now.

Thanks for the 2 PRs though - I'll merge them and/or equivalent soon.

[aaronjamt]

I've tested these commits on my own machine (they're actually the result of applying the patches from your repo then tweaking until it worked). I agree that the && test isn't very clean but I'm not sure of a better way to do that than duplicating the run_keyscript | unlock mapping line. The idea I had was to leave the if statement alone and move the unlock_mapping ${CRYPTTAB_KEY} to its own inner if statement, then make both statements have an else clause with run_keyscript | unlock mapping.

[aaronjamt]

Actually, while writing that comment, I realized that you could actually have a variable outside the while loop that is set to the the result of the original if statement. Inside the loop, just do if that variable then try keyscript/TPM else use the original run_keyscript | unlock_mapping. Then, in the if section, check if the keyscript/TPM worked and, if not, change the variable to make it try interactively next loop.

[abraha2d]

Tested this on my laptop successfully just now.

Disabled secure boot a few days ago to test some stuff, and was expecting to have to manually enter my passphrase, but it took me a while to figure out how to do that.

Came looking here to poke around and try to figure out how to fall back to graphical password entry, and what do you know... thanks @aaronjamt!

I agree that the logic is hard to follow (the double negatives don't help either), looking forward to the updates.

[aaronjamt]

Tested this on my laptop successfully just now.

Disabled secure boot a few days ago to test some stuff, and was expecting to have to manually enter my passphrase, but it took me a while to figure out how to do that.

Came looking here to poke around and try to figure out how to fall back to graphical password entry, and what do you know... thanks @aaronjamt!

I agree that the logic is hard to follow (the double negatives don't help either), looking forward to the updates.

Glad that this helped someone, I've had it installed on my laptop since I created this PR and kinda forgot about it. I also agree that the logic isn't great, so if you (or anyone else) has suggestions to make it cleaner I'd love to clean it up. I believe the reason I did it this way is to minimize the number of lines changed by the patch file (for future-proofing sake) but maybe a more readable patch that could be understood and fixed down the road would be better.

[evilhamsterman]

@wmcelderry this looks as readable as shell can be to me. It could be done a little differently by putting the unlock_mapping in a separate if to make it a bit easier to document. But this looks to accomplish the goal reasonably well I think it would be good to merge.

[aaronjamt]

I think it would be better to use a boolean variable then have a few nested if statements for the conditions which will toggle the flag if they all pass. Then we can have a simple if flag/else check for the fallback.

[mjkl-gh]

First of all, many thanks for writing these scripts to the maintainer, and thanks for making this PR to fix something I require for this to be usuable.

I was wondering what is holding back this PR? I've recently had to reinstall and was suprised this wasn't merged yet.

[aaronjamt]

First of all, many thanks for writing these scripts to the maintainer, and thanks for making this PR to fix something I require for this to be usuable.

Thanks, I appreciate it! I like to think of it as my duty to the open source community: find something I don't like, fix it, then push it to the rest of the world

I was wondering what is holding back this PR? I've recently had to reinstall and was suprised this wasn't merged yet.

I believe the issue is that the way I've written it is somewhat messy (I don't entirely remember why I wrote it the way I did, if that's the issue I'd be happy to clean it up). Maybe @wmcelderry can pitch in here?