[Libguestfs] [PATCH 2/2] options: Allow multiple --key parameters and default keys.

[Pino Toscano]

On Saturday, 16 November 2019 09:33:21 CET Richard W.M. Jones wrote:
> On Fri, Nov 15, 2019 at 03:23:02PM +0100, Pino Toscano wrote:
> > On Tuesday, 12 November 2019 19:35:12 CET Richard W.M. Jones wrote:
> > > This allows multiple --key parameters on the command line to match a
> > > single device.  This could either be specified as:
> > > 
> > >   tool --key /dev/sda1:key:trykey1 --key /dev/sda1:key:trykey2
> > > 
> > > which would try "trykey1" and "trykey2" against /dev/sda1.
> > 
> > This seems OK for me, so you can attempt multiple keys for a device.
> > 
> > > And/or you can specify default keys which are tried against each
> > > device (after more specific keys fail), eg:
> > > 
> > >   tool --key :key:defaultkey1 --key :key:defaultkey2
> > > 
> > > which would try "defaultkey1" and "defaultkey2" against all devices.
> > 
> > However I do not see the point in this: IMHO you better make it explicit
> > which key is used for a certain device.
> 
> It's considerably more convenient for callers if they don't have to
> store this mapping between opaque libguestfs device names (a concept
> which is meaningless to most users and to the software which
> orchestrates virt-v2v at scale), and keys.

We use devices already for various things, including partitions and
(in this case) devices for keys. If you use libguestfs tools (virt-v2v
included) then you need to care about them, so "callers are dumb" is
not one of the best arguments you can come up with.

> Also there's no actual penalty to making this feature available, it's
> a natural extension which falls out from the implementation, and it
> doesn't affect performance unless the caller adds multiple --key
> parameters.  It's also how LUKS itself works if you enable
> decrypt_keyctl in crypttab.

This is not about performance or how LUKS works (which both are
irrelevant for libguestfs users), but rather about explicitly specifying
a secret only to its target.

> > Also, this makes it possible so
> > in case of two similar guests like:
> > - /dev/sda1 with key "key1", and /dev/sda2 with key "key2"
> > - /dev/sda1 with key "key2", and /dev/sda2 with key "key1"
> > the above command like will work in the same way, with no indication of
> > which key was successfully used for which device -- so you can silently
> > swap the first guest for the second with no changes to the v2v command
> > line...
> 
> I didn't understand this.  It seems an unlikely scenario in the first
> place, and has no ill effects even if it does occur.

I disagree with your assessment. Let's say that the first guest gets a
new encypted partition with coincidentally the same key as /dev/sda1:
I do not want that with a wildcard --key that device is silently opened
without the user acting and explicitly saying "this is the key for this
device" -- even if it is the same as another device.

> 
> > What's the use case for this?
> 
> So that IMS doesn't have to collect an exact mapping between opaque
> libguestfs device names and keys, which will require some user to have
> to enter device names that they don't understand into a UI without any
> making typos rather than just supplying a list of keys that apply to a
> single guest.

Few things here:
- if the UI does not help the user insert the proper mapping between
  devices (which cloudforms already knows) and keys, then it's a
  problem of the UI
- users can make typos even with your solution
- if the keys are provided without the UI in a machine-parseable format
  already, it means the user is bypassing the UI and choosing to
  "go low-level" on they own already

> > Please split this patch in two:
> > - multiple keys for the same device
> > - keys for all devices

This still stands.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20191118/85e45757/attachment.sig>