[PATCH common 0/4] options: Support Windows BitLocker (RHBZ#1808977).
Support transparent decryption/inspection of Windows guests encrypted with BitLocker encryption. This won't make much sense without the associated libguestfs patches which I will post momentarily. (Submodules, ho hum) Rich.
Fall back to luks-open if we're using libguestfs <= 1.43.1. --- options/decrypt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/options/decrypt.c b/options/decrypt.c index 683cf5e..d868f70 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -97,11 +97,15 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) /* Try each key in turn. */ for (j = 0; keys[j] != NULL; ++j) { - /* XXX Should we call guestfs_luks_open_ro if readonly flag + /* 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; -- 2.25.0
Since we're no longer only inspecting LUKS devices, use a more generic name. This rewrite also makes the function clearer. --- options/decrypt.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/options/decrypt.c b/options/decrypt.c index d868f70..58f8df9 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -28,6 +28,7 @@ #include <string.h> #include <libintl.h> #include <error.h> +#include <errno.h> #include <assert.h> #include "c-ctype.h" @@ -37,31 +38,27 @@ #include "options.h" /** - * Make a LUKS map name from the partition name, - * eg. C<"/dev/vda2" =E<gt> "luksvda2"> + * Make a cryptsetup map name from the partition name, + * eg. C<"/dev/vda2" =E<gt> "cryptvda2"> */ -static void -make_mapname (const char *device, char *mapname, size_t len) +static char * +make_mapname (const char *device) { size_t i = 0; - - if (len < 5) - abort (); - strcpy (mapname, "luks"); - mapname += 4; - len -= 4; + char *ret; if (STRPREFIX (device, "/dev/")) - i = 5; + device += 5; - for (; device[i] != '\0' && len >= 1; ++i) { - if (c_isalnum (device[i])) { - *mapname++ = device[i]; - len--; - } + if (asprintf (&ret, "crypt%s", &device[i]) == -1) + error (EXIT_FAILURE, errno, "asprintf"); + + for (i = 5; i < strlen (ret); ++i) { + if (!c_isalnum (ret[i])) + memmove (&ret[i], &ret[i+1], strlen (&ret[i])); } - *mapname = '\0'; + return ret; } /** @@ -83,8 +80,7 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) for (i = 0; partitions[i] != NULL; ++i) { CLEANUP_FREE char *type = guestfs_vfs_type (g, partitions[i]); if (type && STREQ (type, "crypto_LUKS")) { - char mapname[32]; - make_mapname (partitions[i], mapname, sizeof mapname); + CLEANUP_FREE char *mapname = make_mapname (partitions[i]); #ifdef GUESTFS_HAVE_LUKS_UUID CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]); -- 2.25.0
On Monday, 30 March 2020 16:58:47 CEST Richard W.M. Jones wrote:
- for (; device[i] != '\0' && len >= 1; ++i) { - if (c_isalnum (device[i])) { - *mapname++ = device[i]; - len--; - } + if (asprintf (&ret, "crypt%s", &device[i]) == -1) + error (EXIT_FAILURE, errno, "asprintf"); + + for (i = 5; i < strlen (ret); ++i) { + if (!c_isalnum (ret[i])) + memmove (&ret[i], &ret[i+1], strlen (&ret[i])); }
I'd say that this can be simplified even more: instead of first copying the string (asprintf) and then remove all the unwanted characters by shifting the characters after (with memmove), simply allocate a string of the maximum length needed and then copy the wanted characters. ... that is, use the old approach. -- Pino Toscano
For BitLocker disks cryptsetup does not (yet? ever?) support reading UUIDs and this function will fail. This does not matter here so just ignore the error. Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788. --- options/decrypt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/options/decrypt.c b/options/decrypt.c index 58f8df9..069a83f 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -83,7 +83,11 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) CLEANUP_FREE char *mapname = make_mapname (partitions[i]); #ifdef GUESTFS_HAVE_LUKS_UUID - CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]); + /* This may fail for Windows BitLocker disks, so hide errors. */ + CLEANUP_FREE char *uuid; + guestfs_push_error_handler (g, NULL, NULL); + uuid = guestfs_luks_uuid (g, partitions[i]); + guestfs_pop_error_handler (g); #else const char *uuid = NULL; #endif -- 2.25.0
On Monday, 30 March 2020 16:58:48 CEST Richard W.M. Jones wrote:
For BitLocker disks cryptsetup does not (yet? ever?) support reading UUIDs and this function will fail. This does not matter here so just ignore the error.
Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788. --- options/decrypt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/options/decrypt.c b/options/decrypt.c index 58f8df9..069a83f 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -83,7 +83,11 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) CLEANUP_FREE char *mapname = make_mapname (partitions[i]);
#ifdef GUESTFS_HAVE_LUKS_UUID - CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]); + /* This may fail for Windows BitLocker disks, so hide errors. */ + CLEANUP_FREE char *uuid; + guestfs_push_error_handler (g, NULL, NULL); + uuid = guestfs_luks_uuid (g, partitions[i]); + guestfs_pop_error_handler (g);
The problem is that this suppressed all the errors, even valid ones in Linux guests, which we want to know. What is the actual error? Can we detect and selectively ignore it instead? -- Pino Toscano
--- mltools/tools_utils.mli | 5 ++--- options/decrypt.c | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/mltools/tools_utils.mli b/mltools/tools_utils.mli index ab70f58..9912a50 100644 --- a/mltools/tools_utils.mli +++ b/mltools/tools_utils.mli @@ -195,9 +195,8 @@ val is_btrfs_subvolume : Guestfs.guestfs -> string -> bool (** Checks if a filesystem is a btrfs subvolume. *) val inspect_decrypt : Guestfs.guestfs -> key_store -> unit -(** Simple implementation of decryption: look for any [crypto_LUKS] - partitions and decrypt them, then rescan for VGs. This only works - for Fedora whole-disk encryption. *) +(** Simple implementation of decryption: look for any encrypted + partitions and decrypt them, then rescan for VGs. *) val with_timeout : string -> int -> ?sleep:int -> (unit -> 'a option) -> 'a (** [with_timeout op timeout ?sleep fn] implements a timeout loop. diff --git a/options/decrypt.c b/options/decrypt.c index 069a83f..eac21d3 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -62,10 +62,8 @@ make_mapname (const char *device) } /** - * Simple implementation of decryption: look for any C<crypto_LUKS> - * partitions and decrypt them, then rescan for VGs. This only works - * for Fedora whole-disk encryption. WIP to make this work for other - * encryption schemes. + * Simple implementation of decryption: look for any encrypted + * partitions and decrypt them, then rescan for VGs. */ void inspect_do_decrypt (guestfs_h *g, struct key_store *ks) @@ -79,7 +77,8 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks) for (i = 0; partitions[i] != NULL; ++i) { CLEANUP_FREE char *type = guestfs_vfs_type (g, partitions[i]); - if (type && STREQ (type, "crypto_LUKS")) { + if (type && + (STREQ (type, "crypto_LUKS") || STREQ (type, "BitLocker"))) { CLEANUP_FREE char *mapname = make_mapname (partitions[i]); #ifdef GUESTFS_HAVE_LUKS_UUID -- 2.25.0
participants (2)
-
Pino Toscano -
Richard W.M. Jones