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

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> Il 05/03/2014 09:24, Markus Armbruster ha scritto:
>> Paolo Bonzini <address@hidden> writes:
>>
>>> Il 28/02/2014 23:08, Eric Blake ha scritto:
>>>> Use the fact that we are calling the next release "2.0" to actually kill
>>>> qemu disk encryption as a horribly botched feature, on the grounds that
>>>> we are doing users a favor by not letting them use broken encryption?
>>>
>>> Only for qemu, of course---qemu-img would still have support for
>>> encryption, and there's no reason to risk stability by removing all
>>> the monitor code right now.
>>
>> Right now = for 2.0?
>
> Possibly:
>
> diff --git a/block.c b/block.c
> index 38bbdf3..794946c 100644
> --- a/block.c
> +++ b/block.c
> @@ -1384,6 +1384,12 @@ done:
>      }
>      QDECREF(options);
>
> +    if (bdrv_key_required(bs) && use_bdrv_whitelist) {
> +        ret = -EINVAL;
> +        error_setg(errp, "Encrypted images are not supported by QEMU "
> +                   "anymore.\nPlease use qemu-img to unencrypt them.");
> +        goto close_nad_fail;
> +    }
>      if (!bdrv_key_required(bs)) {
>          bdrv_dev_change_media_cb(bs, true);
>      }

Okay, I guess that's simple enough to be considered for 2.0.

>> I'm not trying to push anything into 2.0.  I'm trying to clean up
>> another mess (qerror, to be precise), and the encrypted images mess is
>> in my way.  I don't expect to complete the qerror job in time for 2.0.
>
> What are you cleaning up exactly?

I want to get rid of qerror_report() & friends.

qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP.  It should not be used elsewhere.
However, uses have crept into many places, and they keep creeping.

Ever since we added the latest error infrastructure, we planned to get
rid of qerror_report() "eventually".  I'm working on turning
"eventually" into "actually".  Got stuck in the messy code dealing with
encryption keys, and had to figure out how it actually works.  Where
"works" is strictly in the sense of "what it does", because what it does
doesn't actually work, at least not reliably.

>>> However, wouldn't we have the same problems even with a sane encrypted
>>> image format (based on LUKS, for example)?
>>
>> Yes, and when we get that, we'll shoehorn it into the same idiotic user
>> interface we have now :)
>>
>>> Let's just open bugs (oh if only Launchpad supported tracker bugs) for now.
>>
>> Filing bugs won't help me with cleaning up qerror.  If preserving the
>> current idiotic encrypted image interfaces is required, I'll have no
>> choice but pour in the necessary work.  Sure wish I could use the time
>> for something more immediately useful than preserving an idiotic
>> interface to a feature we don't want anybody to use.
>
> I'm not sure why it helps to kill an idiotic user interface, if we (a)
> have plans to reimplement the same feature in the future (b) don't
> have any ideas on how to have a non-idiotic user interface for the
> same feature.

I think this thread has proven (b) false.  We may not have a complete
plan, but what we have is certainly more than not having any ideas.

> Fixing it, at least partly---that helps.  But killing it doesn't help,
> unless we drop all plans to have sane image encryption.

Fixing helps if the current state can be evolved into the desired state,
and reaching the desired state by evolution from the current state is a
reasonably efficient way to get there.

Killing helps if the current state is a dead end.

I think as far as interfaces are concerned, we should start over.  The
existing interfaces are too badly designed to serve as a basis.

Once we have sane interfaces designed, we can find out whether the
existing code can be productively evolved to implement them, and what it
would take to preserve the existing idiotic interfaces for backward
compatibility.  Then decide whether we want to do either.

Doing all this work feels pointless to me without a use case, and a real
encryption machinery to interface to.

Before we work on the latter, I'd like to see some of the former.  LUKS
works in guests, too.