Re: [PATCH v3 05/10] encryption: Add <cipher> and <ivgen> to encryption

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

 



On Fri, Jun 24, 2016 at 04:53:34PM -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               |  83 ++++++++++++-
docs/schemas/storagecommon.rng                     |  44 ++++++-
src/conf/domain_conf.c                             |  11 ++
src/util/virstorageencryption.c                    | 138 +++++++++++++++++++++
src/util/virstorageencryption.h                    |  14 +++
.../qemuxml2argv-luks-disk-cipher.xml              |  45 +++++++
.../qemuxml2xmlout-luks-disk-cipher.xml            |   1 +
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, 378 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.xml
create mode 120000 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


+static int
+virStorageEncryptionInfoParseCipher(xmlNodePtr info_node,
+                                    virStorageEncryptionInfoDefPtr info)
+{
+    int ret = -1;
+    char *size_str = NULL;
+
+    if (!(info->cipher_name = virXMLPropString(info_node, "name"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing cipher info name string"));

Either 'missing cipher name' or use "cipher info missing '%s'
attribute"...

+        goto cleanup;
+    }
+
+    if ((size_str = virXMLPropString(info_node, "size")) &&
+        virStrToLong_uip(size_str, NULL, 10, &info->cipher_size) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("cannot parse cipher info size string '%s'"),

"cannot parse cipher size: '%s'"

+                       size_str);
+        goto cleanup;
+    }
+
+    if (!size_str) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("cipher info missing 'size' attribute"));

to share the string with this error.

Also, should we be validating these by converting them to an enum?

+        goto cleanup;
+    }
+

+    /* Optional */

Hence no error following the calls.

+    info->cipher_mode = virXMLPropString(info_node, "mode");
+    info->cipher_hash = virXMLPropString(info_node, "hash");
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(size_str);
+    return ret;
+}
+
+
+static int
+virStorageEncryptionInfoParseIvgen(xmlNodePtr info_node,
+                                   virStorageEncryptionInfoDefPtr info)
+{
+    if (!(info->ivgen_name = virXMLPropString(info_node, "name"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing ivgen info name string"));
+        return -1;
+    }
+

+    /* Optional */

Also redundant.

+    info->ivgen_hash = virXMLPropString(info_node, "hash");
+
+    return 0;
+}
+
+
static virStorageEncryptionPtr
virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
{
@@ -196,6 +286,28 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
        VIR_FREE(nodes);
    }

+    if (ret->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+        xmlNodePtr tmpnode;
+
+        if ((tmpnode = virXPathNode("./cipher[1]", ctxt))) {
+            if (virStorageEncryptionInfoParseCipher(tmpnode, &ret->encinfo) < 0)
+                goto cleanup;
+        }
+
+        if ((tmpnode = virXPathNode("./ivgen[1]", ctxt))) {

+            /* If no cipher node, then fail */

Rather than putting it in this comment,

+            if (!ret->encinfo.cipher_name) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                                _("missing storage encryption cipher"));

how about putting it in the error:
"ivgen element found but cipher is missing"

+                goto cleanup;
+            }
+
+            if (virStorageEncryptionInfoParseIvgen(tmpnode, &ret->encinfo) < 0)
+                goto cleanup;
+        }
+    }
+
+
    return ret;

 cleanup:
@@ -250,6 +362,28 @@ virStorageEncryptionSecretFormat(virBufferPtr buf,
    return 0;
}

+
+static void
+virStorageEncryptionInfoDefFormat(virBufferPtr buf,
+                                  const virStorageEncryptionInfoDef *enc)
+{
+    virBufferAsprintf(buf, "<cipher name='%s' size='%u'",
+                      enc->cipher_name, enc->cipher_size);
+    if (enc->cipher_mode)
+        virBufferAsprintf(buf, " mode='%s'", enc->cipher_mode);
+    if (enc->cipher_hash)
+        virBufferAsprintf(buf, " hash='%s'", enc->cipher_hash);
+    virBufferAddLit(buf, "/>\n");
+
+    if (enc->ivgen_name) {
+        virBufferAsprintf(buf, "<ivgen name='%s'", enc->ivgen_name);
+        if (enc->ivgen_hash)
+            virBufferAsprintf(buf, " hash='%s'", enc->ivgen_hash);
+        virBufferAddLit(buf, "/>\n");
+    }

The strings need to be escaped.

+}
+
+
int
virStorageEncryptionFormat(virBufferPtr buf,
                           virStorageEncryptionPtr enc)

ACK with the strings escaped.

Jan

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