Ease TPM Disk Unlock Key sealing/resealing after TOTP mismatch (firmware upgrade) + warn and die changes #1482
Conversation
|
Now works as expected on TPM1/TPM2.
|
|
And now rebasing on master so that #1478 is considered tested on qemu, but unrelated here. |
tlaurion
force-pushed
the
ease_tpm_disk_unlock_key_resealing_after_totp_mismatch-warn_and_die_changes
branch
from
798bbff
to
bca4469
Compare
|
And now cleaning commits and squashing some more |
tlaurion
force-pushed
the
ease_tpm_disk_unlock_key_resealing_after_totp_mismatch-warn_and_die_changes
branch
2 times, most recently
from
0c99c33
to
d20eea7
Compare
…when resealing TOTP)
Changes:
- As per master: when TOTP cannot unseal TOTP, user is prompted to either reset or regenerate TOTP
- Now, when either is done and a previous TPM Disk Unlock Key was setuped, the user is guided into:
- Regenerating checksums and signing them
- Regenerating TPM disk Unlock Key and resealing TPM disk Unlock Key with passphrase into TPM
- LUKS header being modified, user is asked to resign kexec.sig one last time prior of being able to default boot
- When no previous Disk Unlock Key was setuped, the user is guided into:
- The above, plus
- Detection of LUKS containers,suggesting only relevant partitions
- Addition of TRACE and DEBUG statements to troubleshoot actual vs expected behavior while coding
- Were missing under TPM Disk Unlock Key setup codepaths
- Fixes for linuxboot#645 : We now check if only one slots exists and we do not use it if its slot1.
- Also shows in DEBUG traces now
Unrelated staged changes
- ash_functions: warn and die now contains proper spacing and eye attaction
- all warn and die calls modified if containing warnings and too much punctuation
- unify usage of term TPM Disk Unlock Key and Disk Recovery Key
Tested working on both TPM1/TPM2 under debian bookwork, standard encrypted TLVM setup
tlaurion
force-pushed
the
ease_tpm_disk_unlock_key_resealing_after_totp_mismatch-warn_and_die_changes
branch
from
d20eea7
to
67c865d
Compare
|
@JonathonHall-Purism : this is ready for review. Tested on both qemu-coreboot-whiptail-tpm[1,2] I didn't push my luck here, but I think we are due to adding an Here is the visual, running now one last time where a TPM2 Disk Unlock Key previously setuped ( TPM Disk Unlock Key already setuped (no boot default defined after firmware upgrade: TOTP unsealing error)
And then if attempting to default boot without a boot default defined
Default boot looks like TODO in next issues/PR:
Workaround is to pass USB security dongle to another qube and then back to testing vm. Annoying but...
|
tlaurion
changed the title
|
@natterangell this fixes #1474 Or does it?! |
|
@JonathonHall-Purism Seems like I talked too fast.... Putting in draft. |
There was a problem hiding this comment.
Thanks @tlaurion. I see this went back to draft but I still reviewed, hope the comments help you track down any remaining issues. Looks like the initial setup flow may need to be tested (kexec-save-default, saving a LUKS key when none has ever been saved before), I noted some things there that I think will cause it to fail.
This is definitely a bit usability improvement for this feature 💪
I think the suggestion to add 'info' is a good one but can be done later if you prefer, the cleanups to warn/die are great. I think 'initrd/bin/unpack_initramfs.sh' may be the solution to extracting the crypttab from the initrd, noted that in some specific comments.
initrd/bin/gui-init
Outdated
| @@ -173,7 +173,8 @@ generate_totp_hotp() | |||
| # clear screen | |||
| printf "\033c" | |||
| else | |||
| warn "Unsealing TOTP/HOTP secret from previous sealed measurements failed. Try "Generate new HOTP/TOTP secret" option if you updated firmware content." | |||
| warn "Unsealing TOTP/HOTP secret from previous sealed measurements failed" | |||
| warn "Try "Generate new HOTP/TOTP secret" option if you updated firmware content" | |||
There was a problem hiding this comment.
The internal quotes need to be escaped to appear in the output (or change the outer quotes to single quotes)
There was a problem hiding this comment.
I don't see this addressed in the review commit (8809588)
There was a problem hiding this comment.
@JonathonHall-Purism should be fixed now
There was a problem hiding this comment.
Looks good in 47eba7d 👍
| for uuid in `cat "$TMP_KEY_DEVICES" | cut -d\ -f2`; do | ||
| # NOTE: discard operation (TRIM) is activated by default if no crypptab found in initrd | ||
| echo "luks-$uuid UUID=$uuid /secret.key luks,discard" | tee -a "$INITRD_DIR/$crypttab_file" | ||
| # TODO: cpio -t is unfit here :( it just extracts early cpio header and not the whole file. Replace with something else |
There was a problem hiding this comment.
I think the 'something else' you want is initrd/bin/unpack_initramfs.sh 😉
That's designed to unpack concatenated initrds like Linux does, it works for the early microcode initrd followed by the real initrd, details in the documentation comment at the top of the file.
There was a problem hiding this comment.
Hmmm. ZSTD would now be a new requirement. Will switch that as being default for all boards and see if things break for legacy boards and if it does, bye bye legacy boards #1421
~ # unpack_initramfs.sh /boot/initrd.img-6.1.0-11-amd64 /tmp/test
DEBUG: Unpacking /boot/initrd.img-6.1.0-11-amd64 to /tmp/test
DEBUG: archive segment 303730373031: 54560 bytes
DEBUG: archive segment 000000000000: 224 bytes
DEBUG: archive segment 28b52ffd8460: 53378107 bytes
~ # find /tmp/test | grep crypttab
/tmp/test/cryptroot/crypttab
There was a problem hiding this comment.
@JonathonHall-Purism Applied change at 03d8f93. Will now use that in code thanks for the tip!
There was a problem hiding this comment.
@JonathonHall-Purism works under e291797
There was a problem hiding this comment.
Looks good in 47eba7d 👍
… boards Rationale: cpio -t alone cannot extract initrd past early cpio (microcode) in most packed initrd. unpack_initramfs.sh already under master comes to the rescue, but its usage up to today was limited to pass firmware blobs to final OS under boards/librem_mini_v2 Debian OSes (and probably others) need to have cryptroot/crypttab overriden directly, otherwise generic generation of crypttab is not enough. Extracting crypttab and overriding directly what is desired by final OS and exposed into /boot/initrd is the way to go otherwise hacking on top of hacks. This brings default packed modules under Heads to 5 modules, which needs to be deactivate in board configs if undesired: user@heads-tests-deb12:~/heads$ grep -Rn "?= y" modules/ | grep -v MUSL modules/zlib:1:CONFIG_ZLIB ?= y modules/zstd:3:CONFIG_ZSTD ?= y modules/exfatprogs:2:CONFIG_EXFATPROGS ?= y modules/busybox:2:CONFIG_BUSYBOX ?= y modules/e2fsprogs:2:CONFIG_E2FSPROGS ?= y
tlaurion
force-pushed
the
ease_tpm_disk_unlock_key_resealing_after_totp_mismatch-warn_and_die_changes
branch
from
d14b7ad
to
03d8f93
Compare
|
@JonathonHall-Purism : created staging commit 8809588 for review prior of modifying code to use unpack_initramfs.sh in code. Committing into same branch cancelled building for 03d8f93. Kicked the build to resume from failing at |
|
Thanks @tlaurion . Reviewed and commented (I'm unable to resolve threads but I noted which were fixed). The remaining notes were the quotes in gui-init (minor) and the grep in kexec-save-default. 100% agree with including zstd by default. Will watch out for changes using unpack_initramfs.sh, let me know if you have any trouble with that. |
No need to worry for now (but still looking for first sign of trouble to officially drop legacy boards) Size changes for t430-legacy-hotp Repro Where important changes here are Which basically tells us that adding /bin/zstd-decompress of 174352 bytes ends up being added under tools.cpio for the same size variation (uncompressed) Ending compressed (amongs all the small changes in scripts which are neglictible because highly compressible) to So the cost of adding ZSTD can be approximated to 4107264-4169216=-61952 bytes (a loss of 61952 bytes). |
…ommits once confirmed good
tlaurion
force-pushed
the
ease_tpm_disk_unlock_key_resealing_after_totp_mismatch-warn_and_die_changes
branch
from
8809588
to
64ad01f
Compare
tlaurion
changed the title
|
@natterangell e291797 should finally fix it. Now initrd is really parsed and crypttab found there is overriden. Would need to fix wiki https://osresearch.net/InstallingOS/#default-boot-and-disk-unlock I think this is a massive improvement. Let me know how that goes for you! Tested:
@JonathonHall-Purism Ready for code review, I will then squash and clean commit on a rainy day. |
…king uniformization for DUK
|
@SvenSemmler 47eba7d creates proper suggestion in code for LUKS, and changes here finally uses Qubes initrd to extract crypttab and create overrides when there is two LUKS containers (BRTFS Qubes installation). I tested this commit on BRTFS Q4.2 install and it works:
If you want to deep dive into seeing how Heads work, you can enable/disable DEBUG+TRACE output by toggling it in the configuration setting menu: #1479. Without debug active on X230: |
|
Thanks for addressing everything @tlaurion , this looks ready to merge to me 👍 |
|
QOS: Qubes OS Reused PR rom on main laptop to
Further todos:
The commit trail is not super clean, but past experience on Github and off-channel discussion with @JonathonHall-Purism made us agree that commit trail is not so ugly as of now and commits show what was fixed as part of the discussions under this PR. |
|
On the road, so brief reply. ROM tested and verified working (x230-maximized and t420-totp-maximized).
Discovered that the previous ugly workaround for Debian prevented booting unless unsealing TPM, so nice to get rid of that!Great work! I can adjust the wiki in a separate PR.
…----
Edited by tlaurion to only keep relevant quotes from email reply (bad bad github)
|
This one I think is a long-awaited one! When a disk has TPM Disk Unlock Key (DUK) previously setup, resealing TPM/resetting TPM goes through the logic of reusing previously setup LVM / LUKS container devices and goes through resigning automatically!
Ready for review, works perfectly on tpm1 and tpm2!
Fixes #1474 and #645 #1488
TODO: