Re: [Qemu-devel] [PATCH v4 13/26] crypto: implement the LUKS block encryption format

[Daniel P. Berrange]

On Fri, Mar 11, 2016 at 03:31:59PM -0700, Eric Blake wrote:
> On 02/29/2016 05:00 AM, Daniel P. Berrange wrote:
> > Provide a block encryption implementation that follows the
> > LUKS/dm-crypt specification.
> > 
> > This supports all combinations of hash, cipher algorithm,
> > cipher mode and iv generator that are implemented by the
> > current crypto layer.
> > 
> > The notable missing feature is support for the 'xts'
> > cipher mode, which is commonly used for disk encryption
> > instead of 'cbc'. This is because it is not provided by
> > either nettle or libgcrypt. A suitable implementation
> > will be identified & integrated later.
> 
> Stale paragraph, you implemented it earlier in the series.
> 
> > 
> > There is support for opening existing volumes formatted
> > by dm-crypt, and for formatting new volumes. In the latter
> > case it will only use key slot 0.
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> 
> 
> > +static int
> > +qcrypto_block_luks_open(QCryptoBlock *block,
> > +                        QCryptoBlockOpenOptions *options,
> > +                        QCryptoBlockReadFunc readfunc,
> > +                        void *opaque,
> > +                        unsigned int flags,
> > +                        Error **errp)
> > +{
> 
> > +    /* Read the entire LUKS header, minus the key material from
> > +     * the underling device */
> 
> s/underling/underlying/ (although the typo does read rather humorously -
> I now have a mental image of a LUKS overlord :)
> 
> 
> > +++ b/qapi/crypto.json
> > @@ -117,12 +117,13 @@
> 
> >  ##
> >  # QCryptoBlockOptionsBase:
> > @@ -143,7 +144,8 @@
> >  # The options that apply to QCow/QCow2 AES-CBC encryption format
> >  #
> >  # @key-secret: #optional the ID of a QCryptoSecret object providing the
> > -#              decryption key
> > +#              decryption key. Mandatory except when probing image for
> > +#              metadata only.
> 
> Aha - I think this hunk may belong earlier in the series...

Yes, it does belong in the previous patch.

> 
> >  #
> >  # Since: 2.6
> >  ##
> > @@ -151,6 +153,45 @@
> >    'data': { '*key-secret': 'str' }}
> >  
> >  ##
> > +# QCryptoBlockOptionsLUKS:
> > +#
> > +# The options that apply to LUKS encryption format
> > +#
> > +# @key-secret: #optional the ID of a QCryptoSecret object providing the
> > +#              decryption key
> 
> ...Although you may want to duplicate it here.

Yep, will do.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|