Re: [Qemu-devel] [PATCH v9 13/20] qcow2: add support for LUKS encryption format

2017-06-23 Thread Daniel P. Berrange 
On Wed, Jun 21, 2017 at 04:59:02PM +0200, Max Reitz wrote:
> On 2017-06-21 16:46, Max Reitz wrote:
> > On 2017-06-21 16:42, Max Reitz wrote:
> >> On 2017-06-19 19:34, Daniel P. Berrange wrote:
> >>> This adds support for using LUKS as an encryption format
> >>> with the qcow2 file, using the new encrypt.format parameter
> >>> to request "luks" format. e.g.
> >>>
> >>>   # qemu-img create --object secret,data=123456,id=sec0 \
> >>>-f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 \
> >>>test.qcow2 10G
> >>>
> >>> The legacy "encryption=on" parameter still results in
> >>> creation of the old qcow2 AES format (and is equivalent
> >>> to the new 'encryption-format=aes'). e.g. the following are
> >>> equivalent:
> >>>
> >>>   # qemu-img create --object secret,data=123456,id=sec0 \
> >>>-f qcow2 -o encryption=on,encrypt.key-secret=sec0 \
> >>>test.qcow2 10G
> >>>
> >>>  # qemu-img create --object secret,data=123456,id=sec0 \
> >>>-f qcow2 -o encryption-format=aes,encrypt.key-secret=sec0 \
> >>>test.qcow2 10G
> >>>
> >>> With the LUKS format it is necessary to store the LUKS
> >>> partition header and key material in the QCow2 file. This
> >>> data can be many MB in size, so cannot go into the QCow2
> >>> header region directly. Thus the spec defines a FDE
> >>> (Full Disk Encryption) header extension that specifies
> >>> the offset of a set of clusters to hold the FDE headers,
> >>> as well as the length of that region. The LUKS header is
> >>> thus stored in these extra allocated clusters before the
> >>> main image payload.
> >>>
> >>> Aside from all the cryptographic differences implied by
> >>> use of the LUKS format, there is one further key difference
> >>> between the use of legacy AES and LUKS encryption in qcow2.
> >>> For LUKS, the initialiazation vectors are generated using
> >>> the host physical sector as the input, rather than the
> >>> guest virtual sector. This guarantees unique initialization
> >>> vectors for all sectors when qcow2 internal snapshots are
> >>> used, thus giving stronger protection against watermarking
> >>> attacks.
> >>>
> >>> Reviewed-by: Eric Blake 
> >>> Reviewed-by: Alberto Garcia 
> >>> Signed-off-by: Daniel P. Berrange 
> >>> ---
> >>>  block/qcow2-cluster.c  |   4 +-
> >>>  block/qcow2-refcount.c |  10 ++
> >>>  block/qcow2.c  | 268 
> >>> ++--
> >>>  block/qcow2.h  |   9 ++
> >>>  qapi/block-core.json   |   5 +-
> >>>  tests/qemu-iotests/082.out | 270 
> >>> -
> >>>  6 files changed, 478 insertions(+), 88 deletions(-)
> >>
> >> Reviewed-by: Max Reitz 
> > 
> > (But due to the split of do_perform_cow(), this will need the same
> > changes that patch 10 and 11 will need (in this case only contextual, in
> > the cases of 10 and 11 there are also functional changes required).)
> 
> (Turns out it will need a functional change, too, because
> do_perform_cow_encrypt() doesn't take the host offset yet.)

Yep, I've just rebased to kevin/block and resolved these problems


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 :|

Re: [Qemu-devel] [PATCH v9 13/20] qcow2: add support for LUKS encryption format

2017-06-21 Thread Max Reitz 
On 2017-06-21 16:46, Max Reitz wrote:
> On 2017-06-21 16:42, Max Reitz wrote:
>> On 2017-06-19 19:34, Daniel P. Berrange wrote:
>>> This adds support for using LUKS as an encryption format
>>> with the qcow2 file, using the new encrypt.format parameter
>>> to request "luks" format. e.g.
>>>
>>>   # qemu-img create --object secret,data=123456,id=sec0 \
>>>-f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 \
>>>test.qcow2 10G
>>>
>>> The legacy "encryption=on" parameter still results in
>>> creation of the old qcow2 AES format (and is equivalent
>>> to the new 'encryption-format=aes'). e.g. the following are
>>> equivalent:
>>>
>>>   # qemu-img create --object secret,data=123456,id=sec0 \
>>>-f qcow2 -o encryption=on,encrypt.key-secret=sec0 \
>>>test.qcow2 10G
>>>
>>>  # qemu-img create --object secret,data=123456,id=sec0 \
>>>-f qcow2 -o encryption-format=aes,encrypt.key-secret=sec0 \
>>>test.qcow2 10G
>>>
>>> With the LUKS format it is necessary to store the LUKS
>>> partition header and key material in the QCow2 file. This
>>> data can be many MB in size, so cannot go into the QCow2
>>> header region directly. Thus the spec defines a FDE
>>> (Full Disk Encryption) header extension that specifies
>>> the offset of a set of clusters to hold the FDE headers,
>>> as well as the length of that region. The LUKS header is
>>> thus stored in these extra allocated clusters before the
>>> main image payload.
>>>
>>> Aside from all the cryptographic differences implied by
>>> use of the LUKS format, there is one further key difference
>>> between the use of legacy AES and LUKS encryption in qcow2.
>>> For LUKS, the initialiazation vectors are generated using
>>> the host physical sector as the input, rather than the
>>> guest virtual sector. This guarantees unique initialization
>>> vectors for all sectors when qcow2 internal snapshots are
>>> used, thus giving stronger protection against watermarking
>>> attacks.
>>>
>>> Reviewed-by: Eric Blake 
>>> Reviewed-by: Alberto Garcia 
>>> Signed-off-by: Daniel P. Berrange 
>>> ---
>>>  block/qcow2-cluster.c  |   4 +-
>>>  block/qcow2-refcount.c |  10 ++
>>>  block/qcow2.c  | 268 
>>> ++--
>>>  block/qcow2.h  |   9 ++
>>>  qapi/block-core.json   |   5 +-
>>>  tests/qemu-iotests/082.out | 270 
>>> -
>>>  6 files changed, 478 insertions(+), 88 deletions(-)
>>
>> Reviewed-by: Max Reitz 
> 
> (But due to the split of do_perform_cow(), this will need the same
> changes that patch 10 and 11 will need (in this case only contextual, in
> the cases of 10 and 11 there are also functional changes required).)

(Turns out it will need a functional change, too, because
do_perform_cow_encrypt() doesn't take the host offset yet.)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 13/20] qcow2: add support for LUKS encryption format

2017-06-21 Thread Max Reitz 
On 2017-06-21 16:42, Max Reitz wrote:
> On 2017-06-19 19:34, Daniel P. Berrange wrote:
>> This adds support for using LUKS as an encryption format
>> with the qcow2 file, using the new encrypt.format parameter
>> to request "luks" format. e.g.
>>
>>   # qemu-img create --object secret,data=123456,id=sec0 \
>>-f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 \
>>test.qcow2 10G
>>
>> The legacy "encryption=on" parameter still results in
>> creation of the old qcow2 AES format (and is equivalent
>> to the new 'encryption-format=aes'). e.g. the following are
>> equivalent:
>>
>>   # qemu-img create --object secret,data=123456,id=sec0 \
>>-f qcow2 -o encryption=on,encrypt.key-secret=sec0 \
>>test.qcow2 10G
>>
>>  # qemu-img create --object secret,data=123456,id=sec0 \
>>-f qcow2 -o encryption-format=aes,encrypt.key-secret=sec0 \
>>test.qcow2 10G
>>
>> With the LUKS format it is necessary to store the LUKS
>> partition header and key material in the QCow2 file. This
>> data can be many MB in size, so cannot go into the QCow2
>> header region directly. Thus the spec defines a FDE
>> (Full Disk Encryption) header extension that specifies
>> the offset of a set of clusters to hold the FDE headers,
>> as well as the length of that region. The LUKS header is
>> thus stored in these extra allocated clusters before the
>> main image payload.
>>
>> Aside from all the cryptographic differences implied by
>> use of the LUKS format, there is one further key difference
>> between the use of legacy AES and LUKS encryption in qcow2.
>> For LUKS, the initialiazation vectors are generated using
>> the host physical sector as the input, rather than the
>> guest virtual sector. This guarantees unique initialization
>> vectors for all sectors when qcow2 internal snapshots are
>> used, thus giving stronger protection against watermarking
>> attacks.
>>
>> Reviewed-by: Eric Blake 
>> Reviewed-by: Alberto Garcia 
>> Signed-off-by: Daniel P. Berrange 
>> ---
>>  block/qcow2-cluster.c  |   4 +-
>>  block/qcow2-refcount.c |  10 ++
>>  block/qcow2.c  | 268 
>> ++--
>>  block/qcow2.h  |   9 ++
>>  qapi/block-core.json   |   5 +-
>>  tests/qemu-iotests/082.out | 270 
>> -
>>  6 files changed, 478 insertions(+), 88 deletions(-)
> 
> Reviewed-by: Max Reitz 

(But due to the split of do_perform_cow(), this will need the same
changes that patch 10 and 11 will need (in this case only contextual, in
the cases of 10 and 11 there are also functional changes required).)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 13/20] qcow2: add support for LUKS encryption format

2017-06-21 Thread Max Reitz 
On 2017-06-19 19:34, Daniel P. Berrange wrote:
> This adds support for using LUKS as an encryption format
> with the qcow2 file, using the new encrypt.format parameter
> to request "luks" format. e.g.
> 
>   # qemu-img create --object secret,data=123456,id=sec0 \
>-f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 \
>test.qcow2 10G
> 
> The legacy "encryption=on" parameter still results in
> creation of the old qcow2 AES format (and is equivalent
> to the new 'encryption-format=aes'). e.g. the following are
> equivalent:
> 
>   # qemu-img create --object secret,data=123456,id=sec0 \
>-f qcow2 -o encryption=on,encrypt.key-secret=sec0 \
>test.qcow2 10G
> 
>  # qemu-img create --object secret,data=123456,id=sec0 \
>-f qcow2 -o encryption-format=aes,encrypt.key-secret=sec0 \
>test.qcow2 10G
> 
> With the LUKS format it is necessary to store the LUKS
> partition header and key material in the QCow2 file. This
> data can be many MB in size, so cannot go into the QCow2
> header region directly. Thus the spec defines a FDE
> (Full Disk Encryption) header extension that specifies
> the offset of a set of clusters to hold the FDE headers,
> as well as the length of that region. The LUKS header is
> thus stored in these extra allocated clusters before the
> main image payload.
> 
> Aside from all the cryptographic differences implied by
> use of the LUKS format, there is one further key difference
> between the use of legacy AES and LUKS encryption in qcow2.
> For LUKS, the initialiazation vectors are generated using
> the host physical sector as the input, rather than the
> guest virtual sector. This guarantees unique initialization
> vectors for all sectors when qcow2 internal snapshots are
> used, thus giving stronger protection against watermarking
> attacks.
> 
> Reviewed-by: Eric Blake 
> Reviewed-by: Alberto Garcia 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/qcow2-cluster.c  |   4 +-
>  block/qcow2-refcount.c |  10 ++
>  block/qcow2.c  | 268 ++--
>  block/qcow2.h  |   9 ++
>  qapi/block-core.json   |   5 +-
>  tests/qemu-iotests/082.out | 270 
> -
>  6 files changed, 478 insertions(+), 88 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v9 13/20] qcow2: add support for LUKS encryption format

2017-06-19 Thread Daniel P. Berrange 
This adds support for using LUKS as an encryption format
with the qcow2 file, using the new encrypt.format parameter
to request "luks" format. e.g.

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encrypt.format=luks,encrypt.key-secret=sec0 \
   test.qcow2 10G

The legacy "encryption=on" parameter still results in
creation of the old qcow2 AES format (and is equivalent
to the new 'encryption-format=aes'). e.g. the following are
equivalent:

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption=on,encrypt.key-secret=sec0 \
   test.qcow2 10G

 # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption-format=aes,encrypt.key-secret=sec0 \
   test.qcow2 10G

With the LUKS format it is necessary to store the LUKS
partition header and key material in the QCow2 file. This
data can be many MB in size, so cannot go into the QCow2
header region directly. Thus the spec defines a FDE
(Full Disk Encryption) header extension that specifies
the offset of a set of clusters to hold the FDE headers,
as well as the length of that region. The LUKS header is
thus stored in these extra allocated clusters before the
main image payload.

Aside from all the cryptographic differences implied by
use of the LUKS format, there is one further key difference
between the use of legacy AES and LUKS encryption in qcow2.
For LUKS, the initialiazation vectors are generated using
the host physical sector as the input, rather than the
guest virtual sector. This guarantees unique initialization
vectors for all sectors when qcow2 internal snapshots are
used, thus giving stronger protection against watermarking
attacks.

