Re: [Qemu-block] [PATCH v2 12/17] qcow2: convert QCow2 to use QCryptoBlock for encryption

[Fam Zheng]

On Thu, 01/21 10:50, Daniel P. Berrange wrote:
> On Thu, Jan 21, 2016 at 05:54:23PM +0800, Fam Zheng wrote:
> > On Wed, 01/20 17:38, Daniel P. Berrange wrote:
> > > This converts the qcow2 driver to make use of the QCryptoBlock
> > > APIs for encrypting image content. As well as continued support
> > > for the legacy QCow2 encryption format, the appealing benefit
> > > is that it enables support for the LUKS format inside qcow2.
> > 
> > FWIW, with today's QEMU, it's possible to stack format drivers on top of 
> > each
> > other.  In other words, even without this patch, we can make LUKS driver
> > encrypt/decrypt the qcow2 payload, while keeping them completely orthogonal.
> 
> Yep, that is certainly possible, and it is what is intended for using
> LUKS with RBD, iSCSI, & other network drivers.
> 
> I think there is value in having LUKS integrated directly into qcow2
> though. It means that given a qcow2 file you can 100% reliably
> distinguish between a file created with the intention of QEMU managing
> the LUKS encryption, from a file where the guest OS happens to have
> set up LUKS encryption in its virtual disk. If you don't have this,
> then given a random qcow2 file, you have to probe to see if LUKS is
> present or not. Given the security issues we've had in the past with
> raw images being turned into qcow2 images by a malicious guest writing
> a qcow2 header, I feel that having explicitly integration LUKS support
> in QCow is worthwhile as a concept.

Yes, I'm not objecting to explicit (read: mandatory) encryption in qcow2 in any
way, and extending the format spec for LUKS is a good thing.

I mentioned stacked drivers because I wonder how good we can do in reusing
block/crypto.c code to achieve this (to save 500+ new code in qcow2). For
example I have a rough idea along this:

* In qcow2 spec, define type "2" of crypt_method for LUKS encryption, and state
  that if this method is used, the payload must be LUKS format as defined in
  cryptsetup's docs/on-disk-format.pdf, and driver will take care of
  encrypting/decrypting data in a way that is transparent to guest.

* In qcow2_open, if header's crypt_method is LUKS, set a special flag in the
  BlockDriverState to indicate that block layer should create an overlay crypto
  driver (which will be luks in this case), on top of this BDS. To do this we
  need some modification to bdrv_open.

* When qcow2_open is done, block layer will then instantiate a LUKS driver,
  which backed by the qcow2 BDS (as the BDS.file). This LUKS BDS will then be
  attached to guest device.

* From the guest PoV, R/W reqs are served as if it's an ordinary qcow2; from
  LUKS driver's PoV, it works properly formatted luks data, which is backed by
  whatever it doesn't care, be it an iscsi target, rbd block, local file, or
  a qcow2 image that has metadata; from the qcow2 driver's PoV, it only made
  sure that a LUKS driver covers itself, at instantiation time.  Everything
  else operates the same way as a non-encrypted qcow2.

* Of course, qcow2_create would need some similar changes to the block layer as
  to bdrv_open, but that shouldn't be too hard.

Makes sense?

Fam