From 01c032df0401d1acb0b060ac2a2ef0412acc28be Mon Sep 17 00:00:00 2001 From: Ondrej Kozina Date: Mon, 15 Aug 2022 17:01:16 +0200 Subject: [PATCH] Do not reload LUKS2 metadata when not necessary. Following API calls trigerred LUKS2 metadata reload from storage in case of failure: crypt_convert crypt_keyslot_add_by_key crypt_keyslot_add_by_keyfile_device_offset crypt_keyslot_add_by_passphrase crypt_keyslot_change_by_passphrase crypt_reencrypt_init_by_keyring crypt_reencrypt_init_by_passphrase This patch replaces LUKS2 metadata reload with backup LUKS2 metadata copy kept in memory that is updated on each sucessfull metadata write and rolled back to it whenever needed in any of those calls listed above. --- lib/luks2/luks2.h | 2 + lib/luks2/luks2_json_metadata.c | 84 ++++++++++++++++++++++++++++++--- lib/luks2/luks2_reencrypt.c | 8 ++-- lib/setup.c | 22 +++++---- 4 files changed, 97 insertions(+), 19 deletions(-) diff --git a/lib/luks2/luks2.h b/lib/luks2/luks2.h index 3e619931..5aeeb270 100644 --- a/lib/luks2/luks2.h +++ b/lib/luks2/luks2.h @@ -121,6 +121,7 @@ struct luks2_hdr { uint8_t salt2[LUKS2_SALT_L]; char uuid[LUKS2_UUID_L]; void *jobj; + void *jobj_rollback; }; struct luks2_keyslot_params { @@ -167,6 +168,7 @@ int LUKS2_hdr_version_unlocked(struct crypt_device *cd, int LUKS2_hdr_read(struct crypt_device *cd, struct luks2_hdr *hdr, int repair); int LUKS2_hdr_write(struct crypt_device *cd, struct luks2_hdr *hdr); int LUKS2_hdr_write_force(struct crypt_device *cd, struct luks2_hdr *hdr); +int LUKS2_hdr_rollback(struct crypt_device *cd, struct luks2_hdr *hdr); int LUKS2_hdr_dump(struct crypt_device *cd, struct luks2_hdr *hdr); int LUKS2_hdr_dump_json(struct crypt_device *cd, struct luks2_hdr *hdr, const char **json); diff --git a/lib/luks2/luks2_json_metadata.c b/lib/luks2/luks2_json_metadata.c index 41114625..c6accd05 100644 --- a/lib/luks2/luks2_json_metadata.c +++ b/lib/luks2/luks2_json_metadata.c @@ -1110,6 +1110,33 @@ int LUKS2_hdr_validate(struct crypt_device *cd, json_object *hdr_jobj, uint64_t return 0; } +static bool hdr_json_free(json_object **jobj) +{ + assert(jobj); + + if (json_object_put(*jobj)) + *jobj = NULL; + + return (*jobj == NULL); +} + +static int hdr_update_copy_for_rollback(struct crypt_device *cd, struct luks2_hdr *hdr) +{ + json_object **jobj_copy; + + assert(hdr); + assert(hdr->jobj); + + jobj_copy = (json_object **)&hdr->jobj_rollback; + + if (!hdr_json_free(jobj_copy)) { + log_dbg(cd, "LUKS2 rollback metadata copy still in use"); + return -EINVAL; + } + + return json_object_copy(hdr->jobj, jobj_copy) ? -ENOMEM : 0; +} + /* FIXME: should we expose do_recovery parameter explicitly? */ int LUKS2_hdr_read(struct crypt_device *cd, struct luks2_hdr *hdr, int repair) { @@ -1141,6 +1168,9 @@ int LUKS2_hdr_read(struct crypt_device *cd, struct luks2_hdr *hdr, int repair) } else device_read_unlock(cd, crypt_metadata_device(cd)); + if (!r && (r = hdr_update_copy_for_rollback(cd, hdr))) + log_dbg(cd, "Failed to update rollback LUKS2 metadata."); + return r; } @@ -1153,18 +1183,50 @@ static int hdr_cleanup_and_validate(struct crypt_device *cd, struct luks2_hdr *h int LUKS2_hdr_write_force(struct crypt_device *cd, struct luks2_hdr *hdr) { + int r; + if (hdr_cleanup_and_validate(cd, hdr)) return -EINVAL; - return LUKS2_disk_hdr_write(cd, hdr, crypt_metadata_device(cd), false); + r = LUKS2_disk_hdr_write(cd, hdr, crypt_metadata_device(cd), false); + + if (!r && (r = hdr_update_copy_for_rollback(cd, hdr))) + log_dbg(cd, "Failed to update rollback LUKS2 metadata."); + + return r; } int LUKS2_hdr_write(struct crypt_device *cd, struct luks2_hdr *hdr) { + int r; + if (hdr_cleanup_and_validate(cd, hdr)) return -EINVAL; - return LUKS2_disk_hdr_write(cd, hdr, crypt_metadata_device(cd), true); + r = LUKS2_disk_hdr_write(cd, hdr, crypt_metadata_device(cd), true); + + if (!r && (r = hdr_update_copy_for_rollback(cd, hdr))) + log_dbg(cd, "Failed to update rollback LUKS2 metadata."); + + return r; +} + +int LUKS2_hdr_rollback(struct crypt_device *cd, struct luks2_hdr *hdr) +{ + json_object **jobj_copy; + + assert(hdr->jobj_rollback); + + log_dbg(cd, "Rolling back in-memory LUKS2 json metadata."); + + jobj_copy = (json_object **)&hdr->jobj; + + if (!hdr_json_free(jobj_copy)) { + log_dbg(cd, "LUKS2 header still in use"); + return -EINVAL; + } + + return json_object_copy(hdr->jobj_rollback, jobj_copy) ? -ENOMEM : 0; } int LUKS2_hdr_uuid(struct crypt_device *cd, struct luks2_hdr *hdr, const char *uuid) @@ -1201,10 +1263,19 @@ int LUKS2_hdr_labels(struct crypt_device *cd, struct luks2_hdr *hdr, void LUKS2_hdr_free(struct crypt_device *cd, struct luks2_hdr *hdr) { - if (json_object_put(hdr->jobj)) - hdr->jobj = NULL; - else if (hdr->jobj) + json_object **jobj; + + assert(hdr); + + jobj = (json_object **)&hdr->jobj; + + if (!hdr_json_free(jobj)) log_dbg(cd, "LUKS2 header still in use"); + + jobj = (json_object **)&hdr->jobj_rollback; + + if (!hdr_json_free(jobj)) + log_dbg(cd, "LUKS2 rollback metadata copy still in use"); } static uint64_t LUKS2_keyslots_size_jobj(json_object *jobj) @@ -1306,8 +1377,7 @@ int LUKS2_hdr_restore(struct crypt_device *cd, struct luks2_hdr *hdr, int r, fd, devfd = -1, diff_uuid = 0; ssize_t ret, buffer_size = 0; char *buffer = NULL, msg[1024]; - struct luks2_hdr hdr_file; - struct luks2_hdr tmp_hdr = {}; + struct luks2_hdr hdr_file = {}, tmp_hdr = {}; uint32_t reqs = 0; r = device_alloc(cd, &backup_device, backup_file); diff --git a/lib/luks2/luks2_reencrypt.c b/lib/luks2/luks2_reencrypt.c index abc03064..2fcde4b7 100644 --- a/lib/luks2/luks2_reencrypt.c +++ b/lib/luks2/luks2_reencrypt.c @@ -2898,8 +2898,8 @@ out: log_err(cd, _("Failed to resume device %s."), name); device_release_excl(cd, crypt_data_device(cd)); - if (r < 0 && crypt_load(cd, CRYPT_LUKS2, NULL) < 0) - log_dbg(cd, "Cannot reload context after failure."); + if (r < 0 && LUKS2_hdr_rollback(cd, hdr) < 0) + log_dbg(cd, "Failed to rollback LUKS2 metadata after failure."); return r; } @@ -3106,8 +3106,8 @@ static int reencrypt_init(struct crypt_device *cd, r = reencrypt_keyslot; out: device_release_excl(cd, crypt_data_device(cd)); - if (r < 0 && crypt_load(cd, CRYPT_LUKS2, NULL) < 0) - log_dbg(cd, "Cannot reload context after failure."); + if (r < 0 && LUKS2_hdr_rollback(cd, hdr) < 0) + log_dbg(cd, "Failed to rollback LUKS2 metadata after failure."); return r; } diff --git a/lib/setup.c b/lib/setup.c index c75e06b9..aace19db 100644 --- a/lib/setup.c +++ b/lib/setup.c @@ -743,12 +743,18 @@ out: return r; } -static void _luks2_reload(struct crypt_device *cd) +static void _luks2_rollback(struct crypt_device *cd) { if (!cd || !isLUKS2(cd->type)) return; - (void) _crypt_load_luks2(cd, 1, 0); + if (LUKS2_hdr_rollback(cd, &cd->u.luks2.hdr)) { + log_err(cd, _("Failed to rollback LUKS2 metadata in memory.")); + return; + } + + free(cd->u.luks2.keyslot_cipher); + cd->u.luks2.keyslot_cipher = NULL; } static int _crypt_load_luks(struct crypt_device *cd, const char *requested_type, @@ -3127,7 +3133,7 @@ int crypt_header_restore(struct crypt_device *cd, } else if (isLUKS2(cd->type) && (!requested_type || isLUKS2(requested_type))) { r = LUKS2_hdr_restore(cd, &cd->u.luks2.hdr, backup_file); if (r) - _luks2_reload(cd); + (void) _crypt_load_luks2(cd, 1, 0); } else if (isLUKS1(cd->type) && (!requested_type || isLUKS1(requested_type))) r = LUKS_hdr_restore(backup_file, &cd->u.luks1.hdr, cd); else @@ -3581,7 +3587,7 @@ int crypt_keyslot_add_by_passphrase(struct crypt_device *cd, out: crypt_free_volume_key(vk); if (r < 0) { - _luks2_reload(cd); + _luks2_rollback(cd); return r; } return keyslot; @@ -3698,7 +3704,7 @@ int crypt_keyslot_change_by_passphrase(struct crypt_device *cd, out: crypt_free_volume_key(vk); if (r < 0) { - _luks2_reload(cd); + _luks2_rollback(cd); return r; } return keyslot_new; @@ -3790,7 +3796,7 @@ out: crypt_safe_free(new_password); crypt_free_volume_key(vk); if (r < 0) { - _luks2_reload(cd); + _luks2_rollback(cd); return r; } return keyslot; @@ -5751,7 +5757,7 @@ int crypt_convert(struct crypt_device *cd, if (r < 0) { /* in-memory header may be invalid after failed conversion */ - _luks2_reload(cd); + _luks2_rollback(cd); if (r == -EBUSY) log_err(cd, _("Cannot convert device %s which is still in use."), mdata_device_path(cd)); return r; @@ -6163,7 +6169,7 @@ int crypt_keyslot_add_by_key(struct crypt_device *cd, out: crypt_free_volume_key(vk); if (r < 0) { - _luks2_reload(cd); + _luks2_rollback(cd); return r; } return keyslot;