Reviewed-by: Eric Blake 
Reviewed-by: Alberto Garcia 
Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c  |   4 +-
 block/qcow2-refcount.c |  10 ++
 block/qcow2.c  | 268 ++--
 block/qcow2.h  |   9 ++
 qapi/block-core.json   |   5 +-
 tests/qemu-iotests/082.out | 270 -
 6 files changed, 478 insertions(+), 88 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c4a256d..edec6a7 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -395,7 +395,9 @@ static int coroutine_fn do_perform_cow(BlockDriverState *bs,
 
 if (bs->encrypted) {
 Error *err = NULL;
-int64_t sector = (src_cluster_offset + offset_in_cluster)
+int64_t sector = (s->crypt_physical_offset ?
+  (cluster_offset + offset_in_cluster) :
+  (src_cluster_offset + offset_in_cluster))
  >> BDRV_SECTOR_BITS;
 assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
 assert((bytes & ~BDRV_SECTOR_MASK) == 0);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7c06061..81c22e6 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1856,6 +1856,16 @@ static int calculate_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 return ret;
 }
 
+/* encryption */
+if (s->crypto_header.length) {
+ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
+s->crypto_header.offset,
+s->crypto_header.length);
+if (ret < 0) {
+return ret;
+}
+}
+
 return check_refblocks(bs, res, fix, rebuild, refcount_table, nb_clusters);
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 9edd0a0..bb6e7c1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -66,6 +66,7 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -80,6 +81,86 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 }
 
 
+static ssize_t qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset,
+  uint8_t *buf, size_t buflen,
+  void *opaque, Error **errp)
+{
+BlockDriverState *bs = opaque;
+BDRVQcow2State *s = bs->opaque;
+ssize_t ret;
+
+if ((offset + buflen) > s->crypto_header.length) {
+error_setg(errp, "Request for data outside of extension header");
+return -1;
+}
+
+ret = bdrv_pread(bs->file,
+ s->crypto_header.offset + offset, buf, buflen);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Could not read encryption header");
+return -1;
+}
+return ret;
+}
+
+
+static ssize_t qcow2_crypto_hdr_init_func(QCryptoBlock *block, size_t 
headerlen,
+  void *opaque, Error **errp)
+{
+