From 27f42786fe2bcf9f4eba17d1869e69dcde6f9bda Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 2 May 2024 18:34:36 +0200 Subject: [PATCH 1/3] cryptenroll: do not combine pcrlock and signed PCR policies in TPM mode We currently do not support pcrlock policies and signed PCR policies in combination. Hence, when we auto-discover both, let's disable signed PCR policies if pcrlock is available too (simple because that covers more ground). Fixes: #32565 --- src/cryptenroll/cryptenroll-tpm2.c | 16 ++++++++++------ src/cryptenroll/cryptenroll-tpm2.h | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/cryptenroll/cryptenroll-tpm2.c b/src/cryptenroll/cryptenroll-tpm2.c index 1656dc1e83..1423f3b2ac 100644 --- a/src/cryptenroll/cryptenroll-tpm2.c +++ b/src/cryptenroll/cryptenroll-tpm2.c @@ -249,8 +249,8 @@ int enroll_tpm2(struct crypt_device *cd, const char *device_key, Tpm2PCRValue *hash_pcr_values, size_t n_hash_pcr_values, - const char *pubkey_path, - bool load_pubkey, + const char *pcr_pubkey_path, + bool load_pcr_pubkey, uint32_t pubkey_pcr_mask, const char *signature_path, bool use_pin, @@ -307,10 +307,13 @@ int enroll_tpm2(struct crypt_device *cd, } TPM2B_PUBLIC public = {}; - if (load_pubkey) { - r = tpm2_load_pcr_public_key(pubkey_path, &pubkey.iov_base, &pubkey.iov_len); + /* Load the PCR public key if specified explicitly, or if no pcrlock policy was specified and + * automatic loading of PCR public keys wasn't disabled explicitly. The reason we turn this off when + * pcrlock is configured is simply that we currently not support both in combination. */ + if (pcr_pubkey_path || (load_pcr_pubkey && !pcrlock_path)) { + r = tpm2_load_pcr_public_key(pcr_pubkey_path, &pubkey.iov_base, &pubkey.iov_len); if (r < 0) { - if (pubkey_path || signature_path || r != -ENOENT) + if (pcr_pubkey_path || signature_path || r != -ENOENT) return log_error_errno(r, "Failed to read TPM PCR public key: %m"); log_debug_errno(r, "Failed to read TPM2 PCR public key, proceeding without: %m"); @@ -329,7 +332,8 @@ int enroll_tpm2(struct crypt_device *cd, return log_debug_errno(r, "Failed to read TPM PCR signature: %m"); } } - } + } else + pubkey_pcr_mask = 0; bool any_pcr_value_specified = tpm2_pcr_values_has_any_values(hash_pcr_values, n_hash_pcr_values); diff --git a/src/cryptenroll/cryptenroll-tpm2.h b/src/cryptenroll/cryptenroll-tpm2.h index 4522b0b595..d722ed66a0 100644 --- a/src/cryptenroll/cryptenroll-tpm2.h +++ b/src/cryptenroll/cryptenroll-tpm2.h @@ -9,14 +9,14 @@ #if HAVE_TPM2 int load_volume_key_tpm2(struct crypt_device *cd, const char *cd_node, const char *device, void *ret_vk, size_t *ret_vks); -int enroll_tpm2(struct crypt_device *cd, const void *volume_key, size_t volume_key_size, const char *device, uint32_t seal_key_handle, const char *device_key, Tpm2PCRValue *hash_pcr_values, size_t n_hash_pcr_values, const char *pubkey_path, bool disable_loading_pubkey, uint32_t pubkey_pcr_mask, const char *signature_path, bool use_pin, const char *pcrlock_path, int *ret_slot_to_wipe); +int enroll_tpm2(struct crypt_device *cd, const void *volume_key, size_t volume_key_size, const char *device, uint32_t seal_key_handle, const char *device_key, Tpm2PCRValue *hash_pcr_values, size_t n_hash_pcr_values, const char *pubkey_path, bool load_pcr_pubkey, uint32_t pubkey_pcr_mask, const char *signature_path, bool use_pin, const char *pcrlock_path, int *ret_slot_to_wipe); #else static inline int load_volume_key_tpm2(struct crypt_device *cd, const char *cd_node, const char *device, void *ret_vk, size_t *ret_vks) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 unlocking not supported."); } -static inline int enroll_tpm2(struct crypt_device *cd, const void *volume_key, size_t volume_key_size, const char *device, uint32_t seal_key_handle, const char *device_key, Tpm2PCRValue *hash_pcr_values, size_t n_hash_pcr_values, const char *pubkey_path, bool disable_loading_pubkey, uint32_t pubkey_pcr_mask, const char *signature_path, bool use_pin, const char *pcrlock_path, int *ret_slot_to_wipe) { +static inline int enroll_tpm2(struct crypt_device *cd, const void *volume_key, size_t volume_key_size, const char *device, uint32_t seal_key_handle, const char *device_key, Tpm2PCRValue *hash_pcr_values, size_t n_hash_pcr_values, const char *pubkey_path, bool load_pcr_pubkey, uint32_t pubkey_pcr_mask, const char *signature_path, bool use_pin, const char *pcrlock_path, int *ret_slot_to_wipe) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 key enrollment not supported."); } From e6ca81d43422e6ea7b1a99c15d21b71f1628f32f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 2 May 2024 18:41:36 +0200 Subject: [PATCH 2/3] cryptenroll: determine TPM enrollment parmaeters only if we actually do TPM enrollments Otherwise we'll do work (and possibly generate fatal errors) where we really shouldn't. --- src/cryptenroll/cryptenroll.c | 44 ++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/cryptenroll/cryptenroll.c b/src/cryptenroll/cryptenroll.c index 6e700d3014..04352bfec6 100644 --- a/src/cryptenroll/cryptenroll.c +++ b/src/cryptenroll/cryptenroll.c @@ -637,31 +637,33 @@ static int parse_argv(int argc, char *argv[]) { } } - if (auto_pcrlock) { - assert(!arg_tpm2_pcrlock); + if (arg_enroll_type == ENROLL_TPM2) { + if (auto_pcrlock) { + assert(!arg_tpm2_pcrlock); - r = tpm2_pcrlock_search_file(NULL, NULL, &arg_tpm2_pcrlock); - if (r < 0) { - if (r != -ENOENT) - log_warning_errno(r, "Search for pcrlock.json failed, assuming it does not exist: %m"); - } else - log_info("Automatically using pcrlock policy '%s'.", arg_tpm2_pcrlock); - } + r = tpm2_pcrlock_search_file(NULL, NULL, &arg_tpm2_pcrlock); + if (r < 0) { + if (r != -ENOENT) + log_warning_errno(r, "Search for pcrlock.json failed, assuming it does not exist: %m"); + } else + log_info("Automatically using pcrlock policy '%s'.", arg_tpm2_pcrlock); + } - if (auto_public_key_pcr_mask) { - assert(arg_tpm2_public_key_pcr_mask == 0); - arg_tpm2_public_key_pcr_mask = INDEX_TO_MASK(uint32_t, TPM2_PCR_KERNEL_BOOT); - } + if (auto_public_key_pcr_mask) { + assert(arg_tpm2_public_key_pcr_mask == 0); + arg_tpm2_public_key_pcr_mask = INDEX_TO_MASK(uint32_t, TPM2_PCR_KERNEL_BOOT); + } - if (auto_hash_pcr_values && !arg_tpm2_pcrlock) { /* Only lock to PCR 7 by default if no pcrlock policy is around (which is a better replacement) */ - assert(arg_tpm2_n_hash_pcr_values == 0); + if (auto_hash_pcr_values && !arg_tpm2_pcrlock) { /* Only lock to PCR 7 by default if no pcrlock policy is around (which is a better replacement) */ + assert(arg_tpm2_n_hash_pcr_values == 0); - if (!GREEDY_REALLOC_APPEND( - arg_tpm2_hash_pcr_values, - arg_tpm2_n_hash_pcr_values, - &TPM2_PCR_VALUE_MAKE(TPM2_PCR_INDEX_DEFAULT, /* hash= */ 0, /* value= */ {}), - 1)) - return log_oom(); + if (!GREEDY_REALLOC_APPEND( + arg_tpm2_hash_pcr_values, + arg_tpm2_n_hash_pcr_values, + &TPM2_PCR_VALUE_MAKE(TPM2_PCR_INDEX_DEFAULT, /* hash= */ 0, /* value= */ {}), + 1)) + return log_oom(); + } } return 1; From 3f2402171b26e9257f41417d749fe207fc329085 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 May 2024 16:12:04 +0200 Subject: [PATCH 3/3] tpm2-util: tweak JSON condition check As for the other fields let's check if the actual variable we serialize is set before serializing it. This shouldn't make any difference, since the pubkey and the PCR mask should always be set together or neither, but I think it's easier to grok this way, and makes the function nicely "dumb": it serializes what is specified, without trying to be smart by suppressng specified fields. --- src/shared/tpm2-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index e0c2c63878..3afeb09516 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -7382,7 +7382,7 @@ int tpm2_make_luks2_json( JSON_BUILD_PAIR("tpm2-pin", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PIN)), JSON_BUILD_PAIR("tpm2_pcrlock", JSON_BUILD_BOOLEAN(flags & TPM2_FLAGS_USE_PCRLOCK)), JSON_BUILD_PAIR_CONDITION(pubkey_pcr_mask != 0, "tpm2_pubkey_pcrs", JSON_BUILD_VARIANT(pkmj)), - JSON_BUILD_PAIR_CONDITION(pubkey_pcr_mask != 0, "tpm2_pubkey", JSON_BUILD_IOVEC_BASE64(pubkey)), + JSON_BUILD_PAIR_CONDITION(iovec_is_set(pubkey), "tpm2_pubkey", JSON_BUILD_IOVEC_BASE64(pubkey)), JSON_BUILD_PAIR_CONDITION(iovec_is_set(salt), "tpm2_salt", JSON_BUILD_IOVEC_BASE64(salt)), JSON_BUILD_PAIR_CONDITION(iovec_is_set(srk), "tpm2_srk", JSON_BUILD_IOVEC_BASE64(srk)), JSON_BUILD_PAIR_CONDITION(iovec_is_set(pcrlock_nv), "tpm2_pcrlock_nv", JSON_BUILD_IOVEC_BASE64(pcrlock_nv))));