Re: [Qemu-devel] The unholy encrypted image key mess

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 05.03.2014 um 09:15 hat Markus Armbruster geschrieben:
>> Eric Blake <address@hidden> writes:
>> > Uggh - so there's no current way to hot-plug a device in state GOTKEY
>> > short of using a two-command sequence?  It would be nicer if hot-plug
>> > had a way to fail to add encrypted devices unless the user also passes
>> > the password at the same time, creating the device directly into the
>> > GOTKEY state.
>> 
>> I can't see why QMP commands would ever want to create in state NEEDKEY.
>> We could easily avoid it there: give QMP commands creating
>> BlockDriverStates an optional password parameter, fail the command if
>> the BDS is encrypted and the password parameter is missing.
>
> Yes.
>
>> For HMP, we need to make up our minds how to do passwords.
>> 
>> The current way is to tie NEEDKEY to "guest paused".  I hate that.
>> 
>> Another way is to make the commands adding BDS prompt for necessary
>> passwords.  We still have to deal with state NEEDKEY while we're waiting
>> for the user's reply.  Need to take care to hide the new BDS.  Create it
>> anonymous, and publish it only after setting the key?
>
> HMP is just a QMP user, so if QMP never creates images in a NEEDKEY
> state, HMP doesn't either. And that's fine: Let QMP return an error
> ("this needs a password and you didn't specify one") and then ask the
> user for a password and retry.
>
> Solves the whole problem with the NEEDKEY state by eliminating it. I'm
> not entirely sure if NEEDKEY is the only state of not fully initialised
> BDSes, but perhaps it really is.

The qmp command needs to be fixed to truly fail when a password is
missing, not create a BDS in state NEEDKEY and fail.  But I think we
want that change anyway, incompatible or not.

>> We'd have to do the same for the command line, of course.
>
> This one could become a bit trickier because you'd have to ask for the
> password not only before you let the VM run, but even before you create
> the virtual disk devices.

Yes.  Ask right away, and right on stdout/stdin.  Improvement for human
users, because then they don't have to hunt for the monitor (which may
not be on stdout/stdin).

Possible show-stopper for tools.  Right now, the way to start a guest
with encrypted images is with -S, supply all passwords via QMP, then
cont.

Not sure how to best reconcile the needs of humans and tools here.

>> Incompatible change, but since this stuff doesn't really work and really
>> shouldn't be used...
>
> I'm not even sure if it is incompatible on the external interface. HMP
> would be similar enough, and QMP already returns errors for encrypted
> disks in qmp_change_blockdev() and has no pre-2.0 interface for
> hotplugging disks.
>
> Oh right, that error is a non-error and the block device is created
> anyway. This is stupid, we'd have to make an incompatible change to
> change this into a real error.

Yes.

> We also need to make sure that blockdev-add fails on encrypted images if
> no password is given, this may still be missing.

Yes.