Re: [Libguestfs] [libguestfs-common PATCH 1/2] options: extract & refactor decryption of mountables (partitions)

The inspect_do_decrypt() function interates over the list of partitions. For each partition, it fetches the potentially matching keys, and tries each key in turn. If no key unlocks the partition, the program exits with an error message. If at least one partition is unlocked, then LVM is rescanned. Decouple "partition" from the above logic, replacing it with "mountable". Call the new function decrypt_mountables(). As a part of the extraction, clean up the following readability warts: - CLEANUP_FREE* and other variable declarations intermixed with code, - a "goto" statement that is not used on an error path, - needless conditions on a one-shot "is_bitlocker" helper variable, - nondescript array indices, - guestfs_int_count_strings() called for counting the length of the full string list, only to check if the string list is non-empty, - a comment about GUESTFS_CRYPTSETUP_OPEN_READONLY placed outside of GUESTFS_HAVE_CRYPTSETUP_OPEN. Regression-checked with: - libguestfs/tests/luks/test-key-option-inspect.sh - guestfs-tools/inspector/test-virt-inspector-luks.sh Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1658126 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- options/decrypt.c | 128 +++++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 53 deletions(-) diff --git a/options/decrypt.c b/options/decrypt.c index 434b7d58c2a7..9141cf5193ad 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -65,6 +65,78 @@ make_mapname (const char *device, char *mapname, size_t len) *mapname = '\0'; } +static bool +decrypt_mountables (guestfs_h *g, const char * const *mountables, + struct key_store *ks) +{ + bool decrypted_some = false; + const char * const *mnt_scan = mountables; + const char *mountable; + + while ((mountable = *mnt_scan++) != NULL) { + CLEANUP_FREE char *type = NULL; + CLEANUP_FREE char *uuid = NULL; + CLEANUP_FREE_STRING_LIST char **keys = NULL; + char mapname[32]; + const char * const *key_scan; + const char *key; + + type = guestfs_vfs_type (g, mountable); + if (type == NULL) + continue; + + /* "cryptsetup luksUUID" cannot read a UUID on Windows BitLocker disks + * (unclear if this is a limitation of the format or cryptsetup). + */ + if (STREQ (type, "crypto_LUKS")) { +#ifdef GUESTFS_HAVE_LUKS_UUID + uuid = guestfs_luks_uuid (g, mountable); +#endif

+ } else if (STRNEQ (type, "BitLocker")) + continue; + + /* Grab the keys that we should try with this device, based on device name, + * or UUID (if any). + */ + keys = get_keys (ks, mountable, uuid); + assert (keys[0] != NULL); + + /* Generate a node name for the plaintext (decrypted) device node. */ + make_mapname (mountable, mapname, sizeof mapname); + + /* Try each key in turn. */ + key_scan = (const char * const *)keys; + while ((key = *key_scan++) != NULL) { + int r; + + guestfs_push_error_handler (g, NULL, NULL); +#ifdef GUESTFS_HAVE_CRYPTSETUP_OPEN + /* XXX Should we set GUESTFS_CRYPTSETUP_OPEN_READONLY if readonly is + * set? This might break 'mount_ro'. + */

+ r = guestfs_cryptsetup_open (g, mountable, key, mapname, -1); +#else + r = guestfs_luks_open (g, mountable, key, mapname); +#endif + guestfs_pop_error_handler (g); + + if (r == 0) + break; + } + + if (key == NULL) + error (EXIT_FAILURE, 0, + _("could not find key to open LUKS encrypted %s.\n\n" + "Try using --key on the command line.\n\n" + "Original error: %s (%d)"), + mountable, guestfs_last_error (g), guestfs_last_errno (g)); + + decrypted_some = true; + } + + return decrypted_some; +} + /** * Simple implementation of decryption: look for any encrypted * partitions and decrypt them, then rescan for VGs. @@ -73,62 +145,12 @@ void inspect_do_decrypt (guestfs_h *g, struct key_store *ks) { CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions (g); + bool need_rescan; + if (partitions == NULL) exit (EXIT_FAILURE); - int need_rescan = 0, r; - size_t i, j; - - for (i = 0; partitions[i] != NULL; ++i) { - CLEANUP_FREE char *type = guestfs_vfs_type (g, partitions[i]); - if (type && - (STREQ (type, "crypto_LUKS") || STREQ (type, "BitLocker"))) { - bool is_bitlocker = STREQ (type, "BitLocker"); - char mapname[32]; - make_mapname (partitions[i], mapname, sizeof mapname); - -#ifdef GUESTFS_HAVE_LUKS_UUID - CLEANUP_FREE char *uuid = NULL; - - /* This fails for Windows BitLocker disks because cryptsetup - * luksUUID cannot read a UUID (unclear if this is a limitation - * of the format or cryptsetup). - */ - if (!is_bitlocker) - uuid = guestfs_luks_uuid (g, partitions[i]); -#else - const char *uuid = NULL; -#endif - - CLEANUP_FREE_STRING_LIST char **keys = get_keys (ks, partitions[i], uuid); - assert (guestfs_int_count_strings (keys) > 0); - - /* Try each key in turn. */ - for (j = 0; keys[j] != NULL; ++j) { - /* XXX Should we set GUESTFS_CRYPTSETUP_OPEN_READONLY if readonly - * is set? This might break 'mount_ro'. - */ - guestfs_push_error_handler (g, NULL, NULL); -#ifdef GUESTFS_HAVE_CRYPTSETUP_OPEN - r = guestfs_cryptsetup_open (g, partitions[i], keys[j], mapname, -1); -#else - r = guestfs_luks_open (g, partitions[i], keys[j], mapname); -#endif - guestfs_pop_error_handler (g); - if (r == 0) - goto opened; - } - error (EXIT_FAILURE, 0, - _("could not find key to open LUKS encrypted %s.\n\n" - "Try using --key on the command line.\n\n" - "Original error: %s (%d)"), - partitions[i], guestfs_last_error (g), - guestfs_last_errno (g)); - - opened: - need_rescan = 1; - } - } + need_rescan = decrypt_mountables (g, (const char * const *)partitions, ks); if (need_rescan) { if (guestfs_lvm_scan (g, 1) == -1)