[libvirt] [PATCH] storage: Add capability to use LUKS encryption for disk backend

[John Ferlan]

On 05/29/2018 07:02 AM, Peter Krempa wrote:
> On Thu, May 24, 2018 at 19:50:09 -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1560946
>>
>> Following the model of the Logical backend, use qemu-img on
>> the created device to set up for LUKS encryption.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>
>>  works much better with the settle patch applied from:
>>
>>   https://www.redhat.com/archives/libvir-list/2018-May/msg01847.html
>>
>>
>>  src/storage/storage_backend_disk.c | 43 ++++++++++++++++++++++++--------------
>>  1 file changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
>> index 7b4549c34d..a3003fd0b5 100644
>> --- a/src/storage/storage_backend_disk.c
>> +++ b/src/storage/storage_backend_disk.c
> 
> I must say that I'm not a fan of adding features to the 'disk' backend.
> Using the disk backend is borderline insane for managing disk
> partitions.
> 
> [...]
> 

So in your opinion, does that mean the above bug should just closed? I
don't want to assume anything about anyone's sanity ;-). Even though to
a degree I agree as I found this particularly odd to create/pass a
device/partition to qemu-img. In a way I was surprised it worked, but
then again since the Logical back-end also allows encryption of a single
volume - doing so for disk volumes would appear to be similar, albeit
strange.

Depending on configuration it may not stand the test of time (e.g.
reboot) either especially if your infrastructure changes - you'd have to
recreate the secret for a different /dev/sdXN.


>> @@ -893,6 +887,12 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
>>          goto cleanup;
>>      }
>>  
>> +    /* If we're going to encrypt using LUKS, then we could need up to
>> +     * an extra 2MB for the LUKS header - so account for that now */
>> +    if (vol->target.encryption &&
>> +        vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS)
>> +        endOffset += 2 * 1024 * 1024;
> 
> 
> I don't think it's a good idea to change 'endOffset' after calling
> virStorageBackendDiskPartBoundaries as the function is looking up space
> in the existing partition table. With this if the size is just right and
> you increase it afterwards the partition will not fit in the place found
> by that function.
> 

Oh right, sigh. Brain cramp. If this were to be done, then need to
adjust vol->target.capacity before virStorageBackendDiskPartBoundaries

John


>> +
>>      virCommandAddArgFormat(cmd, "%lluB", startOffset);
>>      virCommandAddArgFormat(cmd, "%lluB", endOffset);
>>