Re: [Libguestfs] [libguestfs PATCH 2/3] introduce the "clevis_luks_unlock" API

On 06/28/22 16:57, Richard W.M. Jones wrote: > On Tue, Jun 28, 2022 at 01:54:17PM +0200, Laszlo Ersek wrote: >> Introduce a new guestfs API called "clevis_luks_unlock". At the libguestfs >> level, it is quite simple; it wraps the "clevis luks unlock" guest command >> (implemented by the "clevis-luks-unlock" executable, which is in fact a >> shell script). >> >> The complexity is instead in the network-based disk encryption >> (Clevis/Tang) scheme. Useful documentation: >> >> - https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/... >> - https://github.com/latchset/clevis#clevis >> - https://github.com/latchset/tang#tang >> >> The package providing "clevis-luks-unlock" is usually called >> "clevis-luks", occasionally "clevis". Some distros don't package clevis at >> all. Add the new API under a new option group (which may not be available) >> called "clevisluks". >> >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453 >> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; >> --- >> daemon/Makefile.am | 1 + >> generator/actions_core.ml | 38 +++++++++++++ >> generator/proc_nr.ml | 1 + >> daemon/clevis-luks.c | 58 ++++++++++++++++++++ >> appliance/packagelist.in | 4 ++ >> lib/MAX_PROC_NR | 2 +- >> lib/guestfs.pod | 19 +++++-- >> 7 files changed, 118 insertions(+), 5 deletions(-) >> >> diff --git a/daemon/Makefile.am b/daemon/Makefile.am >> index bbd49f9ea1e0..f50faecd6930 100644 >> --- a/daemon/Makefile.am >> +++ b/daemon/Makefile.am >> @@ -96,10 +96,11 @@ guestfsd_SOURCES = \ >> btrfs.c \ >> caml-stubs.c \ >> cap.c \ >> checksum.c \ >> cleanups.c \ >> + clevis-luks.c \ >> cmp.c \ >> command.c \ >> command.h \ >> compress.c \ >> copy.c \ >> diff --git a/generator/actions_core.ml b/generator/actions_core.ml >> index 807150615426..1b1fdcf77209 100644 >> --- a/generator/actions_core.ml >> +++ b/generator/actions_core.ml >> @@ -9718,6 +9718,44 @@ and I<not> the name of the underlying block device." }; >> style = RErr, [String (Pathname, "dir"); String (FileOut, "filename")], []; >> visibility = VInternal; >> shortdesc = "read directories entries"; >> longdesc = "Internal function for readdir." }; >> >> + { defaults with >> + name = "clevis_luks_unlock"; added = (1, 49, 3); >> + style = RErr, >> + [String (Device, "device"); String (PlainString, "mapname")], > > PlainString because we create the device from the mapname, we're not > passing (this) device in as a parameter, so this is correct. > > However do we never need any optional parameters? cryptsetup_open has > [OBool "readonly"; OString "crypttype"]. Right, I evaluated those (and some more) questions in <https://bugzilla.redhat.com/show_bug.cgi?id=1809453#c13>;: - guestfs_cryptsetup_open() permits "crypttype" = bitlk, but that makes no sense with clevis (no BitLocker, just LUKS, with Clevis -- I read this in the Clevis documentation) - guestfs_cryptsetup_open() permits "readonly" = 1, but read-only mappings are not supported by clevis-luks-unlock(1).

>> + []; >> + optional = Some "clevisluks"; >> + test_excuse = "needs networking and a configured Tang server"; >> + shortdesc = "open an encrypted LUKS block device with Clevis and Tang"; >> + longdesc = "\ >> +This command opens a block device that has been encrypted according to the Linux >> +Unified Key Setup (LUKS) standard, using network-bound disk encryption (NBDE). >> + >> +C<device> is the encrypted block device. >> + >> +The appliance will connect to the Tang servers noted in the tree of Clevis pins >> +that is bound to a keyslot of the LUKS header. The Clevis pin tree may comprise >> +C<sss> (redudancy) pins as internal nodes (optionally), and C<tang> pins as >> +leaves. C<tpm2> pins are not supported. The appliance unlocks the encrypted >> +block device by combining responses from the Tang servers with metadata from the >> +LUKS header; there is no C<key> parameter. >> + >> +This command will fail if networking has not been enabled for the appliance. >> +Refer to C<guestfs_set_network>. >> + >> +The command creates a new block device called F</dev/mapper/mapname>. Reads and >> +writes to this block device are decrypted from and encrypted to the underlying >> +C<device> respectively. Close the decrypted block device with >> +C<guestfs_cryptsetup_close>. >> + >> +C<mapname> cannot be C<\"control\"> because that name is reserved by >> +device-mapper. >> + >> +If this block device contains LVM volume groups, then calling >> +C<guestfs_lvm_scan> with the C<activate> parameter C<true> will make them >> +visible. >> + >> +Use C<guestfs_list_dm_devices> to list all device mapper devices." }; > > Some lines here are longer than 80 characters, No, the longest line (excluding the \n in the source code) is exactly 80 characters long. Is that a problem as well? > which might cause > problems. Be better to stick to a bit less than 80 chars. I've noticed that the pod files use shorter lines, but couldn't figure out if it was 72 chars, 64 chars, or something else. They are not all consistent.