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

[Laszlo Ersek]

On 07/01/22 11:56, Laszlo Ersek wrote:

> This code runs (in virt-v2v, for example) on the following call path:
> 
>   main                                       [v2v/v2v.ml]
>     create_standard_options                  [common/mltools/tools_utils.ml]
>     Getopt.parse
>       parse_key_selector                     [common/mltools/tools_utils.ml]
>         List.push_back
> 
>     (* at this point, cmdline_options.ks
>      * has been populated
>      *)
> 
>     convert                                  [convert/convert.ml]
>       (* skipped in virt-v2v, because it
>        * enables networking anyway:
>        *)
>       key_store_requires_network             [common/mltools/tools_utils.ml]
> 
>       g#set_network
>       g#launch
> 
>       inspect_decrypt                        [common/mltools/tools_utils.ml]
>         c_inspect_decrypt                    [common/mltools/tools_utils.ml]
>           guestfs_int_mllib_inspect_decrypt  [common/mltools/tools_utils-c.c]
> 
>             loop over cmdline_options.ks:
>               key_store_import_key           [common/options/keys.c]
> 
>             inspect_do_decrypt               [common/options/decrypt.c]
>               decrypt_mountables             [common/options/decrypt.c]
>                 MACRO CHECK HERE
>                   guestfs_clevis_luks_unlock
>             free_key_store                   [common/options/keys.c]
> 
> This is why I said "it's too late to report this error in
> decrypt_mountables()"!
> 
> In comparison, here's how the C tools work (such as virt-cat):
> 
>   main                                   [cat/cat.c]
>     OPTION_key                           [common/options/options.h]
>       key_store_add_from_selector        [common/options/keys.c]
>         key_store_import_key             [common/options/keys.c]
> 
>     /* "ks" has been populated */
> 
>     key_store_requires_network           [common/options/keys.c]
>     guestfs_set_network
>     guestfs_launch
>     inspect_mount                        [common/options/options.h]
>       inspect_mount_handle               [common/options/inspect.c]
>         inspect_do_decrypt               [common/options/decrypt.c]
>           decrypt_mountables             [common/options/decrypt.c]
>             MACRO CHECK HERE
>               guestfs_clevis_luks_unlock
>     free_key_store                       [common/options/keys.c]
> 
> Note the following important facts:
> 
> (1) The macro check is reached, in both kinds of tools, way after the
> options have been parsed, and quite a bit after the appliance has been
> launched.
> 
> (2) The OCaml and C *option parsers* share *nothing* related to keys.
> Consider:
> 
> - OCaml does not call key_store_add_from_selector() at all,
> 
> - while OCaml does call key_store_import_key(), it does so *outside* of
> option parsing. Namely, "parse_key_selector" records the command line
> options in an OCaml-only data structure (cmdline_options.ks). That is
> then translated to an *ephemeral* C-language keystore much later, with
> key_store_import_key() -- only in guestfs_int_mllib_inspect_decrypt()!
> That is, after the appliance is running.
> 
> - Correspondingly: in the C tools, the C-language keystore is freed
> *after* inspection completes (that is, free_key_store() is called
> *after* inspect_mount() returns), but in OCaml, the ephemeral keystore
> is released (correctly) *inside* guestfs_int_mllib_inspect_decrypt() --
> that is, internally to the outer function "inspect_decrypt".
> 
> 
> If we want to catch the lack of GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK during
> option parsing, then I need to implement a brand new C function, and
> wrap it for OCaml too (OCaml cannot check the C-language macro
> GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK).
> 
> For this: because we already have "key_store_requires_network" (same
> name, but totally separate impl in C and OCaml), I'd have to rename that
> function (both impls) to "key_store_contains_clevis", and use it for two
> purposes:
> 
> - right after option parsing, if the key store contains clevis, but the
> feature test macro is missing, bail.
> 
> - just before launching the appliance, if the key store contains clevis,
> enable networking.

There could be an easier solution for this BTW. Namely:

- For the C language tools, extend key_store_add_from_selector() with
the GUESTFS_HAVE_CLEVIS_LUKS_UNLOCK macro check. (Review the call tree
to see what that's right.) This would be a small modification to

- For the OCaml language tools, extend "parse_key_selector" with the
macro check. For that, I'll add a very small helper function.

Both of these changes only affect patch "options, mltools/tools_utils:
parse "--key ID:clevis" options".

Regarding the present patch, I'll keep the macro check, but replace the
error message with abort() -- as it should never be reached.

I'll post v3, hopefully soon.

Thanks!
Laszlo