[Libguestfs] [libguestfs-common PATCH v2 06/12] options: add back-end for LUKS decryption with Clevis+Tang
Richard W.M. Jones
rjones at redhat.com
Thu Jun 30 16:10:03 UTC 2022
On Thu, Jun 30, 2022 at 02:20:22PM +0200, Laszlo Ersek wrote:
> 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 at redhat.com>
> ---
>
> Notes:
> v2:
>
> - check GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK; error out if unavailable [Rich]
>
> v1 notes were:
>
> 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 | 13 ++++++++++++-
> options/keys.c | 7 ++++++-
> 3 files changed, 24 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..97c8b88d16aa 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -155,11 +155,22 @@ 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)
> +#ifdef GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK
> + r = guestfs_clevis_luks_unlock (g, mountable, mapname);
> +#else
> + error (EXIT_FAILURE, 0,
> + _("'clevis_luks_unlock', needed for decrypting %s, is "
> + "unavailable in this libguestfs version"), mountable);
To be clear on why this is correct, it's because this code is only
used during option parsing in command line tools, where we want to
exit when an error happens.
> +#endif
> + 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 *
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
More information about the Libguestfs
mailing list