Re: [PATCH 02/13] qcrypto-luks: implement encryption key management

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	Maxim Levitsky <mlevitsk@redhat.com>,
	Max Reitz <mreitz@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH 02/13] qcrypto-luks: implement encryption key management
Date: Thu, 6 Feb 2020 13:49:18 +0000	[thread overview]
Message-ID: <20200206134918.GM2391707@redhat.com> (raw)
In-Reply-To: <87blqbu8xe.fsf@dusky.pond.sub.org>

On Thu, Feb 06, 2020 at 02:44:45PM +0100, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > Kevin Wolf <kwolf@redhat.com> writes:
> >
> >> Am 05.02.2020 um 11:03 hat Markus Armbruster geschrieben:
> >>> Kevin Wolf <kwolf@redhat.com> writes:
> [...]
> >>> > Adding a key gets more complicated with your proposed interface because
> >>> > state must be set explicitly now whereas before it was derived
> >>> > automatically from the fact that if you give a key, only active makes
> >>> > sense.
> >>> 
> >>> The explicitness could be viewed as an improvement :)
> >>
> >> Not really. I mean, I really know to appreciate the advantages of
> >> -blockdev where needed, but usually I don't want to type all that stuff
> >> for the most common tasks. qemu-img amend is similar.
> >>
> >> For deleting, I might actually agree that explicitness is an
> >> improvement, but for creating it's just unnecessary verbosity.
> >>
> >>> If you'd prefer implicit here: Max has patches for making union tags
> >>> optional with a default.  They'd let you default active to true.
> >>
> >> I guess this would improve the usability in this case.
> 
> Thinking and writing in the "Making QEMU easier for management tools and
> applications" monster thread have made me realize we're mixing up two
> aspects that ought to be kept separate: machine-friendly QMP and
> human-friendly CLI.
> 
> You argue that
> 
>     $ qemu-img amend -o encrypt.keys.0.new-secret=sec0 test.qcow2
> 
> is nicer than
> 
>     $ qemu-img amend -o encrypt.keys.0.state=active,encrypt.keys.0.secret=sec0 test.qcow2
> 
> and you do have a point: humans want their CLI terse.  Redundancy is
> unwanted, except perhaps to protect users from dangerous accidents.  In
> this example, state=active is redundant when a secret is given, because
> anything else would be an error.
> 
> In QMP, however, we like things simple and explicit, and we eschew
> magic.
> 
> This particular magic might just be simple enough to be acceptable in
> QMP.  We'd "merely" have to support explicit defaults in the schema (a
> clear improvement if you ask me), and optional union tags (tolerable as
> long as the default comes from the schema, I guess).
> 
> My point is: QAPI schema design *must* focus on QMP and nothing else.
> If we try to serve both QMP and human-friendly CLI, we'll likely botch
> both.
> 
> I believe a truly human-friendly CLI requires more than just
> human-friendly concrete syntax for QMP.  Same as HMP, really.

A human-friendly approach to this problem would never even
have the generic "amend" design IMHO. Friendly would be to
have a CLI that is approx the same as "cryptsetup" provides
eg

    $ qemu-img add-key /path/to/disk
    enter key>..
    re-enter key>...

or

    qemu-img add-key --keyfile /some/file.txt /path/to/disk

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|