thr3ads.net - Libguestfs - [Libguestfs] [common PATCH 1/2] options: rename key.device as key.id [Nov 2019]

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2019-Nov-29 11:18 UTC

[Libguestfs] [PATCH 0/1] Allow UUIDs for --key identifiers.

This combined patch series enables to decrypt LUKS devices on inspection
by allowing the UUID of the LUKS device with the --key syntax.

I opted for reusing the option instead of adding another one, as I think
that device names and UUIDs are different enough that can be properly
distinguished.

A test for this (patch #4) can be applied only when the patches for
common are applied, and the submodule updated.

Pino Toscano (1):
  New API: luks_uuid

 daemon/luks.c             | 25 +++++++++++++++++++++++++
 generator/actions_core.ml |  8 ++++++++
 generator/proc_nr.ml      |  1 +
 lib/MAX_PROC_NR           |  2 +-
 4 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.21.0

Pino Toscano

2019-Nov-29 11:18 UTC

head link

[Libguestfs] [PATCH 1/1] New API: luks_uuid

Return the UUID of a LUKS device.
---
 daemon/luks.c             | 25 +++++++++++++++++++++++++
 generator/actions_core.ml |  8 ++++++++
 generator/proc_nr.ml      |  1 +
 lib/MAX_PROC_NR           |  2 +-
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/daemon/luks.c b/daemon/luks.c
index 5c48a91eb..d631cb100 100644
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -292,3 +292,28 @@ do_luks_kill_slot (const char *device, const char *key, int
keyslot)
 
   return 0;
 }
+
+char *
+do_luks_uuid (const char *device)
+{
+  const char *argv[MAX_ARGS];
+  size_t i = 0;
+
+  ADD_ARG (argv, i, "cryptsetup");
+  ADD_ARG (argv, i, "luksUUID");
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, NULL);
+
+  char *out = NULL;
+  CLEANUP_FREE char *err = NULL;
+  int r = commandv (&out, &err, (const char * const *) argv);
+
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    return NULL;
+  }
+
+  trim (out);
+
+  return out;
+}
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index 8443ae79e..cb7e8dcd0 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -9728,4 +9728,12 @@ it is useful when you have added a new device or deleted
an
 existing device (such as when the C<guestfs_luks_open> API
 is used)." };
 
+  { defaults with
+    name = "luks_uuid"; added = (1, 41, 9);
+    style = RString (RPlainString, "uuid"), [String (Device,
"device")], [];
+    optional = Some "luks";
+    shortdesc = "get the UUID of a LUKS device";
+    longdesc = "\
+This returns the UUID of the LUKS device C<device>." };
+
 ]
diff --git a/generator/proc_nr.ml b/generator/proc_nr.ml
index efa8c5d21..11a557076 100644
--- a/generator/proc_nr.ml
+++ b/generator/proc_nr.ml
@@ -514,6 +514,7 @@ let proc_nr = [
 504, "part_get_gpt_attributes";
 505, "f2fs_expand";
 506, "lvm_scan";
+507, "luks_uuid";
 ]
 
 (* End of list.  If adding a new entry, add it at the end of the list
diff --git a/lib/MAX_PROC_NR b/lib/MAX_PROC_NR
index 80e3e6eab..055b6671a 100644
--- a/lib/MAX_PROC_NR
+++ b/lib/MAX_PROC_NR
@@ -1 +1 @@
-506
+507
-- 
2.21.0

Pino Toscano

2019-Nov-29 11:20 UTC

head link

[Libguestfs] [common PATCH 1/2] options: rename key.device as key.id

In the future it will be also something else other than the device name.
---
 mltools/tools_utils-c.c | 4 ++--
 options/key-option.pod  | 7 ++++---
 options/keys.c          | 8 ++++----
 options/options.h       | 8 +++++---
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/mltools/tools_utils-c.c b/mltools/tools_utils-c.c
index 3b80091..6c43b8d 100644
--- a/mltools/tools_utils-c.c
+++ b/mltools/tools_utils-c.c
@@ -57,8 +57,8 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv)
     struct key_store_key key;
 
     elemv = Field (keysv, 0);
