Libguestfs - Oct 2020 - Re: [PATCH common v2 4/4] options: Ignore errors from guestfs_luks_uuid.

For links to the original patch series, see:
https://bugzilla.redhat.com/show_bug.cgi?id=1808977#c8

The original feedback was that ignoring errors from guestfs_luks_uuid
would ignore legitimate errors from non-BitLocker disks, so I have
modified this series so that errors are only ignored in the BitLocker
case.  As noted in the 4th patch there is no actual error in the
BitLocker case, cryptsetup luksUUID simply exits without printing
anything.

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

---
 options/decrypt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/options/decrypt.c b/options/decrypt.c
index d868f70..45de5b2 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -38,18 +38,18 @@
 
 /**
  * Make a LUKS map name from the partition name,
- * eg. C<"/dev/vda2" =E<gt> "luksvda2">
+ * eg. C<"/dev/vda2" =E<gt> "cryptvda2">
  */
 static void
 make_mapname (const char *device, char *mapname, size_t len)
 {
   size_t i = 0;
 
-  if (len < 5)
+  if (len < 6)
     abort ();
-  strcpy (mapname, "luks");
-  mapname += 4;
-  len -= 4;
+  strcpy (mapname, "crypt");
+  mapname += 5;
+  len -= 5;
 
   if (STRPREFIX (device, "/dev/"))
     i = 5;
-- 
2.27.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 102abff..1d1ac8a 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 45de5b2..8eb24bc 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -65,10 +65,8 @@ make_mapname (const char *device, char *mapname, size_t len)
 }
 
 /**
- * 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)
@@ -82,7 +80,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"))) {
       char mapname[32];
       make_mapname (partitions[i], mapname, sizeof mapname);
 
-- 
2.27.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.

Note there is no error message, cryptsetup simply returns with a bad
exit code:
><rescue> cryptsetup luksUUID /dev/sda2
><rescue> echo $?
1

Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788.
---
 options/decrypt.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/options/decrypt.c b/options/decrypt.c
index 8eb24bc..6b1c0a8 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -25,6 +25,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <string.h>
 #include <libintl.h>
 #include <error.h>
@@ -82,11 +83,23 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
     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 = guestfs_luks_uuid (g, partitions[i]);
+      CLEANUP_FREE char *uuid;
+      if (!is_bitlocker)
+        uuid = guestfs_luks_uuid (g, partitions[i]);
+      else {
+        /* This may fail for Windows BitLocker disks because
+         * cryptsetup luksUUID cannot read a UUID (unclear if
+         * this is a limitation of the format or cryptsetup).
+         */
+        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.27.0

On Mon, Sep 07, 2020 at 10:41:20AM +0100, 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.
>
>Note there is no error message, cryptsetup simply returns with a bad
>exit code:
>
>><rescue> cryptsetup luksUUID /dev/sda2
>><rescue> echo $?
>1
>
>Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788.
>---
> options/decrypt.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/options/decrypt.c b/options/decrypt.c
>index 8eb24bc..6b1c0a8 100644
>--- a/options/decrypt.c
>+++ b/options/decrypt.c
>@@ -25,6 +25,7 @@
>
> #include <stdio.h>
> #include <stdlib.h>
>+#include <stdbool.h>
> #include <string.h>
> #include <libintl.h>
> #include <error.h>
>@@ -82,11 +83,23 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>     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 = guestfs_luks_uuid (g, partitions[i]);
>+      CLEANUP_FREE char *uuid;
>+      if (!is_bitlocker)
>+        uuid = guestfs_luks_uuid (g, partitions[i]);
>+      else {
>+        /* This may fail for Windows BitLocker disks because
>+         * cryptsetup luksUUID cannot read a UUID (unclear if
>+         * this is a limitation of the format or cryptsetup).
>+         */
>+        guestfs_push_error_handler (g, NULL, NULL);
>+        uuid = guestfs_luks_uuid (g, partitions[i]);
I have yet to look at the libguestfs patches, but I do not completely understand
what is the reason for calling "guestfs_luks_uuid" when you know it
will fail.
Or is there a case when it might be useful to get the result?

Other than that the series looks fine to me.
>+        guestfs_pop_error_handler (g);
>+      }
> #else
>       const char *uuid = NULL;
> #endif
>-- 
>2.27.0
>
>_______________________________________________
>Libguestfs mailing list
>Libguestfs@redhat.com
>https://www.redhat.com/mailman/listinfo/libguestfs