WiP: Improve checksum verification upon totp unseal errors #1508
Conversation
Raw notes: - Writing multi-line output to /dev/kmsg didn't work. We now output one line of message in DEBUG calls - When TOTP unseal errors happened, the use needed to also sign /boot without knowing if things were right. - Now integrity report is given - If gpg detached signature or hashes are invalid, we give the user a detailed report of hashes being different is detaches signature is good - There were multiple instances of verify_global_hashes - The one from gui-init has been moved to etc/functions, and calls requiring to update the hashes now pass optional parameter - Are some /tmp/hash_output relevance missed somewhere? What should be improved further?
|
Played around with qemu-fbwhiptail-tpm1-hotp, qemu-fbwhiptail-tpm2-hotp and love the improvements. |
|
Currently fails on default boot and select boot option. Putting as draft |
|
From reviewing verify_global_hashes, it looks like there were significant differences between these two functions. We can't just replace one with the other. In particular:
I do think this needs to be refactored, but we can't just replace one with the other. |
| if [ -f "$TMP_PACKAGE_TRIGGER_PRE" ]; then | ||
| PRE_CHANGED_FILES=$(grep '^CHANGED_FILES' $TMP_PACKAGE_TRIGGER_POST | cut -f 2 -d '=' | tr -d '"') | ||
| TEXT="The following files failed the verification process BEFORE package updates ran:\n${PRE_CHANGED_FILES}\n\nCompare against the files $CONFIG_BRAND_NAME has detected have changed:\n${CHANGED_FILES}\n\nThis could indicate a compromise!" | ||
| TEXT+="$UPDATE_TEXT" | ||
| # if files changed after package manager started, probably caused by package manager | ||
| elif [ -f "$TMP_PACKAGE_TRIGGER_POST" ]; then | ||
| LAST_PACKAGE_LIST=$(grep -E "^(Install|Remove|Upgrade|Reinstall):" $TMP_PACKAGE_TRIGGER_POST) | ||
| UPDATE_INITRAMFS_PACKAGE=$(grep '^UPDATE_INITRAMFS_PACKAGE' $TMP_PACKAGE_TRIGGER_POST | cut -f 2 -d '=' | tr -d '"') | ||
|
|
||
| if [ "$UPDATE_INITRAMFS_PACKAGE" != "" ]; then | ||
| TEXT="The following files failed the verification process AFTER package updates ran:\n${CHANGED_FILES}\n\nThis is likely due to package triggers in$UPDATE_INITRAMFS_PACKAGE.\n\nYou will need to update your checksums for all files in /boot." | ||
| TEXT+="$UPDATE_TEXT" | ||
| else | ||
| TEXT="The following files failed the verification process AFTER package updates ran:\n${CHANGED_FILES}\n\nThis might be due to the following package updates:\n$LAST_PACKAGE_LIST.\n\nYou will need to update your checksums for all files in /boot." | ||
| TEXT+="$UPDATE_TEXT" | ||
| fi | ||
| else |
There was a problem hiding this comment.
This logic isn't new (moved from gui-init), but while we're here....
This looks like logic to allow an OS package manager to place files on /boot indicating:
- what files failed validation before updating packages
- what files failed validation after updating packages
- what packages were updated
This then controls the prompts shown by Heads if files do not validate. But kexec_package_trigger_{pre/post}.txt aren't themselves authenticated in any way (which would have to somehow "authenticate" that your package manager really made these changes). So as far as I can tell, somebody tampering /boot could just place these files here with whatever content they want and thereby control what Heads will say when it observes the tampering.
Does this package manager support exist in any OS?
I noted some additional details from my read-through here, but really the key question to me is - why should we trust these kexec_package_trigger_* files?
| whiptail $MAIN_MENU_BG_COLOR --title "Measured Integrity Report" --msgbox "$date\n\nTOTP: $TOTP\nHOTP: $HOTP\nBOOT INTEGRITY SIGNATURE: $SIGNED\nBOOT INTEGRITY HASHES: $HASH\n\nPress OK to continue with detailed information in case of errors" 0 80 | ||
|
|
There was a problem hiding this comment.
IMO:
- All the CAPS introduced in the error text, as well as the much longer labels for integrity, make this difficult to read. There's no longer any separation between the "labels" and "content" for errors (screenshot below)
- I think the separation between "signature" and "hashes" should be an implementation detail of Heads - that's how we authenticate /boot. The normal user flow should just say whether /boot integrity was authenticated or not, so it's easier to understand if you are not an expert in Heads. (I'd be OK with providing detailed information as an optional thing or on the console, etc. to cater to advanced users too, but putting it front-and-center IMO makes it a requirement to be an knowledgeable in Heads internals to use Heads.)
Re: the caps, this is what I was greeted with when firing this up in qemu:
From a glance it is really hard to parse this and know what to look at. The text-only prompt is a bit limiting but I'd suggest:
- Don't use all caps for the errors, this doesn't add anything and just makes it harder to read
- Go back to just "INTEGRITY: " instead of the last two lines, which would make the labels all similar-length again and reduce a lot of verbosity
- Take out the colon from the HOTP error so the colons form a bit of separation between the labels and contents
There was a problem hiding this comment.
Oh and also, it did not show me any detailed information when I pressed Enter :-/
|
Thinking back to the actual intent of the change - why do you want to report /boot integrity when TOTP/HOTP unseal fails? Seems like this is adding another prompt in an already nontrivial flow, and I think it'd be hard for users who aren't experts in Heads to understand what this information has to do with the issue at hand.
TOTP/HOTP re-seal doesn't affect the integrity of /boot. Why would you need to sign /boot without knowing the prior status in this case? |
|
The intent of this was once again for tpm disk unlock key specifically but more globally for users to not sign /boot content when totp unsealing fails and resealing totp. When one updates totp, tpm Disk Unlock Key needs to be updated (which takes totp + runtime measurements into another sealed secret which is authenticated) , but doing so, LUKS header changes and its local hash kept under /boot changes which is signed under kexec.sig and needs to be updated. On tpm2, primary handle needs to be updated as well. As for pre trigger and post trigger files under /boot, I will have to check history to see which os populates those and if they are still relevant. I do not use any OS doing those but thought probably wrongly that pureos was the intent of support here. Will check later but as of now I think the unification of those functions will need more thinking. |
Raw notes:
What should be improved further?