-    key.device = strdup (String_val (Field (elemv, 0)));
-    if (!key.device)
+    key.id = strdup (String_val (Field (elemv, 0)));
+    if (!key.id)
       caml_raise_out_of_memory ();
 
     v = Field (elemv, 1);
diff --git a/options/key-option.pod b/options/key-option.pod
index cc188dc..02dcf18 100644
--- a/options/key-option.pod
+++ b/options/key-option.pod
@@ -1,15 +1,16 @@
 =item B<--key> SELECTOR
 
 Specify a key for LUKS, to automatically open a LUKS device when using
-the inspection.
+the inspection.  C<ID> must be the libguestfs device name of the LUKS
+device.
 
 =over 4
 
-=item B<--key> C<DEVICE>:key:KEY_STRING
+=item B<--key> C<ID>:key:KEY_STRING
 
 Use the specified C<KEY_STRING> as passphrase.
 
-=item B<--key> C<DEVICE>:file:FILENAME
+=item B<--key> C<ID>:file:FILENAME
 
 Read the passphrase from F<FILENAME>.
 
diff --git a/options/keys.c b/options/keys.c
index 782bdb6..7c391ac 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -148,7 +148,7 @@ get_keys (struct key_store *ks, const char *device)
     for (i = 0; i < ks->nr_keys; ++i) {
       struct key_store_key *key = &ks->keys[i];
 
-      if (STRNEQ (key->device, device))
+      if (STRNEQ (key->id, device))
         continue;
 
       switch (key->type) {
@@ -193,8 +193,8 @@ key_store_add_from_selector (struct key_store *ks, const
char *selector)
   }
 
   /* 1: device */
-  key.device = strdup (fields[0]);
-  if (!key.device)
+  key.id = strdup (fields[0]);
+  if (!key.id)
     error (EXIT_FAILURE, errno, "strdup");
 
   /* 2: key type */
@@ -265,6 +265,6 @@ free_key_store (struct key_store *ks)
       free (key->file.name);
       break;
     }
-    free (key->device);
+    free (key->id);
   }
 }
