From 465e1f5df50464e84941ccfa1f9fa7883738e0b1 Mon Sep 17 00:00:00 2001 From: Thierry Laurion Date: Wed, 20 May 2026 10:13:02 -0400 Subject: [PATCH] initrd: fix TPM1 counter auth regression (#2068) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #2068 introduced a regression where `increment_tpm_counter` was changed from hardcoded `-pwdc ''` (empty counter auth per TCG spec) to `-pwdc "${tpm_passphrase:-}"` (owner passphrase), while counters continued to be created with `-pwdc ''`. This caused every increment to compute SHA1(owner_pass) against a counter created with SHA1(""), producing persistent TPM_AUTHFAIL. Per TCG TPM Main Spec Part 3, TPM_CreateCounter uses owner auth (-pwdo) but TPM_IncrementCounter uses the counter's own authData — not the owner password. The correct design for Heads' rollback counter is empty auth. The repeated auth failures (3 per boot) accumulated the TPM's dictionary attack (DA) failedTries counter until lockout was reached (~10 boots = 30 failures). Users reported "hours of waiting" on affected hardware. On some implementations the DA state persisted through tpm forceclear. Fix: restore TCG-compliant empty counter auth: - tpm1_counter_increment: detect explicit -pwdc '' and call tpm directly, bypassing _tpm_auth_retry. Non-empty or absent -pwdc falls through to owner-auth retry path for migration of counters created by pre-fix code. - check_tpm_counter: create counters with -pwdc '' instead of owner passphrase. - increment_tpm_counter: increment with -pwdc '' instead of owner passphrase; counter_create fallback uses empty auth. - oem-factory-reset.sh: create counters with -pwdc ''. Signed-off-by: Thierry Laurion --- doc/tpm.md | 31 +++++++++++++++++++++++++++ initrd/bin/oem-factory-reset.sh | 2 +- initrd/bin/tpmr.sh | 25 +++++++++++++++++++--- initrd/etc/functions.sh | 37 +++++++++++---------------------- 4 files changed, 66 insertions(+), 29 deletions(-) diff --git a/doc/tpm.md b/doc/tpm.md index 90f7ec064..0b8709fac 100644 --- a/doc/tpm.md +++ b/doc/tpm.md @@ -330,6 +330,37 @@ counter passes preflight or the user chooses to continue. exit status of `tee` (always 0), not `tpmr.sh`. See [ux-patterns.md](ux-patterns.md#tpm-counter-patterns) for the correct pattern. +### TPM1 counter auth + +TPM1 rollback counters use **empty auth** (`SHA1("")`) per the TCG +specification (TPM Main Spec Part 3: `TPM_CreateCounter` uses owner +auth `-pwdo`, but `TPM_IncrementCounter` uses the counter's own +authData — not the owner password). + +A regression in PR #2068 changed `increment_tpm_counter` from +hardcoded `-pwdc ''` to `-pwdc "${tpm_passphrase:-}"` (owner +passphrase), while counters continued to be created with `-pwdc ''`. +This caused every increment to compute SHA1(owner_pass) against a +counter created with SHA1(""), producing persistent `TPM_AUTHFAIL`. + +The repeated auth failures (3 per boot) accumulated the TPM's +dictionary attack (DA) failedTries counter until lockout was reached +(~10 boots = 30 failures). Users reported "hours of waiting" on +affected hardware. On some implementations, the DA state persisted +through `tpm forceclear`. + +The fix (PR #2117): +- `tpm1_counter_increment`: detect explicit `-pwdc ''` and call + `tpm counter_increment` directly, bypassing `_tpm_auth_retry`. + Non-empty `-pwdc` or absent flag falls through to the owner-auth + retry path for migration of counters created by pre-fix code. +- `check_tpm_counter` and `increment_tpm_counter`: create and + increment counters with `-pwdc ''` instead of owner passphrase. +- `oem-factory-reset.sh`: uses `-pwdc ''` for counter creation. + +TPM2 counters are unaffected — they still require owner auth for +`tpm2 nvincrement`. + --- ## TPM secret sealing internals (TPM2) diff --git a/initrd/bin/oem-factory-reset.sh b/initrd/bin/oem-factory-reset.sh index ece2f8f59..5dc315965 100755 --- a/initrd/bin/oem-factory-reset.sh +++ b/initrd/bin/oem-factory-reset.sh @@ -868,7 +868,7 @@ generate_checksums() { if [ "$CONFIG_TPM" = "y" ]; then if [ "$CONFIG_IGNORE_ROLLBACK" != "y" ]; then tpmr.sh counter_create \ - -pwdc "${TPM_PASS:-}" \ + -pwdc '' \ -la -3135106223 | tee /tmp/counter >/dev/null 2>&1 || whiptail_error_die "Unable to create TPM counter" diff --git a/initrd/bin/tpmr.sh b/initrd/bin/tpmr.sh index 46f4581d8..fec10c105 100755 --- a/initrd/bin/tpmr.sh +++ b/initrd/bin/tpmr.sh @@ -429,7 +429,8 @@ _tpm_auth_retry() { } # tpm1_counter_create - Create a TPM1 rollback counter. -# Delegates to _tpm_auth_retry for passphrase-driven retry on auth failure. +# Owner passphrase (via _tpm_auth_retry -pwdo) is required for NV_DefineSpace. +# Callers pass -pwdc '' for empty counter auth per TCG spec. tpm1_counter_create() { TRACE_FUNC _tpm_auth_retry "counter_create" "stdout" "tpm1" "-pwdo" tpm counter_create "$@" @@ -443,17 +444,35 @@ tpm1_counter_read() { # tpm1_counter_increment - Increment a TPM1 rollback counter. # Args: -ix [ -pwdc ] +# +# Auth behaviour: +# -pwdc '' : empty counter auth (SHA1 of ""), correct per TCG spec. +# Calls tpm directly without retry — caller handles fallback. +# -pwdc : owner passphrase auth via _tpm_auth_retry (migration). +# (no -pwdc) : owner passphrase auth via _tpm_auth_retry (backward compat). tpm1_counter_increment() { TRACE_FUNC local counter_id="" + local pwdc_provided="n" + local pwdc_value="" while [ $# -gt 0 ]; do case "$1" in -ix) counter_id="$2"; shift 2 ;; - -pwdc) shift 2 ;; # passphrase handled by _tpm_auth_retry + -pwdc) pwdc_provided="y"; pwdc_value="$2"; shift 2 ;; *) shift ;; esac done - _tpm_auth_retry "counter_increment" "stdout" "tpm1" "-pwdc" tpm counter_increment -ix "$counter_id" + if [ "$pwdc_provided" = "y" ] && [ -z "$pwdc_value" ]; then + # Empty counter auth per TCG spec: this is the normal counter + # increment (no secret). Bypass _tpm_auth_retry because the + # empty string is intentional, not a user input error. + # Use || return so set -e doesn't kill the script on DA failure. + DEBUG "tpm1_counter_increment: empty auth, calling tpm directly" + tpm counter_increment -ix "$counter_id" -pwdc '' || return $? + else + _tpm_auth_retry "counter_increment" "stdout" "tpm1" "-pwdc" \ + tpm counter_increment -ix "$counter_id" + fi } tpm2_counter_create() { diff --git a/initrd/etc/functions.sh b/initrd/etc/functions.sh index f9b0694fd..b3188563d 100644 --- a/initrd/etc/functions.sh +++ b/initrd/etc/functions.sh @@ -1848,7 +1848,8 @@ check_tpm_counter() { TRACE_FUNC LABEL=${2:-3135106223} - tpm_passphrase="$3" + # $3 (tpm_passphrase) was used by pre-PR #2068 code but is now intentionally + # ignored — counters are created with empty auth (-pwdc '') per TCG spec. # if the /boot.hashes file already exists, read the TPM counter ID # from it. if [ -r "$1" ]; then @@ -1857,12 +1858,8 @@ check_tpm_counter() { DEBUG "Extracted TPM_COUNTER: '$TPM_COUNTER' from $1" else DEBUG "$1 does not exist - creating new TPM counter" - # Warn user: TPM Owner Passphrase is required to create a new TPM counter - if [ ! -s /tmp/secret/tpm_owner_passphrase ]; then - WARN "TPM Owner Passphrase is required to create a new TPM counter for /boot content rollback prevention" - fi - - # attempt to make a new counter, capturing any stderr for debugging + # Create TPM counter with empty counter auth per TCG spec (no secret). + # Owner passphrase is not needed for the counter auth itself. DEBUG "Invoking tpmr.sh counter_create with label $LABEL" # run it, then record the exit status explicitly; the '!' operator # cannot be used because it would hide the real return code. @@ -1872,7 +1869,7 @@ check_tpm_counter() { ( set +e tpmr.sh counter_create \ - -pwdc "${tpm_passphrase:-}" \ + -pwdc '' \ -la "$LABEL" \ >/tmp/counter 2> >(tee >(SINK_LOG "tpm counter_create stderr") >&2) echo $? > /tmp/counter_create_rc @@ -2050,22 +2047,11 @@ increment_tpm_counter() { counter_present="y" fi - # Prefer explicit passphrase, otherwise reuse cached TPM owner passphrase. + # TPM2 uses owner-auth fallback in tpm2_counter_inc; TPM1 uses empty counter + # auth (SHA1("")) per TCG spec — no owner passphrase needed for increment. + # Keep the cached owner passphrase for TPM2 fallback. if [ -z "$tpm_passphrase" ] && [ -s /tmp/secret/tpm_owner_passphrase ]; then tpm_passphrase="$(cat /tmp/secret/tpm_owner_passphrase)" - DEBUG "increment_tpm_counter: using cached TPM owner passphrase" - fi - - # TPM1 counter_increment requires owner auth in practice on this path. - # origin/master typically reached this with cached owner passphrase already set, - # but the newer reseal/update flows can call this later in the session after - # that cache is absent. Prompt once and cache to avoid empty -pwdc failures. - if [ "$CONFIG_TPM2_TOOLS" != "y" ] && [ -z "$tpm_passphrase" ]; then - WARN "TPM Owner Passphrase is required to update rollback counter before signing updated boot hashes." - DEBUG "increment_tpm_counter: TPM1 path has no cached/provided owner passphrase; prompting now" - prompt_tpm_owner_password - tpm_passphrase="$tpm_owner_passphrase" - DEBUG "increment_tpm_counter: TPM1 owner passphrase obtained and cached" fi # Try to increment the counter. We normally hide the verbose @@ -2094,7 +2080,7 @@ increment_tpm_counter() { increment_ok="y" fi else - # TPM1 path uses owner auth in practice. + # TPM1 counter uses empty auth (SHA1 of "") per TCG spec. # NOTE: tpmtotp C code prints ALL output (success + errors) to stdout. # We must capture stdout to detect failures properly. # DO_WITH_DEBUG internally captures the command's stderr (tee /dev/stderr @@ -2104,7 +2090,7 @@ increment_tpm_counter() { if ( set -o pipefail DO_WITH_DEBUG --mask-position 5 \ - tpmr.sh counter_increment -ix "$counter_id" -pwdc "${tpm_passphrase:-}" \ + tpmr.sh counter_increment -ix "$counter_id" -pwdc '' \ 2>/dev/null | tee /tmp/counter-"$counter_id" >/dev/null ); then increment_ok="y" @@ -2123,10 +2109,11 @@ increment_tpm_counter() { # run counter_create but tee its stdout to a file so we still see # the interactive prompt and any informational messages. + # Empty counter auth (-pwdc '') per TCG spec. if ( set -o pipefail DO_WITH_DEBUG --mask-position 3 \ - tpmr.sh counter_create -pwdc "${tpm_passphrase:-}" -la 3135106223 \ + tpmr.sh counter_create -pwdc '' -la 3135106223 \ 2> >(tee >(SINK_LOG "tpm counter_create stderr") >&2) | tee /tmp/new-counter >/dev/null ); then