From 73b19166e0d20453f2bae64c31e972d4eecc2deb Mon Sep 17 00:00:00 2001 From: Thierry Laurion Date: Tue, 5 May 2026 19:16:53 -0400 Subject: [PATCH] Bugfix: Fix TPM auth retry, counter error handling, and NV error surfacing This commit fixes regressions introduced by PR #2068, merged to origin/master on 2026-04-07. Regressions fixed (present in origin/master post-PR #2068): - No "out of resources" (0x15) TPM counter error detection - TPM2 counter increment had no auth retry on wrong passphrase - TPM1 counter increment had no error handling - tpm1_seal silenced NV define/write errors - Duplicate TPM1/TPM2 retry loops (~100 lines of redundant code) - counter_present dead code (now fixed with counter_read check) - Comment mismatch (stdout vs console) now fixed - set -e issue in check_tpm_counter (wrapped in subshell) Fixes implemented: - Add shared _tpm_auth_retry helper for TPM1/TPM2 - check_tpm_counter only triggers tpm_reset_required on 0x15 errors - tpm1_seal surfaces NV errors with retry loop - Simplify reset_tpm to verify tpmr.sh reset exit code Copilot review fixes: - Fix counter_present dead code: add counter_read check - Fix doc references: tpmr -> tpmr.sh for consistency - Fix comment at line 1901: stdout goes to /dev/null via tee - Wrap tpmr.sh counter_create in subshell for set -e compatibility Signed-off-by: Thierry Laurion --- initrd/bin/gui-init.sh | 37 ++- initrd/bin/tpmr.sh | 433 +++++++++++++++++++++++++----------- initrd/etc/functions.sh | 74 +++--- initrd/etc/gui_functions.sh | 8 + 4 files changed, 379 insertions(+), 173 deletions(-) diff --git a/initrd/bin/gui-init.sh b/initrd/bin/gui-init.sh index 7c7164e7b..7ed3f5834 100755 --- a/initrd/bin/gui-init.sh +++ b/initrd/bin/gui-init.sh @@ -191,11 +191,19 @@ prompt_update_checksums() { --yesno "You have chosen to update the checksums and sign all of the files in /boot.\n\nThis means that you trust that these files have not been tampered with.\n\nYou will need your GPG key available, and this change will modify your disk.\n\nDo you want to continue?" 0 80); then if update_checksums; then return 0 + fi + # update_checksums may have set the TPM-reset-required marker + # during its execution (e.g. check_tpm_counter hit "out of + # resources"). Show the targeted TPM message instead of the + # generic failure so the user knows exactly what to do. + if tpm_reset_required; then + whiptail_error --title 'TPM Reset Required' \ + --msgbox "Cannot sign /boot: TPM state is inconsistent.\n\nReset the TPM first (Options -> TPM/TOTP/HOTP Options -> Reset the TPM), then update checksums." 0 80 else whiptail_error --title 'ERROR' \ --msgbox "Failed to update checksums / sign default config" 0 80 - return 1 fi + return 1 fi return 1 } @@ -411,8 +419,13 @@ EOF skip_to_menu="true" return 1 ;; + # "Reset the TPM" from the TOTP failure whiptail menu. + # The gate runs first to verify /boot integrity. If the gate + # fails *because* TPM reset is required (e.g. stale counters), + # the || tpm_reset_required bypass lets reset_tpm() proceed — + # it clears counters and creates a fresh one. p) - if gate_reseal_with_integrity_report && reset_tpm && update_totp && BG_COLOR_MAIN_MENU="normal"; then + if { gate_reseal_with_integrity_report || tpm_reset_required; } && reset_tpm && update_totp && BG_COLOR_MAIN_MENU="normal"; then reseal_tpm_disk_decryption_key || prompt_missing_gpg_key_action fi ;; @@ -806,8 +819,11 @@ show_tpm_totp_hotp_options_menu() { update_totp && update_hotp || true fi ;; + # "Reset the TPM" from the TPM/TOTP/HOTP options whiptail menu. + # Same gate-bypass pattern: if the gate fails because TPM + # reset is required, proceed to reset_tpm() anyway. r) - if gate_reseal_with_integrity_report && reset_tpm; then + if { gate_reseal_with_integrity_report || tpm_reset_required; } && reset_tpm; then reseal_tpm_disk_decryption_key || prompt_missing_gpg_key_action fi ;; @@ -837,7 +853,20 @@ reset_tpm() { return 1 fi - tpmr.sh reset "$tpm_owner_passphrase" + # Verify TPM reset succeeded before proceeding to counter + # creation, signing, TOTP generation, and DUK resealing. + # A failed reset would leave the TPM in an inconsistent state + # (old passphrase with unknown PCRs), causing confusing errors + # downstream. Show the actual error to the user and return + # to the menu. + local reset_err_file=$(mktemp) + if ! tpmr.sh reset "$tpm_owner_passphrase" >"$reset_err_file" 2>&1; then + ERROR=$(tail -n 1 "$reset_err_file" | fold -s) + rm -f "$reset_err_file" + whiptail_error --title 'ERROR' \ + --msgbox "Error resetting TPM:\n\n${ERROR}" 0 80 + return 1 + fi # now that the TPM is reset, remove invalid TPM counter files mount_boot diff --git a/initrd/bin/tpmr.sh b/initrd/bin/tpmr.sh index 264a15ffa..9dd7609a3 100755 --- a/initrd/bin/tpmr.sh +++ b/initrd/bin/tpmr.sh @@ -1,5 +1,17 @@ #!/bin/bash # TPM Wrapper - to unify tpm and tpm2 subcommands +# +# NOTE: tpmtotp C code (counter_create.c, unsealfile.c, sealfile2.c, etc.) +# prints ALL output (both success and error messages) via printf() to stdout, +# NOT stderr. This is a quirk of the tpmtotp toolkit. When capturing +# output or errors from tpm commands, we must capture stdout (not just stderr). +# +# TPM2 error codes caught by auth-retry grep patterns. +# Reference: TCG TPM2 Part 2 (Structures), Table 18 — TPM_RC (Response Codes) +# https://trustedcomputinggroup.org/resource/tpm-library-specification/ +# 0x98e TPM_RC_AUTH_FAIL — wrong password, retry allowed +# 0x149 TPM_RC_AUTH_UNAVAILABLE — auth handle wrong for entity (e.g. owner +# hierarchy auth vs. NV index auth with authwrite attribute) . /etc/functions.sh @@ -279,10 +291,21 @@ tpm2_counter_read() { echo "$index: $hex_val" } +# tpm2_counter_inc - Increment a TPM2 counter. +# +# Auth behaviour: +# -pwdc "" : try bare nvincrement (index auth — counters created by +# nvdefine have empty NV index auth). On failure, prompt for +# owner passphrase and retry with owner auth up to 3 times. +# -pwdc : use owner auth — retry up to 3 times on auth failure, +# re-prompting passphrase. +# (no -pwdc) : try bare nvincrement first, then prompt and retry as above. +# +# Retry is consistent with tpm2_seal and tpm2_counter_create which also +# re-prompt and retry on authorization failures. tpm2_counter_inc() { TRACE_FUNC - local index pwd - local inc_args=() + local index pass="" while true; do case "$1" in -ix) @@ -290,7 +313,7 @@ tpm2_counter_inc() { shift 2 ;; -pwdc) - pwd="$2" + pass="$2" shift 2 ;; *) @@ -298,100 +321,157 @@ tpm2_counter_inc() { ;; esac done - if [ -n "$pwd" ]; then - inc_args=(-C o -P "$(tpm2_password_hex "$pwd")") + # Try bare nvincrement first (index auth, no owner passphrase needed). + # This is a pre-check before the owner-auth retry loop and is not + # counted against the 3-attempt budget below. + if tpm2 nvincrement "0x$index" 2>/dev/null >/dev/console; then + local hex_val + hex_val="$(tpm2 nvread 0x$index | xxd -pc8)" || return 1 + echo "$index: $hex_val" + return 0 fi - tpm2 nvincrement "${inc_args[@]}" "0x$index" >/dev/console || return 1 - local hex_val - hex_val="$(tpm2 nvread 0x"$index" | xxd -pc8)" || return 1 - echo "$index: $hex_val" -} -tpm1_counter_create() { - TRACE_FUNC + # Retry with owner auth (up to 3 attempts total). + # If -pwdc was provided, it's cached as tpm_owner_passphrase; + # otherwise prompt_tpm_owner_password will prompt and cache. local attempt=0 - while true; do + while [ $attempt -lt 3 ]; do attempt=$((attempt + 1)) - prompt_tpm_owner_password - tpm_owner_passphrase="$(cat /tmp/secret/tpm_owner_passphrase)" - TMP_ERR_FILE=$(mktemp) - if tpm counter_create -pwdo "$tpm_owner_passphrase" "$@" 2>"$TMP_ERR_FILE"; then - rm -f "$TMP_ERR_FILE" + if [ -n "$pass" ]; then + tpm_owner_passphrase="$pass" + else + prompt_tpm_owner_password + fi + local tmp_err_file=$(mktemp) + if tpm2 nvincrement -C o -P "$(tpm2_password_hex "$tpm_owner_passphrase")" "0x$index" 2>"$tmp_err_file" >/dev/console; then + rm -f "$tmp_err_file" + local hex_val + hex_val="$(tpm2 nvread 0x$index | xxd -pc8)" || return 1 + echo "$index: $hex_val" return 0 fi - tmp_err_content="$(cat "$TMP_ERR_FILE")" - rm -f "$TMP_ERR_FILE" - DEBUG "Failed attempt $attempt to create counter from tpm1_counter_create. Stderr: $tmp_err_content" - shred -n 10 -z -u /tmp/secret/tpm_owner_passphrase - if echo "$tmp_err_content" | grep -qiE 'authorization|auth|bad|permission'; then - if [ "$attempt" -ge 3 ]; then - DIE "Unable to create counter from tpm1_counter_create after 3 attempts. Please reset the TPM using the GUI menu (Options -> TPM/TOTP/HOTP Options -> Reset the TPM) and try again." - fi - WARN "Counter creation failed (bad passphrase?). Retrying..." - else - DIE "Unable to create counter from tpm1_counter_create. Please reset the TPM using the GUI menu (Options -> TPM/TOTP/HOTP Options -> Reset the TPM) and try again." + local tmp_err_content="$(cat "$tmp_err_file")" + rm -f "$tmp_err_file" + shred -n 10 -z -u /tmp/secret/tpm_owner_passphrase 2>/dev/null || true + DEBUG "tpm2_counter_inc attempt $attempt failed. Stderr: $tmp_err_content" + if ! echo "$tmp_err_content" | grep -qiE 'authorization|auth|bad|permission|0x98e|0x149'; then + DIE "Can't increment TPM counter for $index, access denied." fi + WARN "Authentication failed, retrying..." done + DIE "Can't increment TPM counter for $index after 3 attempts, access denied." } -tpm2_counter_create() { - TRACE_FUNC - pass="" # owner passphrase from argument - label="" # label argument - while true; do - case "$1" in - -pwdc) - pass="$2" - shift 2 - ;; - -la) - label="$2" - shift 2 - ;; - *) - break - ;; - esac - done - - rand_index="1$(dd if=/dev/urandom bs=1 count=3 2>/dev/null | xxd -pc3)" +# _tpm_auth_retry - Shared retry helper for TPM commands needing owner auth. +# +# Handles both TPM1 (tpmtotp: errors to stdout, uses -pwdo/-pwdc flags) +# and TPM2 (tpm2-tools: errors to stderr, uses -P parameter). +# +# Caching: prompt_tpm_owner_password reuses cached passphrase if available. +# On auth failure the cache is shredded; next prompt will ask the user. +# +# Usage: _tpm_auth_retry