From ae4b4ff4d71bba252f6ed2fb63b7bcac539a5b85 Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Fri, 22 Nov 2024 11:32:03 +0100 Subject: [PATCH] Unlink only volume keys that were previously stored in keyring. This is only preparation for an extension later, however, the volume keys should not be unloaded unconditionally from keyring. Note that all other places dropping keys already check that keys were uploaded through key ID setting. (And for suspend unconditional unlink make sense too.) --- lib/internal.h | 9 +++++---- lib/luks2/luks2_reencrypt.c | 8 ++++---- lib/setup.c | 22 +++++++++++++--------- lib/volumekey.c | 1 + 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 5b2247b1..a85bf57d 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -51,9 +51,10 @@ struct luks2_reencrypt; struct volume_key { int id; - size_t keylength; - const char *key_description; - key_type_t keyring; + size_t keylength; /* length in bytes */ + const char *key_description; /* keyring key name/description */ + key_type_t keyring; /* type of keyring where the key is stored */ + bool uploaded; /* uploaded to keyring, can drop it */ struct volume_key *next; char key[]; }; @@ -235,7 +236,7 @@ int crypt_keyring_get_key_by_name(struct crypt_device *cd, size_t *key_size); int crypt_use_keyring_for_vk(struct crypt_device *cd); void crypt_drop_keyring_key_by_description(struct crypt_device *cd, const char *key_description, key_type_t ktype); -void crypt_drop_keyring_key(struct crypt_device *cd, struct volume_key *vks); +void crypt_drop_uploaded_keyring_key(struct crypt_device *cd, struct volume_key *vks); static inline uint64_t compact_version(uint16_t major, uint16_t minor, uint16_t patch, uint16_t release) { diff --git a/lib/luks2/luks2_reencrypt.c b/lib/luks2/luks2_reencrypt.c index ec647bad..7901f398 100644 --- a/lib/luks2/luks2_reencrypt.c +++ b/lib/luks2/luks2_reencrypt.c @@ -886,7 +886,7 @@ void LUKS2_reencrypt_free(struct crypt_device *cd, struct luks2_reencrypt *rh) free(rh->device_name); free(rh->overlay_name); free(rh->hotzone_name); - crypt_drop_keyring_key(cd, rh->vks); + crypt_drop_uploaded_keyring_key(cd, rh->vks); crypt_free_volume_key(rh->vks); device_release_excl(cd, crypt_data_device(cd)); crypt_unlock_internal(cd, rh->reenc_lock); @@ -2683,7 +2683,7 @@ static int reencrypt_upload_keys(struct crypt_device *cd, if (digest_old >= 0 && !crypt_is_cipher_null(reencrypt_segment_cipher_old(hdr)) && (r = reencrypt_upload_single_key(cd, digest_old, vks))) { - crypt_drop_keyring_key(cd, vks); + crypt_drop_uploaded_keyring_key(cd, vks); return r; } @@ -3879,7 +3879,7 @@ static int reencrypt_init_by_keyslot_context(struct crypt_device *cd, keyslot_new, &vks, params); out: if (r < 0) - crypt_drop_keyring_key(cd, vks); + crypt_drop_uploaded_keyring_key(cd, vks); crypt_free_volume_key(vks); return r < 0 ? r : LUKS2_find_keyslot(hdr, "reencrypt"); } @@ -4470,7 +4470,7 @@ int LUKS2_reencrypt_locked_recovery_by_vks(struct crypt_device *cd, out: if (r < 0) - crypt_drop_keyring_key(cd, vks); + crypt_drop_uploaded_keyring_key(cd, vks); return r; } #endif diff --git a/lib/setup.c b/lib/setup.c index 65480c5e..685d199f 100644 --- a/lib/setup.c +++ b/lib/setup.c @@ -4159,9 +4159,10 @@ static key_serial_t crypt_single_volume_key_load_in_user_keyring(struct crypt_de type_name, user_key_name); kid = keyring_add_key_to_custom_keyring(cd->keyring_key_type, user_key_name, vk->key, vk->keylength, cd->keyring_to_link_vk); - if (kid <= 0) { + if (kid <= 0) log_dbg(cd, "The keyring_link_key_to_keyring function failed (error %d).", errno); - } + else + vk->uploaded = true; return kid; } @@ -4181,7 +4182,7 @@ static int crypt_volume_key_load_in_user_keyring(struct crypt_device *cd, struct if (kid1 <= 0) return -EINVAL; - vk = vk->next; + vk = crypt_volume_key_next(vk); if (vk) { assert(cd->user_key_name2); kid2 = crypt_single_volume_key_load_in_user_keyring(cd, vk, cd->user_key_name2); @@ -4298,7 +4299,7 @@ static int resume_luks2_by_volume_key(struct crypt_device *cd, out: if (r < 0) { - crypt_drop_keyring_key(cd, p_crypt); + crypt_drop_uploaded_keyring_key(cd, p_crypt); if (cd->link_vk_to_keyring && kid1) crypt_unlink_key_from_custom_keyring(cd, kid1); if (cd->link_vk_to_keyring && kid2) @@ -5442,8 +5443,8 @@ int crypt_activate_by_keyslot_context(struct crypt_device *cd, r = unlocked_keyslot; out: if (r < 0) { - crypt_drop_keyring_key(cd, vk); - crypt_drop_keyring_key(cd, crypt_key); + crypt_drop_uploaded_keyring_key(cd, vk); + crypt_drop_uploaded_keyring_key(cd, crypt_key); if (cd->link_vk_to_keyring && kid1) crypt_unlink_key_from_custom_keyring(cd, kid1); if (cd->link_vk_to_keyring && kid2) @@ -7331,8 +7332,10 @@ int crypt_volume_key_load_in_keyring(struct crypt_device *cd, struct volume_key if (kid < 0) { log_dbg(cd, "keyring_add_key_in_thread_keyring failed (error %d)", errno); log_err(cd, _("Failed to load key in kernel keyring.")); - } else + } else { crypt_set_key_in_keyring(cd, 1); + vk->uploaded = true; + } return kid < 0 ? -EINVAL : 0; } @@ -7516,12 +7519,13 @@ int crypt_set_keyring_to_link(struct crypt_device *cd, const char *key_descripti } /* internal only */ -void crypt_drop_keyring_key(struct crypt_device *cd, struct volume_key *vks) +void crypt_drop_uploaded_keyring_key(struct crypt_device *cd, struct volume_key *vks) { struct volume_key *vk = vks; while (vk) { - crypt_drop_keyring_key_by_description(cd, vk->key_description, LOGON_KEY); + if (vk->uploaded) + crypt_drop_keyring_key_by_description(cd, vk->key_description, LOGON_KEY); vk = crypt_volume_key_next(vk); } } diff --git a/lib/volumekey.c b/lib/volumekey.c index 505faf9a..796d3b01 100644 --- a/lib/volumekey.c +++ b/lib/volumekey.c @@ -27,6 +27,7 @@ struct volume_key *crypt_alloc_volume_key(size_t keylength, const char *key) vk->key_description = NULL; vk->keyring = INVALID_KEY; vk->keylength = keylength; + vk->uploaded = false; vk->id = KEY_NOT_VERIFIED; vk->next = NULL;