diff --git a/options/options.h b/options/options.h
index 510e8a8..b83a92b 100644
--- a/options/options.h
+++ b/options/options.h
@@ -104,10 +104,12 @@ struct mp {
 
 /* A key in the key store. */
 struct key_store_key {
-  /* The device this key refers to.  There may be multiple matching
-   * devices in the list.
+  /* An ID for the device this key refers to.  It must be the libguestfs
+   * device name.
+   *
+   * There may be multiple matching devices in the list.
    */
-  char *device;
+  char *id;
 
   enum {
     key_string,             /* key specified as string */
-- 
2.21.0

Pino Toscano

2019-Nov-29 11:20 UTC

head link

[Libguestfs] [common PATCH 2/2] options: allow a UUID as identifier for --key

This way it is possible to specify the UUID of the LUKS device instead
of the libguestfs device name to decrypt a device during the inspection.

Make the usage of the new luks_uuid API conditional, so other projects
using the common submodule do not require a libguestfs version bump.
---
 options/decrypt.c      | 8 +++++++-
 options/key-option.pod | 4 ++--
 options/keys.c         | 4 ++--
 options/options.h      | 6 +++---
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/options/decrypt.c b/options/decrypt.c
index 3511d9f..683cf5e 100644
--- a/options/decrypt.c
+++ b/options/decrypt.c
@@ -86,7 +86,13 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
       char mapname[32];
       make_mapname (partitions[i], mapname, sizeof mapname);
 
-      CLEANUP_FREE_STRING_LIST char **keys = get_keys (ks, partitions[i]);
+#ifdef GUESTFS_HAVE_LUKS_UUID
+      CLEANUP_FREE char *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. */
diff --git a/options/key-option.pod b/options/key-option.pod
index 02dcf18..90a3b15 100644
--- a/options/key-option.pod
+++ b/options/key-option.pod
@@ -1,8 +1,8 @@
 =item B<--key> SELECTOR
 
 Specify a key for LUKS, to automatically open a LUKS device when using
-the inspection.  C<ID> must be the libguestfs device name of the LUKS
-device.
+the inspection.  C<ID> can be either the libguestfs device name, or
+the UUID of the LUKS device.
 
 =over 4
 
diff --git a/options/keys.c b/options/keys.c
index 7c391ac..798315c 100644
--- a/options/keys.c
+++ b/options/keys.c
@@ -126,7 +126,7 @@ read_first_line_from_file (const char *filename)
  * keystore, ask the user.
  */
 char **
-get_keys (struct key_store *ks, const char *device)
+get_keys (struct key_store *ks, const char *device, const char *uuid)
 {
   size_t i, j, len;
   char **r;
@@ -148,7 +148,7 @@ get_keys (struct key_store *ks, const char *device)
     for (i = 0; i < ks->nr_keys; ++i) {
       struct key_store_key *key = &ks->keys[i];
 
-      if (STRNEQ (key->id, device))
+      if (STRNEQ (key->id, device) && (uuid && STRNEQ
(key->id, uuid)))
         continue;
 
       switch (key->type) {
diff --git a/options/options.h b/options/options.h
index b83a92b..9b78302 100644
--- a/options/options.h
+++ b/options/options.h
@@ -104,8 +104,8 @@ struct mp {
 
 /* A key in the key store. */
 struct key_store_key {
-  /* An ID for the device this key refers to.  It must be the libguestfs
-   * device name.
+  /* An ID for the device this key refers to.  It can be either the libguestfs
+   * device name, or the UUID.
    *
    * There may be multiple matching devices in the list.
    */
@@ -150,7 +150,7 @@ extern void print_inspect_prompt (void);
 
 /* in key.c */
 extern char *read_key (const char *param);
-extern char **get_keys (struct key_store *ks, const char *device);
+extern char **get_keys (struct key_store *ks, const char *device, const char
*uuid);
 extern struct key_store *key_store_add_from_selector (struct key_store *ks,
const char *selector);
 extern struct key_store *key_store_import_key (struct key_store *ks, const
struct key_store_key *key);
 extern void free_key_store (struct key_store *ks);
-- 
2.21.0

Pino Toscano

2019-Nov-29 11:21 UTC

head link

[Libguestfs] [PATCH] tests: test --key for a real guest (with inspection)

Test both the decryption by device name, and UUID.
---
 tests/luks/Makefile.am                |  3 +-
 tests/luks/test-key-option-inspect.sh | 48 +++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100755 tests/luks/test-key-option-inspect.sh

diff --git a/tests/luks/Makefile.am b/tests/luks/Makefile.am
index 30c817f87..2330ed72f 100644
--- a/tests/luks/Makefile.am
+++ b/tests/luks/Makefile.am
@@ -20,7 +20,8 @@ include $(top_srcdir)/subdir-rules.mk
 TESTS = \
 	test-luks.sh \
 	test-luks-list.sh \
-	test-key-option.sh
+	test-key-option.sh \
+	test-key-option-inspect.sh
 
 TESTS_ENVIRONMENT = $(top_builddir)/run --test
 
diff --git a/tests/luks/test-key-option-inspect.sh
b/tests/luks/test-key-option-inspect.sh
new file mode 100755
index 000000000..d6abe2531
--- /dev/null
+++ b/tests/luks/test-key-option-inspect.sh
@@ -0,0 +1,48 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2019 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test the --key option.  It is handled by common code so we only need
+# to test one tool (guestfish).
+
+set -e
+
+$TEST_FUNCTIONS
+skip_if_skipped
+skip_unless_feature_available luks
+skip_unless_phony_guest fedora-luks.img
+
+disk=../../test-data/phony-guests/fedora-luks.img
+device=/dev/sda2
+
+# Get the UUID of the LUKS device.
+uuid="$(guestfish --ro -a $disk run : luks-uuid $device)"
+
+# Try to decrypt the disk in different ways:
+# - pass a wrong key via stdin to check the --key value is actually used only
+# - check for /etc/fedora-release as a way to know the LUKS device was
+#   decrypted correctly
+
+# Specify the libguestfs device name of the LUKS device.
+echo wrongkey | guestfish --ro -a $disk -i --keys-from-stdin \
+          --key "$device:key:FEDORA" \
+          exists /etc/fedora-release
+
+# Specify the UUID of the LUKS device.
+echo wrongkey | guestfish --ro -a $disk -i --keys-from-stdin \
+          --key "$uuid:key:FEDORA" \
+          exists /etc/fedora-release
-- 
2.21.0

Richard W.M. Jones

2019-Nov-29 11:35 UTC

head link

Re: [Libguestfs] [PATCH 1/1] New API: luks_uuid

On Fri, Nov 29, 2019 at 12:18:22PM +0100, Pino Toscano
wrote:> Return the UUID of a LUKS device.
> ---
>  daemon/luks.c             | 25 +++++++++++++++++++++++++
>  generator/actions_core.ml |  8 ++++++++
>  generator/proc_nr.ml      |  1 +
>  lib/MAX_PROC_NR           |  2 +-
>  4 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/daemon/luks.c b/daemon/luks.c
> index 5c48a91eb..d631cb100 100644
> --- a/daemon/luks.c
> +++ b/daemon/luks.c
> @@ -292,3 +292,28 @@ do_luks_kill_slot (const char *device, const char
*key, int keyslot)
>  
>    return 0;
>  }
> +
> +char *
> +do_luks_uuid (const char *device)
> +{
> +  const char *argv[MAX_ARGS];
> +  size_t i = 0;
> +
> +  ADD_ARG (argv, i, "cryptsetup");
> +  ADD_ARG (argv, i, "luksUUID");
> +  ADD_ARG (argv, i, device);
> +  ADD_ARG (argv, i, NULL);
Easier than I expected.  I thought we'd have to parse luksDump :-)

ACK

Rich.
> +  char *out = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  int r = commandv (&out, &err, (const char * const *) argv);
> +
> +  if (r == -1) {
> +    reply_with_error ("%s", err);
> +    return NULL;
> +  }
> +
> +  trim (out);
> +
> +  return out;
> +}
> diff --git a/generator/actions_core.ml b/generator/actions_core.ml
> index 8443ae79e..cb7e8dcd0 100644
> --- a/generator/actions_core.ml
> +++ b/generator/actions_core.ml
> @@ -9728,4 +9728,12 @@ it is useful when you have added a new device or
deleted an
>  existing device (such as when the C<guestfs_luks_open> API
>  is used)." };
>  
> +  { defaults with
> +    name = "luks_uuid"; added = (1, 41, 9);
> +    style = RString (RPlainString, "uuid"), [String (Device,
"device")], [];
> +    optional = Some "luks";
> +    shortdesc = "get the UUID of a LUKS device";
> +    longdesc = "\
> +This returns the UUID of the LUKS device C<device>." };
> +
>  ]
> diff --git a/generator/proc_nr.ml b/generator/proc_nr.ml
> index efa8c5d21..11a557076 100644
> --- a/generator/proc_nr.ml
> +++ b/generator/proc_nr.ml
> @@ -514,6 +514,7 @@ let proc_nr = [
>  504, "part_get_gpt_attributes";
>  505, "f2fs_expand";
>  506, "lvm_scan";
> +507, "luks_uuid";
>  ]
>  
>  (* End of list.  If adding a new entry, add it at the end of the list
> diff --git a/lib/MAX_PROC_NR b/lib/MAX_PROC_NR
> index 80e3e6eab..055b6671a 100644
> --- a/lib/MAX_PROC_NR
> +++ b/lib/MAX_PROC_NR
> @@ -1 +1 @@
> -506
> +507
> -- 
> 2.21.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Richard W.M. Jones

2019-Nov-29 11:37 UTC

head link

Re: [Libguestfs] [common PATCH 1/2] options: rename key.device as key.id

On Fri, Nov 29, 2019 at 12:20:03PM +0100, Pino Toscano
wrote:> In the future it will be also something else other than the device name.
> ---
>  mltools/tools_utils-c.c | 4 ++--
>  options/key-option.pod  | 7 ++++---
>  options/keys.c          | 8 ++++----
>  options/options.h       | 8 +++++---
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/mltools/tools_utils-c.c b/mltools/tools_utils-c.c
> index 3b80091..6c43b8d 100644
> --- a/mltools/tools_utils-c.c
> +++ b/mltools/tools_utils-c.c
> @@ -57,8 +57,8 @@ guestfs_int_mllib_inspect_decrypt (value gv, value gpv,
value keysv)
>      struct key_store_key key;
>  
>      elemv = Field (keysv, 0);
> -    key.device = strdup (String_val (Field (elemv, 0)));
> -    if (!key.device)
> +    key.id = strdup (String_val (Field (elemv, 0)));
> +    if (!key.id)
>        caml_raise_out_of_memory ();
>  
>      v = Field (elemv, 1);
> diff --git a/options/key-option.pod b/options/key-option.pod
> index cc188dc..02dcf18 100644
> --- a/options/key-option.pod
> +++ b/options/key-option.pod
> @@ -1,15 +1,16 @@
>  =item B<--key> SELECTOR
>  
>  Specify a key for LUKS, to automatically open a LUKS device when using
> -the inspection.
> +the inspection.  C<ID> must be the libguestfs device name of the
LUKS
> +device.
>  
>  =over 4
>  
> -=item B<--key> C<DEVICE>:key:KEY_STRING
> +=item B<--key> C<ID>:key:KEY_STRING
>  
>  Use the specified C<KEY_STRING> as passphrase.
>  
> -=item B<--key> C<DEVICE>:file:FILENAME
> +=item B<--key> C<ID>:file:FILENAME
>  
>  Read the passphrase from F<FILENAME>.
>  
> diff --git a/options/keys.c b/options/keys.c
> index 782bdb6..7c391ac 100644
> --- a/options/keys.c
> +++ b/options/keys.c
> @@ -148,7 +148,7 @@ get_keys (struct key_store *ks, const char *device)
>      for (i = 0; i < ks->nr_keys; ++i) {
>        struct key_store_key *key = &ks->keys[i];
>  
> -      if (STRNEQ (key->device, device))
> +      if (STRNEQ (key->id, device))
>          continue;
>  
>        switch (key->type) {
> @@ -193,8 +193,8 @@ key_store_add_from_selector (struct key_store *ks,
const char *selector)
>    }
>  
>    /* 1: device */
> -  key.device = strdup (fields[0]);
> -  if (!key.device)
> +  key.id = strdup (fields[0]);
> +  if (!key.id)
>      error (EXIT_FAILURE, errno, "strdup");
>  
>    /* 2: key type */
> @@ -265,6 +265,6 @@ free_key_store (struct key_store *ks)
>        free (key->file.name);
>        break;
>      }
> -    free (key->device);
> +    free (key->id);
>    }
>  }
> diff --git a/options/options.h b/options/options.h
> index 510e8a8..b83a92b 100644
> --- a/options/options.h
> +++ b/options/options.h
> @@ -104,10 +104,12 @@ struct mp {
>  
>  /* A key in the key store. */
>  struct key_store_key {
> -  /* The device this key refers to.  There may be multiple matching
> -   * devices in the list.
> +  /* An ID for the device this key refers to.  It must be the libguestfs
> +   * device name.
> +   *
> +   * There may be multiple matching devices in the list.
>     */
> -  char *device;
> +  char *id;
>  
>    enum {
>      key_string,             /* key specified as string */
> -- 
> 2.21.0
Just a plain "alpha conversion", so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Richard W.M. Jones

2019-Nov-29 11:43 UTC

head link

Re: [Libguestfs] [common PATCH 2/2] options: allow a UUID as identifier for --key

On Fri, Nov 29, 2019 at 12:20:04PM +0100, Pino Toscano
wrote:> This way it is possible to specify the UUID of the LUKS device instead
> of the libguestfs device name to decrypt a device during the inspection.
> 
> Make the usage of the new luks_uuid API conditional, so other projects
> using the common submodule do not require a libguestfs version bump.
> ---
>  options/decrypt.c      | 8 +++++++-
>  options/key-option.pod | 4 ++--
>  options/keys.c         | 4 ++--
>  options/options.h      | 6 +++---
>  4 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 3511d9f..683cf5e 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -86,7 +86,13 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>        char mapname[32];
>        make_mapname (partitions[i], mapname, sizeof mapname);
>  
> -      CLEANUP_FREE_STRING_LIST char **keys = get_keys (ks, partitions[i]);
> +#ifdef GUESTFS_HAVE_LUKS_UUID
Interesting that this is needed.  I suppose because virt-v2v can be
compiled with libguestfs back to 1.40, and it uses the common module
so it would fail to build without this new API being present.  I
hadn't anticipated this.
> +      CLEANUP_FREE char *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. */
> diff --git a/options/key-option.pod b/options/key-option.pod
> index 02dcf18..90a3b15 100644
> --- a/options/key-option.pod
> +++ b/options/key-option.pod
> @@ -1,8 +1,8 @@
>  =item B<--key> SELECTOR
>  
>  Specify a key for LUKS, to automatically open a LUKS device when using
> -the inspection.  C<ID> must be the libguestfs device name of the
LUKS
> -device.
> +the inspection.  C<ID> can be either the libguestfs device name, or
> +the UUID of the LUKS device.
>  
>  =over 4
>  
> diff --git a/options/keys.c b/options/keys.c
> index 7c391ac..798315c 100644
> --- a/options/keys.c
> +++ b/options/keys.c
> @@ -126,7 +126,7 @@ read_first_line_from_file (const char *filename)
>   * keystore, ask the user.
>   */
>  char **
> -get_keys (struct key_store *ks, const char *device)
> +get_keys (struct key_store *ks, const char *device, const char *uuid)
>  {
>    size_t i, j, len;
>    char **r;
> @@ -148,7 +148,7 @@ get_keys (struct key_store *ks, const char *device)
>      for (i = 0; i < ks->nr_keys; ++i) {
>        struct key_store_key *key = &ks->keys[i];
>  
> -      if (STRNEQ (key->id, device))
> +      if (STRNEQ (key->id, device) && (uuid && STRNEQ
(key->id, uuid)))
>          continue;
>  
>        switch (key->type) {
> diff --git a/options/options.h b/options/options.h
> index b83a92b..9b78302 100644
> --- a/options/options.h
> +++ b/options/options.h
> @@ -104,8 +104,8 @@ struct mp {
>  
>  /* A key in the key store. */
>  struct key_store_key {
> -  /* An ID for the device this key refers to.  It must be the libguestfs
> -   * device name.
> +  /* An ID for the device this key refers to.  It can be either the
libguestfs
> +   * device name, or the UUID.
>     *
>     * There may be multiple matching devices in the list.
>     */
> @@ -150,7 +150,7 @@ extern void print_inspect_prompt (void);
>  
>  /* in key.c */
>  extern char *read_key (const char *param);
> -extern char **get_keys (struct key_store *ks, const char *device);
> +extern char **get_keys (struct key_store *ks, const char *device, const
char *uuid);
>  extern struct key_store *key_store_add_from_selector (struct key_store
*ks, const char *selector);
>  extern struct key_store *key_store_import_key (struct key_store *ks, const
struct key_store_key *key);
>  extern void free_key_store (struct key_store *ks);
Looks fine, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Richard W.M. Jones

2019-Nov-29 11:47 UTC

head link

Re: [Libguestfs] [PATCH] tests: test --key for a real guest (with inspection)

On Fri, Nov 29, 2019 at 12:21:57PM +0100, Pino Toscano
wrote:> Test both the decryption by device name, and UUID.
> ---
>  tests/luks/Makefile.am                |  3 +-
>  tests/luks/test-key-option-inspect.sh | 48 +++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
>  create mode 100755 tests/luks/test-key-option-inspect.sh
> 
> diff --git a/tests/luks/Makefile.am b/tests/luks/Makefile.am
> index 30c817f87..2330ed72f 100644
> --- a/tests/luks/Makefile.am
> +++ b/tests/luks/Makefile.am
> @@ -20,7 +20,8 @@ include $(top_srcdir)/subdir-rules.mk
>  TESTS = \
>  	test-luks.sh \
>  	test-luks-list.sh \
> -	test-key-option.sh
> +	test-key-option.sh \
> +	test-key-option-inspect.sh
>  
>  TESTS_ENVIRONMENT = $(top_builddir)/run --test
>  
> diff --git a/tests/luks/test-key-option-inspect.sh
b/tests/luks/test-key-option-inspect.sh
> new file mode 100755
> index 000000000..d6abe2531
> --- /dev/null
> +++ b/tests/luks/test-key-option-inspect.sh
> @@ -0,0 +1,48 @@
> +#!/bin/bash -
> +# libguestfs
> +# Copyright (C) 2019 Red Hat Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
> +
> +# Test the --key option.  It is handled by common code so we only need
> +# to test one tool (guestfish).
> +
> +set -e
> +
> +$TEST_FUNCTIONS
> +skip_if_skipped
> +skip_unless_feature_available luks
> +skip_unless_phony_guest fedora-luks.img
> +
> +disk=../../test-data/phony-guests/fedora-luks.img
> +device=/dev/sda2
> +
> +# Get the UUID of the LUKS device.
> +uuid="$(guestfish --ro -a $disk run : luks-uuid $device)"
> +
> +# Try to decrypt the disk in different ways:
> +# - pass a wrong key via stdin to check the --key value is actually used
only
> +# - check for /etc/fedora-release as a way to know the LUKS device was
> +#   decrypted correctly
> +
> +# Specify the libguestfs device name of the LUKS device.
> +echo wrongkey | guestfish --ro -a $disk -i --keys-from-stdin \
> +          --key "$device:key:FEDORA" \
> +          exists /etc/fedora-release
> +
> +# Specify the UUID of the LUKS device.
> +echo wrongkey | guestfish --ro -a $disk -i --keys-from-stdin \
> +          --key "$uuid:key:FEDORA" \
> +          exists /etc/fedora-release
> -- 
> 2.21.0
ACK series.

In the cover letter you said:
> A test for this (patch #4) can be applied only when the patches for
> common are applied, and the submodule updated.
What I have done in similar situations is to combine the patch which
depends on the update to common/ (which I guess is this one) with the
submodule updated commit hash.  Example:

https://github.com/libguestfs/virt-v2v/commit/51c839c77f83fae87c38ee1883ddc52c3dc1e4ad

This means that pulling in the patch automatically updates the
submodule (at least, sort of, submodules are still a pain).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Possibly Parallel Threads

Search for more maybe matching threads

Libguestfs - Nov 2019 - [common PATCH 1/2] options: rename key.device as key.id

[Libguestfs] [PATCH 0/1] Allow UUIDs for --key identifiers.

[Libguestfs] [PATCH 1/1] New API: luks_uuid

[Libguestfs] [common PATCH 1/2] options: rename key.device as key.id

[Libguestfs] [common PATCH 2/2] options: allow a UUID as identifier for --key

[Libguestfs] [PATCH] tests: test --key for a real guest (with inspection)

Re: [Libguestfs] [PATCH 1/1] New API: luks_uuid

Re: [Libguestfs] [common PATCH 1/2] options: rename key.device as key.id

Re: [Libguestfs] [common PATCH 2/2] options: allow a UUID as identifier for --key

Re: [Libguestfs] [PATCH] tests: test --key for a real guest (with inspection)

Possibly Parallel Threads