Re: [Libguestfs] [libguestfs-common PATCH 07/12] options: add back-end for LUKS decryption with Clevis+Tang

Extend the "matching_key" structure with a new boolean field, "clevis". "clevis" is mutually exclusive with a non-NULL "passphrase" field. If "clevis" is set, open the LUKS device with guestfs_clevis_luks_unlock() -- which requires no explicit passphrase --; otherwise, continue calling guestfs_cryptsetup_open(). This patch introduces no change in observable behavior; there is no user interface yet for setting the "clevis" field to "true". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: This patch introduces a call to guestfs_clevis_luks_unlock(), which is a new libguestfs API (introduced in the sibling libguestfs series). Assuming we still care about building guestfs-tools and virt-v2v against an independent libguestfs (library and appliance), we need to do one of the following things: (1) Make the call dependent on the GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK feature test macro. If it is missing, the library is too old, just set "r = -1", and (possibly) print a warning to stderr.

(2) Cut a new libguestfs release, and bump the minimum version in "m4/guestfs-libraries.m4" (in both guestfs-tools and virt-v2v) to the new version. Both of these have drawbacks. Disadvantages of (1): - Printing a raw warning to stderr is OK for the C-language tools, but the code is used by OCaml tools as well, which have pretty-printed warnings. - Simply skipping the guestfs_clevis_luks_unlock() call -- i.e., pretending it fails -- is not user-friendly. In particular, once we add the new selector type for "--key" later in this series, and document it in "options/key-option.pod", all the tools' docs will pick it up at once, even if the library is too old to provide the interface.

Disadvantages of (2): - We may not want to leap over a large version distance in "m4/guestfs-libraries.m4" (in both guestfs-tools and virt-v2v), for whatever reason. - Even if we make the guestfs_clevis_luks_unlock() API mandatory in the library, the daemon may not implement it -- it's ultimately appliance (host distro) dependent, which is why the API belongs to a new option group called "clevisluks". That is, the actual behavior of guestfs_clevis_luks_unlock() may still differ from what the user expects based on "options/key-option.pod". This drawback is mitigated however: the documentation of the new selector in "options/key-option.pod" references "ENCRYPTED DISKS" in guestfs(3), which in turn references "guestfs_clevis_luks_unlock", which does highlight the "clevisluks" option group. I vote (2) -- cut a new libguestfs release, then bump the "m4/guestfs-libraries.m4" requirements. I'm not doing that just yet in those projects (in the sibling series here): if I did that, those series would not compile; the libguestfs version number would have to be increased first! And I don't know if we want *something else* in the new libguestfs release, additionally.

options/options.h | 6 ++++++ options/decrypt.c | 7 ++++++- options/keys.c | 7 ++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/options/options.h b/options/options.h index 9bd812525d8a..61a385da13ae 100644 --- a/options/options.h +++ b/options/options.h @@ -136,10 +136,16 @@ struct key_store { /* A key matching a particular ID (pathname of the libguestfs device node that * stands for the encrypted block device, or LUKS UUID). */ struct matching_key { + /* True iff the passphrase should be reconstructed using Clevis, talking to + * Tang servers over the network. + */ + bool clevis; + + /* Explicit passphrase, otherwise. */ char *passphrase; }; /* in config.c */ extern void parse_config (void); diff --git a/options/decrypt.c b/options/decrypt.c index 421a38c2a11f..820fbc5629cd 100644 --- a/options/decrypt.c +++ b/options/decrypt.c @@ -155,11 +155,16 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables, for (scan = 0; scan < nr_matches; ++scan) { struct matching_key *key = keys + scan; int r; guestfs_push_error_handler (g, NULL, NULL); - r = guestfs_cryptsetup_open (g, mountable, key->passphrase, mapname, -1); + assert (key->clevis == (key->passphrase == NULL)); + if (key->clevis) + r = guestfs_clevis_luks_unlock (g, mountable, mapname); + else + r = guestfs_cryptsetup_open (g, mountable, key->passphrase, mapname, + -1); guestfs_pop_error_handler (g); if (r == 0) break; } diff --git a/options/keys.c b/options/keys.c index 56fca17a94b5..75c659561c52 100644 --- a/options/keys.c +++ b/options/keys.c @@ -159,15 +159,17 @@ get_keys (struct key_store *ks, const char *device, const char *uuid, switch (key->type) { case key_string: s = strdup (key->string.s); if (!s) error (EXIT_FAILURE, errno, "strdup"); + match->clevis = false; match->passphrase = s; ++match; break; case key_file: s = read_first_line_from_file (key->file.name); + match->clevis = false; match->passphrase = s; ++match; break; } } @@ -176,10 +178,11 @@ get_keys (struct key_store *ks, const char *device, const char *uuid, if (match == r) { /* Key not found in the key store, ask the user for it. */ s = read_key (device); if (!s) error (EXIT_FAILURE, 0, _("could not read key from user")); + match->clevis = false; match->passphrase = s; ++match; } *nr_matches = (size_t)(match - r); @@ -192,11 +195,13 @@ free_keys (struct matching_key *keys, size_t nr_matches) size_t i; for (i = 0; i < nr_matches; ++i) { struct matching_key *key = keys + i; - free (key->passphrase); + assert (key->clevis == (key->passphrase == NULL)); + if (!key->clevis) + free (key->passphrase); } free (keys); } struct key_store *