Re: [PATCH v2 09/15] encryption: Add <cipher> and <ivgen> to encryption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/24/2016 10:10 AM, Peter Krempa wrote:
> On Thu, Jun 23, 2016 at 13:29:05 -0400, John Ferlan wrote:
>> For a luks device, allow the configuration of a specific cipher to be
>> used for encrypting the volume.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>>  docs/formatstorageencryption.html.in               |  81 +++++++++++++-
>>  docs/schemas/storagecommon.rng                     |  44 +++++++-
>>  src/conf/domain_conf.c                             |  11 ++
>>  src/util/virstorageencryption.c                    | 124 +++++++++++++++++++++
>>  src/util/virstorageencryption.h                    |  13 +++
>>  .../qemuxml2argv-luks-disk-cipher.xml              |  41 +++++++
>>  .../qemuxml2xmlout-luks-disk-cipher.xml            |  45 ++++++++
>>  tests/qemuxml2xmltest.c                            |   1 +
>>  tests/storagevolxml2xmlin/vol-luks-cipher.xml      |  23 ++++
>>  tests/storagevolxml2xmlout/vol-luks-cipher.xml     |  23 ++++
>>  tests/storagevolxml2xmltest.c                      |   1 +
>>  11 files changed, 401 insertions(+), 6 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml
>>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml
>>  create mode 100644 tests/storagevolxml2xmlin/vol-luks-cipher.xml
>>  create mode 100644 tests/storagevolxml2xmlout/vol-luks-cipher.xml
>>
>> diff --git a/docs/formatstorageencryption.html.in b/docs/formatstorageencryption.html.in
>> index 3a08192..80111e3 100644
>> --- a/docs/formatstorageencryption.html.in
>> +++ b/docs/formatstorageencryption.html.in
>> @@ -71,6 +71,58 @@
>>        be used as the passphrase to decrypt the volume.
>>        <span class="since">Since 2.0.0</span>.
>>      </p>
>> +    <p>
>> +      For volume creation, it is possible to specify the encryption
>> +      algorithm used to encrypt the luks volume. The following two
>> +      optional elements may be provided for that purpose. It is hypervisor
>> +      dependent as to which algorithms are supported. The default algorithm
>> +      for QEMU is 'aes-256-cbc' using 'essiv' for initialization vector
> 
> At this point this is the default not for qemu, but for the 'DIR'
> backend for the storage driver that is using qemu-img.
> 

Does this wording suffice:

   The default algorithm
   used by the storage driver backend when using qemu-img to create
   the volume is 'aes-256-cbc' using 'essiv' for initialization vector

>> +      generation and 'sha256' hash algorithm for both the cipher and the
>> +      initialization vector generation.
>> +    </p>
>> +
>> +    <dl>
>> +      <dt><code>cipher</code></dt>
>> +      <dd>This element describes the cipher algorithm to be used to either
>> +          encrypt or decrypt the luks volume. This element has the following
>> +          attributes:
>> +          <dl>
>> +            <dt><code>name</code></dt>
>> +            <dd>The name of the cipher algorithm used for data encryption,
>> +            such as 'aes', 'des', 'cast5', 'serpent', 'twofish', etc.
>> +            Support of the specific algorithm is hypervisor dependent.</dd>
> 
> This is storage driver implementation dependent.
> 

OK

>> +            <dt><code>size</code></dt>
>> +            <dd>The size of the cipher in bits, such as '256', '192', '128',
>> +            etc. Support of the specific size for a specific cipher is
>> +            hypervisor dependent.</dd>
>> +            <dt><code>mode</code></dt>
>> +            <dd>An optional cipher algorithm mode such as 'cbc', 'xts',
>> +            'ecb', etc. Support of the specific cipher mode is
>> +            hypervisor dependent.</dd>
>> +            <dt><code>hash</code></dt>
>> +            <dd>An optional master key hash algorithm such as 'md5', 'sha1',
>> +            'sha256', etc. Support of the specific hash algorithm is
>> +            hypervisor dependent.</dd>
>> +          </dl>
>> +      </dd>
>> +      <dt><code>ivgen</code></dt>
>> +      <dd>This optional element describes the initialization vector
>> +          generation algorithm used in conjunction with the
>> +          <code>cipher</code>. If the <code>cipher</code> is not provided,
>> +          then an error will be generated by the parser.
>> +          <dl>
>> +            <dt><code>name</code></dt>
>> +            <dd>The name of the algorithm, such as 'plain', 'plain64',
>> +            'essiv', etc. Support of the specific algorithm is hypervisor
>> +            dependent.</dd>
>> +            <dt><code>hash</code></dt>
>> +            <dd>An optional hash algorithm such as 'md5', 'sha1', 'sha256',
>> +            etc. Support of the specific ivgen hash algorithm is hypervisor
>> +            dependent.</dd>
>> +          </dl>
>> +      </dd>
>> +    </dl>
>> +
>>  
>>      <h2><a name="example">Examples</a></h2>
>>  
>> @@ -84,9 +136,12 @@
>>        &lt;/encryption&gt;</pre>
>>  
>>      <p>
>> -      Here is a simple example, specifying use of the <code>luks</code> format
>> -      where it's assumed that a <code>secret</code> has been defined using a
>> -      <code>usage</code> element with a <code>id</code> of "luks_example":
>> +      Assuming a <a href="formatsecret.html#luksUsageType">
>> +      <code>luks secret</code></a> is already defined using a
>> +      <code>usage</code> element with an <code>id</code> of "luks_example",
> 
> s/id/name/
> 

