Re: [libvirt] [PATCH 17/19] storage: Add support to create a luks volume

2016-06-21 Thread John Ferlan 


On 06/21/2016 09:53 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:56 -0400, John Ferlan wrote:
>> If the volume xml was looking to create a luks volume take the necessary
>> steps in order to make that happen.
>>
>> The processing will be:
>>  1. create a temporary file in the storage pool target path
>>1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
>>depending on how the encryption xml specified fetching the secret
>>1b. create/open the file, initially setting mode to 0600 with current
>>effective uid:gid as owner
>>1c. fetch the secret into a buffer and write that into the file
>>1d. change file protections to 0400
>>
>>  2. create a secret object
>>2a. use a similarly crafted alias to the file name
>>2b. add the file to the object
>>
>>  3. create/add luks options to the commandline
>>3a. at the very least a "key-secret" using the secret object alias
>>3b. if found in the XML the various "cipher" and "ivgen" options
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/libvirt_private.syms   |   1 +
>>  src/storage/storage_backend.c  | 285 
>> ++---
>>  src/storage/storage_backend.h  |   3 +-
>>  src/util/virqemu.c |  23 
>>  src/util/virqemu.h |   6 +
>>  tests/storagevolxml2argvtest.c |   3 +-
>>  6 files changed, 300 insertions(+), 21 deletions(-)
>>

[...]

>> +
>>  static int
>>  virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>>  {
>> @@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char 
>> *qemuimg)
>>   * out what else we have */
>>  int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
>>  
>> -/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
>> - * understands. Since we still support QEMU 0.12 and newer, we need
>> - * to be able to handle the previous format as can be set via a
>> - * compat=0.10 option. */
>> -if (virStorageBackendQemuImgSupportsCompat(qemuimg))
>> -ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>> +/* QEMU 2.6 added support for luks - let's check for that.
>> + * If available, then we can also assume OPTIONS_COMPAT is present */
>> +if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
>> +ret = QEMU_IMG_FORMAT_LUKS;
>> +} else {
>> +/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
>> + * understands. Since we still support QEMU 0.12 and newer, we need
>> + * to be able to handle the previous format as can be set via a
>> + * compat=0.10 option. */
>> +if (virStorageBackendQemuImgSupportsCompat(qemuimg))
>> +ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>> +}
> 
> This looks like it's becoming a source of technical debt. Heaps of
> comments aren't helping.
> 

It's on the short list of things to deal with.

>>  
>>  return ret;
>>  }
>> @@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
>>  const char *inputPath;
>>  const char *inputFormatStr;
>>  int inputFormat;
>> +
>> +char *secretAlias;
>> +const char *secretPath;
>>  };
>>  

[...]

>>  
>> +/* Create a hopefully unique enough name to be used for both the
>> + * secretPath and secretAlias generation
> 
> This won't fly. There's only one way to guarantee that there won't be a
> collision. You need to create a file in a private path.
> 
>> + */
>> +static char *
>> +virStorageBackendCreateQemuImgLuksName(const char *volname,
>> +   virStorageEncryptionPtr enc)
>> +{
>> +char *ret;
>> +
>> +if (enc->secrets[0]->secdef.type == VIR_SECRET_LOOKUP_TYPE_UUID)
>> +ignore_value(virAsprintf(, "%s_%s", volname,
>> + enc->secrets[0]->secdef.u.uuid));
>> +else
>> +ignore_value(virAsprintf(, "%s_%s", volname,
>> + enc->secrets[0]->secdef.u.usage));
> 
> usage is user provided text. Also your example in previous patch uses
> slashes in it. I don't think it will end up well in this case.
> 

Ugh... right.  I'm almost tempted to avoid a file type of secret and
make it be an AES type secret.

This goes away anyway.

>> +return ret;
>> +}
>> +

[...]

>> +static char *
>> +virStorageBackendCreateQemuImgSecretPath(virConnectPtr conn,
>> + virStoragePoolObjPtr pool,
>> + virStorageVolDefPtr vol)
>> +{
>> +virStorageEncryptionPtr enc = vol->target.encryption;
>> +char *str = NULL;
>> +char *secretPath = NULL;
>> +int fd = -1;
>> +uint8_t *secret = NULL;
>> +size_t secretlen = 0;
>> +
>> +if (!enc) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +   _("missing encryption description"));
>> +return NULL;
>> +}
>> +
>> +if (!conn || !conn->secretDriver ||
>> +

Re: [libvirt] [PATCH 17/19] storage: Add support to create a luks volume

2016-06-21 Thread Peter Krempa 
On Mon, Jun 13, 2016 at 20:27:56 -0400, John Ferlan wrote:
> If the volume xml was looking to create a luks volume take the necessary
> steps in order to make that happen.
> 
> The processing will be:
>  1. create a temporary file in the storage pool target path
>1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
>depending on how the encryption xml specified fetching the secret
>1b. create/open the file, initially setting mode to 0600 with current
>effective uid:gid as owner
>1c. fetch the secret into a buffer and write that into the file
>1d. change file protections to 0400
> 
>  2. create a secret object
>2a. use a similarly crafted alias to the file name
>2b. add the file to the object
> 
>  3. create/add luks options to the commandline
>3a. at the very least a "key-secret" using the secret object alias
>3b. if found in the XML the various "cipher" and "ivgen" options
> 
> Signed-off-by: John Ferlan 
> ---
>  src/libvirt_private.syms   |   1 +
>  src/storage/storage_backend.c  | 285 
> ++---
>  src/storage/storage_backend.h  |   3 +-
>  src/util/virqemu.c |  23 
>  src/util/virqemu.h |   6 +
>  tests/storagevolxml2argvtest.c |   3 +-
>  6 files changed, 300 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fdf06ae..0d6d080 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2153,6 +2153,7 @@ virProcessWait;
>  
>  
>  # util/virqemu.h
> +virQEMUBuildLuksOpts;
>  virQEMUBuildObjectCommandlineFromJSON;
>  
>  
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 4965c9e..fc50807 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -55,11 +55,14 @@
>  #include "viralloc.h"
>  #include "internal.h"
>  #include "secret_conf.h"
> +#include "secret_util.h"
>  #include "viruuid.h"
>  #include "virstoragefile.h"
>  #include "storage_backend.h"
>  #include "virlog.h"
>  #include "virfile.h"
> +#include "virjson.h"
> +#include "virqemu.h"
>  #include "stat-time.h"
>  #include "virstring.h"
>  #include "virxml.h"
> @@ -880,6 +883,7 @@ virStoragePloopResize(virStorageVolDefPtr vol,
>  enum {
>  QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
>  QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
> +QEMU_IMG_FORMAT_LUKS,
>  };
>  
>  static bool
> @@ -907,6 +911,27 @@ virStorageBackendQemuImgSupportsCompat(const char 
> *qemuimg)
>  return ret;
>  }
>  
> +
> +static bool
> +virStorageBackendQemuImgSupportsLuks(const char *qemuimg)
> +{
> +bool ret = false;
> +int exitstatus = -1;
> +virCommandPtr cmd = virCommandNewArgList(qemuimg, "create", "-o", "?",
> + "-f", "luks", "/dev/null", 
> NULL);
> +
> +if (virCommandRun(cmd, ) < 0)
> +goto cleanup;
> +
> +if (exitstatus == 0)
> +ret = true;
> +
> + cleanup:
> +virCommandFree(cmd);
> +return ret;
> +}
> +
> +
>  static int
>  virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>  {
> @@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char 
> *qemuimg)
>   * out what else we have */
>  int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
>  
> -/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> - * understands. Since we still support QEMU 0.12 and newer, we need
> - * to be able to handle the previous format as can be set via a
> - * compat=0.10 option. */
> -if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> -ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> +/* QEMU 2.6 added support for luks - let's check for that.
> + * If available, then we can also assume OPTIONS_COMPAT is present */
> +if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
> +ret = QEMU_IMG_FORMAT_LUKS;
> +} else {
> +/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> + * understands. Since we still support QEMU 0.12 and newer, we need
> + * to be able to handle the previous format as can be set via a
> + * compat=0.10 option. */
> +if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> +ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> +}

This looks like it's becoming a source of technical debt. Heaps of
comments aren't helping.

>  
>  return ret;
>  }
> @@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
>  const char *inputPath;
>  const char *inputFormatStr;
>  int inputFormat;
> +
> +char *secretAlias;
> +const char *secretPath;
>  };
>  
> +
>  static int
> -virStorageBackendCreateQemuImgOpts(char **opts,
> +virStorageBackendCreateQemuImgOpts(virStorageEncryptionPtr enc,
> +   char **opts,
> struct _virStorageBackendQemuImgInfo info)
>  {
>

[libvirt] [PATCH 17/19] storage: Add support to create a luks volume

2016-06-13 Thread John Ferlan 
If the volume xml was looking to create a luks volume take the necessary
steps in order to make that happen.

The processing will be:
 1. create a temporary file in the storage pool target path
   1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
   depending on how the encryption xml specified fetching the secret
   1b. create/open the file, initially setting mode to 0600 with current
   effective uid:gid as owner
   1c. fetch the secret into a buffer and write that into the file
   1d. change file protections to 0400

 2. create a secret object
   2a. use a similarly crafted alias to the file name
   2b. add the file to the object

 3. create/add luks options to the commandline
   3a. at the very least a "key-secret" using the secret object alias
   3b. if found in the XML the various "cipher" and "ivgen" options

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms   |   1 +
 src/storage/storage_backend.c  | 285 ++---
 src/storage/storage_backend.h  |   3 +-
 src/util/virqemu.c |  23 
 src/util/virqemu.h |   6 +
 tests/storagevolxml2argvtest.c |   3 +-
 6 files changed, 300 insertions(+), 21 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fdf06ae..0d6d080 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2153,6 +2153,7 @@ virProcessWait;
 
 
 # util/virqemu.h
+virQEMUBuildLuksOpts;
 virQEMUBuildObjectCommandlineFromJSON;
 
 
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 4965c9e..fc50807 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -55,11 +55,14 @@
 #include "viralloc.h"
 #include "internal.h"
 #include "secret_conf.h"
+#include "secret_util.h"
 #include "viruuid.h"
 #include "virstoragefile.h"
 #include "storage_backend.h"
 #include "virlog.h"
 #include "virfile.h"
+#include "virjson.h"
+#include "virqemu.h"
 #include "stat-time.h"
 #include "virstring.h"
 #include "virxml.h"
@@ -880,6 +883,7 @@ virStoragePloopResize(virStorageVolDefPtr vol,
 enum {
 QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
 QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
+QEMU_IMG_FORMAT_LUKS,
 };
 
 static bool
@@ -907,6 +911,27 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
 return ret;
 }
 
+
+static bool
+virStorageBackendQemuImgSupportsLuks(const char *qemuimg)
+{
+bool ret = false;
+int exitstatus = -1;
+virCommandPtr cmd = virCommandNewArgList(qemuimg, "create", "-o", "?",
+ "-f", "luks", "/dev/null", NULL);
+
+if (virCommandRun(cmd, ) < 0)
+goto cleanup;
+
+if (exitstatus == 0)
+ret = true;
+
+ cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
+
 static int
 virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
 {
@@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
  * out what else we have */
 int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
 
-/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
- * understands. Since we still support QEMU 0.12 and newer, we need
- * to be able to handle the previous format as can be set via a
- * compat=0.10 option. */
-if (virStorageBackendQemuImgSupportsCompat(qemuimg))
-ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
+/* QEMU 2.6 added support for luks - let's check for that.
+ * If available, then we can also assume OPTIONS_COMPAT is present */
+if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
+ret = QEMU_IMG_FORMAT_LUKS;
+} else {
+/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
+ * understands. Since we still support QEMU 0.12 and newer, we need
+ * to be able to handle the previous format as can be set via a
+ * compat=0.10 option. */
+if (virStorageBackendQemuImgSupportsCompat(qemuimg))
+ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
+}
 
 return ret;
 }
@@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
 const char *inputPath;
 const char *inputFormatStr;
 int inputFormat;
+
+char *secretAlias;
+const char *secretPath;
 };
 
+
 static int
-virStorageBackendCreateQemuImgOpts(char **opts,
+virStorageBackendCreateQemuImgOpts(virStorageEncryptionPtr enc,
+   char **opts,
struct _virStorageBackendQemuImgInfo info)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-if (info.backingPath)
-virBufferAsprintf(, "backing_fmt=%s,",
-  
virStorageFileFormatTypeToString(info.backingFormat));
-if (info.encryption)
-virBufferAddLit(, "encryption=on,");
-if (info.preallocate)
-virBufferAddLit(, "preallocation=metadata,");
+if (info.format == VIR_STORAGE_FILE_LUKS) {
+virQEMUBuildLuksOpts(, enc,