HOTP asked to be resealed even if TOTP good (Picks up on a reinstalled OS even if firmware measurements haven't changed) #1935
Conversation
|
This is Simulating OS reinstallation (wiping /boot/kexec*)So same firmware, meaning:
Other changes
|
tlaurion
changed the title
|
@JonathonHall-Purism in debug. This is To show 280cb1f
So now user is locked in into following steps:
Full debug log of the combined steps (user doing Reset TPM as asked up to setting TPM DUK) with way more interesting call stack for everyone to understand: Note: |
|
To be clear - previously after OS reinstall (including removal of /boot/kexec* files) the recommendation was to do full TPM reset, and now just resealing the HOTP secret should work, right? |
|
|
||
| counter_value=$(cat $HOTP_COUNTER) | ||
| #if HOTP_COUNTER is not present, bail out | ||
| if [ ! -f $HOTP_COUNTER ]; then |
There was a problem hiding this comment.
@marmarek the issue was that previously, if TOTPM previously sealed/unsealed (measured boot from coreboot+heads), Heads was not looking to see if HOTP counter under /boot/kexec_hotp_counter was still present.
| fi | ||
|
|
||
| if [[ "$CONFIG_TPM" = n && "$HOTP" = "Invalid code" ]]; then | ||
| if [[ "$HOTP" = "Invalid code" ]]; then |
There was a problem hiding this comment.
This check only verified if HOTP was invalid if no TPM was in use.
So now, if there is no /boot/kexec_hotp_counter and TPMTOTP can unseal, user is promoted to reseal HOTP alone (OS reinstall use case without firmware upgrade)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
@marmarek added comments in code: with this PR, HOTP counter not being present will guide user to only reseal HOTP and generate hashes and /boot detached signed digest files, as well as selecting default boot and propose to set TPM DUK.
So TPM reset is not needed anymore in case of OS re-installation for TPM1.
For TPM2, TPM primary handle still needs to be created+hashed, which is TPM Reset is advised for creation and hash creation advised for in output as well.
This is workaround for you issue.
tlaurion
force-pushed
the
hotp_fixup_without_firmware_upgrade_boot_wiped
branch
from
6159012 to
75e766a
Compare
JonathonHall-Purism
left a comment
JonathonHall-Purism
left a comment
There was a problem hiding this comment.
Thanks @tlaurion, strategy looks good to me and I left comments on some of the details 💯
initrd/etc/functions
Outdated
| tee /tmp/counter-$1 >/dev/null 2>&1 || | ||
| die "TPM counter increment failed for rollback prevention. Please reset the TPM" | ||
| die "TPM counter increment failed for rollback prevention. Please reset the TPM. Press Enter to continue" | ||
| #TODO: why the die here needs to say to Press Enter to continue? Should be part of die call? |
There was a problem hiding this comment.
Maybe because oem-factory-reset has its own die()? It's not equivalent to this one though so you can't just delete it (kills some TOP_PID it captured rather than exiting) 🤔
tlaurion
force-pushed
the
hotp_fixup_without_firmware_upgrade_boot_wiped
branch
4 times, most recently
from
5cfcc73 to
47c696f
Compare
| DO_WITH_DEBUG tpm forceclear &>/dev/null | ||
| DO_WITH_DEBUG tpm physicalenable &>/dev/null | ||
| DO_WITH_DEBUG tpm takeown -pwdo "$tpm_owner_password" &>/dev/null | ||
| DO_WITH_DEBUG --mask-position 3 tpm takeown -pwdo "$tpm_owner_password" &>/dev/null |
There was a problem hiding this comment.
DEBUG was exposing TPM owner passphrase on log + debug.log
tlaurion
force-pushed
the
hotp_fixup_without_firmware_upgrade_boot_wiped
branch
2 times, most recently
from
c38c13b to
dae9e85
Compare
| mount /dev/sda1 /boot && mount -o remount,rw /boot && rm /boot/kexec* && mount -o remount,ro /boot | ||
| #Generate keys on OpenPGP smartcard: | ||
| mount-usb && gpg --home=/.gnupg/ --card-edit | ||
| mount-usb --mode rw && gpg --home=/.gnupg/ --card-edit |
There was a problem hiding this comment.
bash history now promotes mount-usb --mode rw. nitpick
…ives call hierarchy, fix HOTP resealing only on OS reinstall, clarify TPM increment workflow Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion
force-pushed
the
hotp_fixup_without_firmware_upgrade_boot_wiped
branch
from
dae9e85 to
1f6a975
Compare
| TOTP="NO TPM" | ||
| else | ||
| TOTP=$(unseal-totp) | ||
| # On platforms using CONFIG_BOOT_EXTRA_TTYS multiple processes may try to |
There was a problem hiding this comment.
No more 3 attempts on boot to unseal TPMTOTP: if multiple consoles (Eg Talos-2 with display console + BMC, at worst we could intruduce small delay if race condition still happening, while die asks user to press Enter now, guiding to reseal TPMTOTP or reset TPM if unable to access TPM NVRAM.
|
|
#1935 (comment) testing showed:
@marmarek if you could restest with 1a8d685 I think this is good to merge after testing + @JonathonHall-Purism final code review is needed (die now wait for input; warn waits 1 second; both of which don't require docs changes after reading. I could clean commit logs once testing is reported fixing everything reported here, (and making console output clear and code clearer and having warnings that can actually be seen, errors that ask user acknowledgement etc, which are all input for now merged first steps of #1888 but I have not much more time to put here after that. |
tlaurion
requested review from
JonathonHall-Purism
and removed request for
JonathonHall-Purism
|
Thanks, I'll test it soon, but it will take me some time. That test system got hit with #1882 again :( |
|
HOTP part seems to be okay now, but signing boot files fails with |
Recontextualization of your use case:
@marmarek what would you like to see here? I agree the diff/file non existience is not helpful here. But what would be?
|
|
Ok, so it seems the issue is about the message - normally the "Missing file" error is about signing /boot content with gpg, and the next step is about signing them (see https://openqa.qubes-os.org/tests/145090#step/firstboot/4 for example). TPM owner password is unexpected/surprising prompt. So, maybe just add one line before that prompt explaining why it's asking for it? Alternatively, change the "Missing file" error to include that info? |
Tested under QEMU - wipe of /boot/kexec_* - TPM reset + boot default + define default + TPM DUK - remove qemu *.rom files (so keyring injected is unique and triggers TPM unseal error on boot) - Reseal TPMTOTP+HOTP succeeds giving debug output of TPM counter increment succeeding - comparing hashes under /boot/kexec_rollback.txt validates TPM increment works and is validated (rollback is to prevent copying old kexec*.txt + kexec.sig under /boot) Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…o when no previous file exists Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…rning to user that he will be prompted for TPM Owner Password to set new TPM counter for /boot content rollback prevention Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…for input Signed-off-by: Thierry Laurion <insurgo@riseup.net>
…answers are escaped prior of being put in variable (remove TODO) Signed-off-by: Thierry Laurion <insurgo@riseup.net>
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
tlaurion
force-pushed
the
hotp_fixup_without_firmware_upgrade_boot_wiped
branch
from
1e60236 to
21adb0d
Compare
|
In last rounds of testing (w530), updating firmware required TPM reset for some reason, where resealing TOTP was not enough. I'll try to repro, but I remember this issue vanished with this PR. (which was abandonned a long time ago...... because it involved way too much work). |
|
I remember I hit similar issue before, it was about one too many |
…nitrd/bin/kexec-seal-key
Signed-off-by: Thierry Laurion <insurgo@riseup.net>
| if [ -r "$1" ]; then | ||
| TPM_COUNTER=$(grep counter- "$1" | cut -d- -f2) | ||
| # Robustly extract the first hex string after 'counter-' on any line | ||
| TPM_COUNTER=$(grep -Eo 'counter-[0-9a-fA-F]+' "$1" | sed -n 's/counter-//p' | head -n1 | tr -d '\n') |
There was a problem hiding this comment.
This is where check_tpm_counter failed and where TPM passphtase was asked in silenced output @marmarek
| DEBUG "Detach-signing boot files under kexec.sig: ${param_files}" | ||
|
|
||
| if sha256sum $param_files 2>/dev/null | gpg \ | ||
| if sha256sum $param_files 2>/dev/null | gpg --detach-sign \ |
There was a problem hiding this comment.
cosmetic; just so its easier to search codebase for gpg --detach-sign operations.
| @@ -0,0 +1,10 @@ | |||
| Welcome to the Recovery Shell! | |||
There was a problem hiding this comment.
@JonathonHall-Purism pushing this forward unless you push veto.
|
|
||
| # Traverse the call stack from the earliest caller to the direct caller of TRACE_FUNC | ||
| for ((i = ${#FUNCNAME[@]} - 1; i > 1; i--)); do | ||
| stack_trace+="${FUNCNAME[i]}(${BASH_SOURCE[i]}:${BASH_LINENO[i - 1]}) -> " |
There was a problem hiding this comment.
Precious: permits to see how we arrived here; important for newcomers, audits of code and debugging.
repro
Normal update
OS reinstallation simulation (TPM measurements good, only heads related kexec* files wiped)
Others./docker_repro.sh make BOARD=qemu-coreboot-fbwhiptail-tpm2 USB_TOKEN=Nitrokey3NFC PUBKEY_ASC=pubkey.asc inject_gpg run./docker_repro.sh make BOARD=qemu-coreboot-fbwhiptail-tpm1 USB_TOKEN=Nitrokey3NFC PUBKEY_ASC=pubkey.asc inject_gpg run```!!! Hit enter to proceed to recovery shell !!! |
|
Also note that when in Quiet mode, accessing to recovery shell will
@marmarek the issue was under /etc/functions for counter operations. see #1935 (comment) |
3 participants
Fixes #1562
Supeseeds #1934 + reviewed changes
@marmarek/ @JonathonHall-Purism : good enough for merge?
EDIT: Thanks @JonathonHall-Purism for your review. All comments addressed.
Changes: