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

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

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

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/51c839c77f83fae87c38ee1883ddc5... 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/