Skip to content

Commit

Permalink
Unlink only volume keys that were previously stored in keyring.
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
mbroz committed Nov 25, 2024
1 parent 9575dad commit ae4b4ff
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 17 deletions.
9 changes: 5 additions & 4 deletions lib/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
};
Expand Down Expand Up @@ -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)
{
Expand Down
8 changes: 4 additions & 4 deletions lib/luks2/luks2_reencrypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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
Expand Down
22 changes: 13 additions & 9 deletions lib/setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/volumekey.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit ae4b4ff

Please sign in to comment.