Libguestfs - Mar 2020 - [PATCH common 2/4] options: Generate cryptsetup mapnames beginning with "crypt..." not "luks..."

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

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

---
 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

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

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