Right.


>> +      a simple example specifying use of the <code>luks</code> format
>> +      for either volume creation without a specific cipher being defined or
>> +      as part of a domain volume definition:
>>      </p>
>>      <pre>
>>        &lt;encryption format='luks'&gt;
>> @@ -94,5 +149,25 @@
>>        &lt;/encryption&gt;
>>      </pre>
>>  
>> +    <p>
>> +      Here is an example, specifying use of the <code>luks</code> format for
>> +      a specific cipher algorihm for volume creation:
>> +    </p>
>> +    <pre>
>> +      &lt;volume&gt;
>> +        &lt;name&gt;twofish.luks&lt;/name&gt;
>> +        &lt;capacity unit='G'&gt;5&lt;/capacity&gt;
>> +        &lt;target&gt;
>> +          &lt;path&gt;/var/lib/libvirt/images/demo.luks&lt;/path&gt;
>> +          &lt;format type='luks'/&gt;
>> +          &lt;encryption format='luks'&gt;
>> +             &lt;secret type='passphrase' usage='luks_example'/&gt;
>> +             &lt;cipher name='twofish' size='256' mode='cbc' hash='sha256'/&gt;
>> +             &lt;ivgen name='plain64' hash='sha256'/&gt;
>> +          &lt;/encryption&gt;
>> +        &lt;/target&gt;
>> +      &lt;/volume&gt;
>> +    </pre>
>> +
>>    </body>
>>  </html>
> 
> [...]
> 
>> diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h
>> index 5e1be3b..34bba03 100644
>> --- a/src/util/virstorageencryption.h
>> +++ b/src/util/virstorageencryption.h
>> @@ -44,6 +44,16 @@ struct _virStorageEncryptionSecret {
>>      virSecretLookupTypeDef seclookupdef;
>>  };
>>  
>> +/* For a key type it's possible to dictate the cipher and if necessary iv */
>> +typedef struct _virStorageEncryptionInfoDef virStorageEncryptionInfoDef;
>> +typedef virStorageEncryptionInfoDef *virStorageEncryptionInfoDefPtr;
>> +struct _virStorageEncryptionInfoDef {
>> +    unsigned int size;
>> +    char *name;
>> +    char *mode;
>> +    char *hash;
> 
> I'd either store the IV generator info here or add a different
> structure. It seems weird to reuse it just because it has two fields
> with same name.
> 

Geez and I thought it was more useful to reuse code...

I'll adjust though, no big deal.  It's all one larger happy structure now.

> 
>> +};
>> +
>>  typedef enum {
>>      /* "default" is only valid for volume creation */
>>      VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
> 
> [...]
> 
>>  virStorageEncryptionPtr virStorageEncryptionCopy(const virStorageEncryption *src)
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml
>> new file mode 100644
>> index 0000000..00399cf
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml
> 
> [...]
> 
>> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml
>> new file mode 100644
>> index 0000000..9ce15c0
>> --- /dev/null
>> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml
> 
> Again the output file is not really needed:
> $ diff -u qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml
> --- qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml	2016-06-24 15:55:42.059454563 +0200
> +++ qemuxml2xmloutdata/qemuxml2xmlout-luks-disk-cipher.xml	2016-06-24 15:55:42.059454563 +0200
> @@ -32,10 +32,14 @@
>        </encryption>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
>      </disk>
> -    <controller type='usb' index='0'/>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
>      <controller type='pci' index='0' model='pci-root'/>
>      <input type='mouse' bus='ps2'/>
>      <input type='keyboard' bus='ps2'/>
> -    <memballoon model='virtio'/>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </memballoon>
>    </devices>
>  </domain>
> 
> 
> Difference to the test files added in previous patch makes this test
> even less useful:
> 
> $ diff -u qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml qemuxml2argvdata/qemuxml2argv-luks-disks.xml
> $
> 
> 

OK same as before... Fixed

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]