diff --git a/src/cryptenroll/cryptenroll-tpm2.c b/src/cryptenroll/cryptenroll-tpm2.c index 6165a31642..21b77bd9c0 100644 --- a/src/cryptenroll/cryptenroll-tpm2.c +++ b/src/cryptenroll/cryptenroll-tpm2.c @@ -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"); diff --git a/src/cryptsetup/cryptsetup-tokens/luks2-tpm2.c b/src/cryptsetup/cryptsetup-tokens/luks2-tpm2.c index bb26193153..40a9ed1ffd 100644 --- a/src/cryptsetup/cryptsetup-tokens/luks2-tpm2.c +++ b/src/cryptsetup/cryptsetup-tokens/luks2-tpm2.c @@ -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. */ diff --git a/src/pcrlock/pcrlock.c b/src/pcrlock/pcrlock.c index 4f5a384072..8906cdff25 100644 --- a/src/pcrlock/pcrlock.c +++ b/src/pcrlock/pcrlock.c @@ -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)) diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c index 7fbddf7a27..2eefb7f575 100644 --- a/src/shared/creds-util.c +++ b/src/shared/creds-util.c @@ -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." }, }; diff --git a/src/shared/cryptsetup-tpm2.c b/src/shared/cryptsetup-tpm2.c index ecb312a960..e989c3c5fd 100644 --- a/src/shared/cryptsetup-tpm2.c +++ b/src/shared/cryptsetup-tpm2.c @@ -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. */ diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index 5c2622a291..f6bf2cc35a 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -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, ¤t_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, ¤t_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; } diff --git a/src/shared/tpm2-util.h b/src/shared/tpm2-util.h index 3723281f42..1d0ed81138 100644 --- a/src/shared/tpm2-util.h +++ b/src/shared/tpm2-util.h @@ -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); diff --git a/src/shared/varlink-io.systemd.Credentials.c b/src/shared/varlink-io.systemd.Credentials.c index d44af7e5f8..620c4f6f51 100644 --- a/src/shared/varlink-io.systemd.Credentials.c +++ b/src/shared/varlink-io.systemd.Credentials.c @@ -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); diff --git a/src/test/test-creds.c b/src/test/test-creds.c index 661a766502..cb57784c4b 100644 --- a/src/test/test-creds.c +++ b/src/test/test-creds.c @@ -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);