Improve error report when PCR changes (#43579)

When PCR is extended during a TPM2 operation, the internal counter of the TPM2 will detect a new extension. systemd already retry this operation several times, but sometimes the report will be an error, and will refer to the wrong PCR. This situation happends when IMA is enabled (when secure boot is active, for example). PCR 10 will receive an extension for each new kernel module that is loaded, logging in the journal errors that are recovered later after a subsequent retry.
2026-09-23 09:28:35 +02:00
parent 0167e36e74 827099139b
commit 1c168322cf
9 changed files with 119 additions and 23 deletions
@@ -589,6 +589,8 @@ int enroll_tpm2(const EnrollContext *c,
                                n_policy_hash,
                                &srk,
                                &secret2);
+                if (r == -EUCLEAN)
+                        return log_error_errno(r, "PCR values kept changing while unsealing the TPM2 secret, giving up. Something on this system extends a PCR continuously.");
                if (r < 0)
                        return log_error_errno(r, "Failed to unseal secret using TPM2: %m");

@@ -135,6 +135,8 @@ int acquire_luks2_key(
                return log_error_errno(SYNTHETIC_ERRNO(ENOANO), "Bad %s.", argon2id ? "password" : "PIN");
        if (r == -ENOLCK)
                return log_error_errno(r, "TPM is in dictionary attack lock-out mode.");
+        if (r == -EUCLEAN)
+                return log_error_errno(r, "PCR values kept changing while unsealing the TPM2 secret, giving up. Something on this system extends a PCR continuously.");
        if (ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r)) {
                log_warning_errno(r, "TPM policy does not match current system state. Either system has been tampered with or policy out-of-date: %m");
                /* Normalize to -EPERM so callers don't confuse it with -ENOANO's "needs PIN" meaning. */
@@ -124,6 +124,10 @@ COMMAND(
 #define PCRLOCK_FILE_SYSTEM_PATH_PREFIX     "/var/lib/pcrlock.d/840-file-system-"
 #define PCRLOCK_PE_INPUT_MAX                (2U * U64_GB)

+/* Maximum number of retries when PCR values keep changing while we submit the policy for, or unseal, the
+ * recovery PIN */
+#define PCRLOCK_RETRY_UNSEAL_MAX            16U
+
 /* The default set of PCRs to lock to */
 #define DEFAULT_PCR_MASK                                     \
        ((UINT32_C(1) << TPM2_PCR_PLATFORM_CODE) |           \
@@ -3849,6 +3853,16 @@ static int make_policy(bool force, RecoveryPinMode recovery_pin_mode) {
                                        policy_session,
                                        &old_policy.prediction,
                                        old_policy.algorithm);
+                        if (r == -EUCLEAN) {
+                                if (attempt >= PCRLOCK_RETRY_UNSEAL_MAX)
+                                        return log_error_errno(r,
+                                                               "PCR values kept changing while submitting the policy, giving up after %u attempts. "
+                                                               "Something on this system extends a PCR continuously.",
+                                                               attempt + 1);
+
+                                log_debug("Trying again (attempt %u), as PCR values changed while submitting the policy.", attempt + 1);
+                                continue;
+                        }
                        if (r < 0)
                                return r;

@@ -3868,12 +3882,12 @@ static int make_policy(bool force, RecoveryPinMode recovery_pin_mode) {
                                        policy_session,
                                        encryption_session,
                                        &secret);
-                        if (r < 0 && (r != -ESTALE || attempt >= 16))
+                        if (r < 0 && (r != -ESTALE || attempt >= PCRLOCK_RETRY_UNSEAL_MAX))
                                return log_error_errno(r, "Failed to unseal PIN: %m");
                        if (r == 0)
                                break;

-                        log_debug("Trying again (attempt %u), as PCR values changed during unlock attempt.", attempt+1);
+                        log_debug("Trying again (attempt %u), as PCR values changed during unlock attempt.", attempt + 1);
                }

                if (secret.iov_len > sizeof_field(TPM2B_AUTH, buffer))
@@ -1362,7 +1362,7 @@ int decrypt_credential_and_warn(

        /* Relevant error codes:
         *
-         *   -EBADMSG      → Corrupted file
+         *   -EBADMSG      → Corrupted file, or unparsable PCR signature file
         *   -EOPNOTSUPP   → Unsupported file type (could be: requires TPM but we have no TPM)
         *   -EHOSTDOWN    → Need PCR signature file, but couldn't find it
         *   -EHWPOISON    → Attempt to unlock with NULL key and either CREDENTIAL_ALLOW_REFUSE is on, or CREDENTIAL_ALLOW_NULL is off, but the system has a TPM and SecureBoot is on
@@ -1386,6 +1386,8 @@ int decrypt_credential_and_warn(
                r = tpm2_load_pcr_signature(tpm2_signature_path, &signature_json);
                if (r == -ENOENT)
                        return log_error_errno(SYNTHETIC_ERRNO(EHOSTDOWN), "Couldn't find PCR signature file: %m");
+                if (r == -EBADMSG)
+                        return log_error_errno(r, "PCR signature file is malformed.");
                if (r < 0)
                        return log_error_errno(r, "Failed to load PCR signature: %m");
        }
@@ -1534,6 +1536,8 @@ int decrypt_credential_and_warn(
                        return log_error_errno(r, "TPM key integrity check failed. Key most likely does not belong to this TPM.");
                if (r == -EADDRNOTAVAIL)
                        return log_error_errno(r, "NV index referenced by key is missing, unwritten, or unusable, it could be for another system.");
+                if (r == -EUCLEAN)
+                        return log_error_errno(r, "PCR values kept changing while unsealing the TPM2 secret, giving up. Something on this system extends a PCR continuously.");
                if (ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r))
                        return log_error_errno(r, "TPM policy does not match current system state. Either system has been tampered with or policy out-of-date: %m");
                if (r < 0)
@@ -1985,7 +1989,8 @@ static const CredentialsVarlinkError credentials_varlink_error_table[] = {
        { "io.systemd.Credentials.NullKeyNotAllowed",      EHWPOISON,    "The key was encrypted with a null key, but that's not allowed during decryption." },
        { "io.systemd.Credentials.KeyBelongsToOtherTPM",   EREMOTE,      "The TPM integrity check for this key failed, key probably belongs to another TPM, or was corrupted." },
        { "io.systemd.Credentials.TPMInDictionaryLockout", ENOLCK,       "The TPM is in dictionary lockout mode, cannot operate." },
-        { "io.systemd.Credentials.UnexpectedPCRState" ,    EUCLEAN,      "Unexpected TPM PCR state of the system." },
+        { "io.systemd.Credentials.UnexpectedPCRState",     EPERM,        "Unexpected TPM PCR state of the system." },
+        { "io.systemd.Credentials.PCRsKeptChanging",       EUCLEAN,      "PCR values kept changing while unsealing the TPM2 secret, something on this system extends a PCR continuously." },
        { "io.systemd.Credentials.NVIndexUnusable",        EADDRNOTAVAIL, "The NV index referenced by the key is missing, unwritten, or unusable, it could be for another system." },
 };

@@ -177,6 +177,8 @@ int acquire_tpm2_key(
                        return log_warning_errno(r, "TPM key integrity check failed. Key enrolled in superblock most likely does not belong to this TPM.");
                if (r == -EADDRNOTAVAIL)
                        return log_warning_errno(r, "NV index referenced by token is missing, unwritten, or unusable, it could be for another system.");
+                if (r == -EUCLEAN)
+                        return log_error_errno(r, "PCR values kept changing while unsealing the TPM2 secret, giving up. Something on this system extends a PCR continuously.");
                if (ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r)) {
                        log_warning_errno(r, "TPM policy does not match current system state. Either system has been tampered with or policy out-of-date: %m");
                        /* Normalize to -EPERM so callers don't confuse it with -ENOANO's "needs PIN" meaning. */
@@ -257,6 +259,8 @@ int acquire_tpm2_key(
                        return log_warning_errno(r, "TPM key integrity check failed. Key enrolled in superblock most likely does not belong to this TPM.");
                if (r == -EADDRNOTAVAIL)
                        return log_warning_errno(r, "NV index referenced by token is missing, unwritten, or unusable, it could be for another system.");
+                if (r == -EUCLEAN)
+                        return log_error_errno(r, "PCR values kept changing while unsealing the TPM2 secret, giving up. Something on this system extends a PCR continuously.");
                if (ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r)) {
                        log_warning_errno(r, "TPM policy does not match current system state. Either system has been tampered with or policy out-of-date: %m");
                        /* Normalize to -EPERM so callers don't confuse it with -ENOANO's "needs PIN" meaning. */
@@ -7196,13 +7196,17 @@ int tpm2_unseal(Tpm2Context *c,
         *   -ENOLCK          → TPM is in dictionary lockout mode
         *   -EREMCHG         → submitted policy doesn't match NV index stored policy (in case of PolicyAuthorizeNV)
         *   -ENOANO          → none of the PolicyOR branches of a policy matched current state
-         *   -EUCLEAN         → PCR state doesn't match expectations
+         *   -EUCLEAN         → a PCR kept being extended while we were unsealing
         *   -EPERM           → stored policy does not match TPM state
         *   -ENOTRECOVERABLE → all other kinds of TPM errors
         *   -EILSEQ          → bad PIN
         *
-         * Of these all four of EREMCHG, ENOANO, EUCLEAN, EPERM can all mean that PCR state is not matching
-         * expectations. */
+         * Of these EREMCHG, ENOANO and EPERM mean that PCR state is not matching expectations. EUCLEAN on
+         * the other hand says nothing about the correctness of the policy: it means the TPM's global PCR
+         * update counter kept moving while we built the policy or unsealed, i.e. something on this system
+         * extends a PCR continuously (the usual suspect is PCR 10 when IMA is enabled directly, or
+         * indirectly via secure boot). We make at most RETRY_UNSEAL_MAX + 1 attempts before reporting
+         * it. */

        TSS2_RC rc;
        int r;
@@ -7360,7 +7364,10 @@ int tpm2_unseal(Tpm2Context *c,
                                        !!pin,
                                        (shard == 1 || !iovec_is_set(pubkey)) ? pcrlock_policy : NULL,
                                        &policy_digest);
-                        if (r == -EUCLEAN && i > 0) {
+                        if (r == -EUCLEAN) {
+                                if (i == 0)
+                                        return log_debug_errno(r, "PCR values kept changing while building the TPM2 policy, giving up after %u unsealing attempts.", RETRY_UNSEAL_MAX + 1);
+
                                log_debug("A PCR value changed during the TPM2 policy session, restarting HMAC key unsealing (%u tries left).", i);
                                retry = true;
                                break;
@@ -7402,7 +7409,11 @@ int tpm2_unseal(Tpm2Context *c,
                                        encryption_session->esys_handle, /* use HMAC session to enable parameter encryption */
                                        ESYS_TR_NONE,
                                        &unsealed);
-                        if (rc == TPM2_RC_PCR_CHANGED && i > 0) {
+                        if (rc == TPM2_RC_PCR_CHANGED) {
+                                if (i == 0)
+                                        return log_debug_errno(SYNTHETIC_ERRNO(EUCLEAN),
+                                                               "PCR values kept changing while unsealing, giving up after %u unsealing attempts.", RETRY_UNSEAL_MAX + 1);
+
                                log_debug("A PCR value changed during the TPM2 policy session, restarting HMAC key unsealing (%u tries left).", i);
                                retry = true;
                                break;
@@ -9284,6 +9295,13 @@ int tpm2_nvpcr_initialize(
                                signature_json,
                                &authorize_policy,
                                &policy_session);
+                if (r == -EUCLEAN) {
+                        /* A PCR was extended while we submitted the policy, so this session is unusable.
+                         * Same situation as below, just observed while building the policy rather than
+                         * while writing. */
+                        if (i > 0)
+                                continue;
+                }
                if (r < 0)
                        return r;

@@ -9824,15 +9842,10 @@ int tpm2_policy_super_pcr(
                                session,
                                &pcr_selection,
                                &current_policy_digest);
-                if (r == -EUCLEAN) {
-                        _cleanup_free_ char *j = NULL;
-
-                        for (uint32_t pcr = 0; pcr < TPM2_PCRS_MAX; pcr++)
-                                if (single_value_pcrs & (UINT32_C(1) << pcr))
-                                        (void) strextendf_with_separator(&j, ", ", "%" PRIu32, pcr);
-
-                        return log_error_errno(r, "Combined value for PCR(s) %s encoded in policy does not match the current TPM state. Either the system has been tampered with or the provided policy is incorrect.", strna(j));
-                }
+                /* -EUCLEAN means some PCR got extended while we were submitting the policy. That's
+                 * transient, so only log at debug level and let the caller restart the session. */
+                if (r == -EUCLEAN)
+                        return log_debug_errno(r, "PCR state changed while submitting the combined PolicyPCR expression, session needs to be restarted.");
                if (r < 0)
                        return log_error_errno(r, "Failed to submit PCR policy to TPM: %m");

@@ -9864,7 +9877,7 @@ int tpm2_policy_super_pcr(
                                &pcr_selection,
                                &current_policy_digest);
                if (r == -EUCLEAN)
-                        return log_error_errno(r, "Value for PCR %" PRIu32 " encoded in policy does not match the current TPM state. Either the system has been tampered with or the provided policy is incorrect.", pcr);
+                        return log_debug_errno(r, "PCR state changed while submitting the PolicyPCR expression for PCR %" PRIu32 ", session needs to be restarted.", pcr);
                if (r < 0)
                        return log_error_errno(r, "Failed to submit PCR policy to TPM: %m");

@@ -12024,8 +12037,16 @@ int tpm2_load_pcr_signature(const char *path, sd_json_variant **ret) {
                return log_debug_errno(r, "Failed to find TPM PCR signature file '%s': %m", path);

        r = sd_json_parse_file(f, discovered_path, 0, ret, NULL, NULL);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to parse TPM PCR signature JSON object '%s': %m", discovered_path);
+        if (r == -ENOMEM)
+                return log_oom_debug();
+        if (r < 0) {
+                log_debug_errno(r, "Failed to parse TPM PCR signature JSON object '%s': %m", discovered_path);
+
+                /* Don't propagate the parser's errno: it reports a string that is not valid UTF-8 as
+                 * -EUCLEAN, and to our callers that errno means "a PCR kept being extended". Report a
+                 * malformed file as such instead, whichever way it is malformed. */
+                return -EBADMSG;
+        }

        return 0;
 }
@@ -51,8 +51,10 @@ static inline bool TPM2_PCR_MASK_VALID(uint32_t pcr_mask) {
 int dlopen_tpm2(int log_level) _dlopen_loader_;

 /* tpm2_unseal() returns a bunch of different errors for various flavours of PCR issues, let's group them.
- * Kept outside the HAVE_TPM2 guard as callers switch on these errnos even in builds without TPM2. */
-#define ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r) IN_SET(r, -EREMCHG, -ENOANO, -EUCLEAN, -EPERM)
+ * Kept outside the HAVE_TPM2 guard as callers switch on these errnos even in builds without TPM2. Note that
+ * -EUCLEAN is not part of this: when returned by tpm2_unseal() it means a PCR was continuously extended
+ * while unsealing, which says nothing about whether the PCR state matches the policy. */
+#define ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(r) IN_SET(r, -EREMCHG, -ENOANO, -EPERM)

 /* Errors that mean the tried TPM2 token does not match the boot state, be it due to wrong PCR state, a
 * different PCR signing key/policy, a different TPM, or an unusable NV index. The caller should keep trying
@@ -338,6 +340,9 @@ int tpm2_make_policy_session(Tpm2Context *c, const Tpm2Handle *primary, const Tp
 int tpm2_policy_auth_value(Tpm2Context *c, const Tpm2Handle *session, TPM2B_DIGEST **ret_policy_digest);
 int tpm2_policy_nv_written(Tpm2Context *c, const Tpm2Handle *session, bool written_set, TPM2B_DIGEST **ret_policy_digest);
 int tpm2_policy_authorize_nv(Tpm2Context *c, const Tpm2Handle *session, const Tpm2Handle *nv_handle, TPM2B_DIGEST **ret_policy_digest);
+/* Note: tpm2_policy_pcr() and tpm2_policy_super_pcr() return -EUCLEAN if the TPM's global PCR update counter
+ * moved while the policy was submitted, i.e. if something extended a PCR in the meantime. That's transient,
+ * callers are supposed to restart the policy session and try again. */
 int tpm2_policy_pcr(Tpm2Context *c, const Tpm2Handle *session, const TPML_PCR_SELECTION *pcr_selection, TPM2B_DIGEST **ret_policy_digest);
 int tpm2_policy_or(Tpm2Context *c, const Tpm2Handle *session, const TPM2B_DIGEST *branches, size_t n_branches, TPM2B_DIGEST **ret_policy_digest);
 int tpm2_policy_super_pcr(Tpm2Context *c, const Tpm2Handle *session, const Tpm2PCRPrediction *prediction, uint16_t algorithm);
@@ -622,6 +627,9 @@ int tpm2_parse_pcr_argument(const char *arg, Tpm2PCRValue **ret_pcr_values, size
 int tpm2_parse_pcr_argument_append(const char *arg, Tpm2PCRValue **pcr_values, size_t *n_pcr_values);
 int tpm2_parse_pcr_argument_to_mask(const char *arg, uint32_t *mask);

+/* Note: tpm2_load_pcr_signature() returns -ENOENT if there is no such file, and -EBADMSG if there is one but
+ * it cannot be parsed. In particular it does not propagate the JSON parser's -EUCLEAN, which means something
+ * else to the TPM2 callers of this. */
 int tpm2_load_pcr_signature(const char *path, sd_json_variant **ret);
 int tpm2_load_pcr_public_key(const char *path, void **ret_pubkey, size_t *ret_pubkey_size);

@@ -79,6 +79,7 @@ static SD_VARLINK_DEFINE_ERROR(NullKeyNotAllowed);
 static SD_VARLINK_DEFINE_ERROR(KeyBelongsToOtherTPM);
 static SD_VARLINK_DEFINE_ERROR(TPMInDictionaryLockout);
 static SD_VARLINK_DEFINE_ERROR(UnexpectedPCRState);
+static SD_VARLINK_DEFINE_ERROR(PCRsKeptChanging);
 static SD_VARLINK_DEFINE_ERROR(NVIndexUnusable);

 SD_VARLINK_DEFINE_INTERFACE(
@@ -113,5 +114,7 @@ SD_VARLINK_DEFINE_INTERFACE(
                &vl_error_TPMInDictionaryLockout,
                SD_VARLINK_SYMBOL_COMMENT("Unexpected TPM PCR state of the system."),
                &vl_error_UnexpectedPCRState,
+                SD_VARLINK_SYMBOL_COMMENT("PCR values kept changing while unsealing the TPM2 secret, something on this system extends a PCR continuously."),
+                &vl_error_PCRsKeptChanging,
                SD_VARLINK_SYMBOL_COMMENT("The NV index referenced by the key is missing, unwritten, or unusable, it could be for another system."),
                &vl_error_NVIndexUnusable);
@@ -503,4 +503,41 @@ TEST(mime_type_matches) {
        }
 }

+TEST(credentials_varlink_error) {
+        /* The errno lookup answers with the first row that matches, so two rows sharing an errno would make
+         * one of them unreachable, and the error the client is told would silently be the wrong one. */
+
+        static const char* const ids[] = {
+                "io.systemd.Credentials.BadFormat",
+                "io.systemd.Credentials.NameMismatch",
+                "io.systemd.Credentials.TimeMismatch",
+                "io.systemd.Credentials.NoSuchUser",
+                "io.systemd.Credentials.BadScope",
+                "io.systemd.Credentials.CantFindPCRSignature",
+                "io.systemd.Credentials.NullKeyNotAllowed",
+                "io.systemd.Credentials.KeyBelongsToOtherTPM",
+                "io.systemd.Credentials.TPMInDictionaryLockout",
+                "io.systemd.Credentials.UnexpectedPCRState",
+                "io.systemd.Credentials.PCRsKeptChanging",
+                "io.systemd.Credentials.NVIndexUnusable",
+        };
+
+        FOREACH_ELEMENT(id, ids) {
+                const CredentialsVarlinkError *e = ASSERT_PTR(credentials_varlink_error_by_id(*id));
+
+                ASSERT_NOT_NULL(e->msg);
+                ASSERT_STREQ(ASSERT_PTR(credentials_varlink_error_by_errno(e->errnum))->id, *id);
+                ASSERT_PTR_EQ(credentials_varlink_error_by_errno(-e->errnum), e);
+        }
+
+        /* The two errnos the PCR diagnosis hangs on */
+        ASSERT_STREQ(ASSERT_PTR(credentials_varlink_error_by_errno(EPERM))->id,
+                     "io.systemd.Credentials.UnexpectedPCRState");
+        ASSERT_STREQ(ASSERT_PTR(credentials_varlink_error_by_errno(EUCLEAN))->id,
+                     "io.systemd.Credentials.PCRsKeptChanging");
+
+        ASSERT_NULL(credentials_varlink_error_by_id("io.systemd.Credentials.NoSuchError"));
+        ASSERT_NULL(credentials_varlink_error_by_errno(EDOM));
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);