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

[libvirt] [PATCH v3 2/2] qemu: Utilize qemu secret objects for RBD auth/secret



Partial fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1182074

If they're available and we need to pass secrets to qemu, then use the
qemu domain secret object in order to pass the secrets for RBD volumes
instead of passing the base64 encoded secret on the command line.

The goal is to make IV secrets the default and have no user interaction
required in order to allow using the IV mechanism. If the mechanism
is not available, then fall back to the current plain mechanism using
a base64 encoded secret.

New API's:

qemu_command.c:
  qemuBuildSecretInfoProps: (private)
    Generate/return a JSON properties object for the IV secret to
    be used by both the command building and eventually the hotplug
    code in order to add the secret object. Code was designed so that
    in the future perhaps hotplug could use it if it made sense.

  qemuBuildSecretIVCommandLine (private)
    Generate and add to the command line the -object secret for the
    IV secret. This will be required for the subsequent RBD reference
    to the object.

  qemuBuildDiskSecinfoCommandLine (private)
    Handle adding the IV secret object.

qemu_domain.c
  qemuDomainSecretSetup: (private)
    Shim to call either the IV or Plain Setup functions based upon
    whether IV secrets are possible (we have the encryption API) or not,
    we have secrets, and of course if the protocol source is RBD.

    Use the qemuDomainSecretSetup in qemuDomainSecretDiskPrepare and
    qemuDomainSecretHostdevPrepare to add the secret rather than assuming
    plain. In the future when iSCSI secrets can use this mechanism for
    either a disk or hostdev there will be less change.

Command Building:

  Adjust the qemuBuildRBDSecinfoURI API's in order to generate the
  specific command options for an IV secret, such as:

    -object secret,id=$alias,keyid=$masterKey,data=$base64encodedencrypted,
            format=base64
    -drive file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
           mon_host=mon1.example.org\:6321,password-secret=$alias,...

  where the 'id=' value is the secret object alias generated by
  concatenating the disk alias and "-ivKey0". The 'keyid= $masterKey'
  is the master key shared with qemu, and the -drive syntax will
  reference that alias as the 'password-secret'. For the -drive
  syntax, the 'id=myname' is kept to define the username, while the
  'key=$base64 encoded secret' is removed.

  While according to the syntax described for qemu commit '60390a21'
  or as seen in the email archive:

    https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04083.html

  it is possible to pass a plaintext password via a file, the qemu
  commit 'ac1d8878' describes the more feature rich 'keyid=' option
  based upon the shared masterKey.

Tests:

  Add mock's for virRandomBytes and gnutls_rnd in order to return a
  constant stream of '0xff' in the bytes for a non random key in order
  to generate "constant" values for the secrets so that the tests can
  use those results to compare results.

Hotplug:

  Since the hotplug code doesn't add command line arguments, passing
  the encoded secret directly to the monitor will suffice.

Signed-off-by: John Ferlan <jferlan redhat com>
---
 src/qemu/qemu_command.c                            | 130 ++++++++++++++++++++-
 src/qemu/qemu_domain.c                             |  56 +++++++--
 ...emuxml2argv-disk-drive-network-rbd-auth-IV.args |  31 +++++
 ...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml |  42 +++++++
 tests/qemuxml2argvmock.c                           |  31 ++++-
 tests/qemuxml2argvtest.c                           |  11 ++
 6 files changed, 290 insertions(+), 11 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 93aaf2a..9b0bd7a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -607,6 +607,119 @@ qemuNetworkDriveGetPort(int protocol,
 }
 
 
+/**
+ * qemuBuildSecretInfoProps:
+ * @secinfo: pointer to the secret info object
+ * @type: returns a pointer to a character string for object name
+ * @props: json properties to return
+ *
+ * Build the JSON properties for the secret info type.
+ *
+ * Returns 0 on success with the filled in JSON property; otherwise,
+ * returns -1 on failure error message set.
+ */
+static int
+qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
+                         const char **type,
+                         virJSONValuePtr *propsret)
+{
+    int ret = -1;
+    char *keyid = NULL;
+    virJSONValuePtr props = NULL;
+
+    *type = "secret";
+
+    if (!(keyid = qemuDomainGetMasterKeyAlias()))
+        return -1;
+
+    if (!(props = virJSONValueNewObject()))
+        goto cleanup;
+
+    if (virJSONValueObjectAdd(props,
+                              "s:data", secinfo->s.iv.ciphertext,
+                              "s:keyid", keyid,
+                              "s:iv", secinfo->s.iv.iv,
+                              "s:format", "base64", NULL) < 0)
+        goto cleanup;
+
+    *propsret = props;
+    props = NULL;
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(keyid);
+    virJSONValueFree(props);
+
+    return ret;
+}
+
+
+/**
+ * qemuBuildSecretIVCommandLine:
+ * @cmd: the command to modify
+ * @secinfo: pointer to the secret info object
+ *
+ * If the secinfo is available and associated with an IV secret,
+ * then format the command line for the secret object. This object
+ * will be referenced by the device that needs/uses it, so it needs
+ * to be in place first.
+ *
+ * Returns 0 on success, -1 w/ error message on failure
+ */
+static int
+qemuBuildSecretIVCommandLine(virCommandPtr cmd,
+                             qemuDomainSecretInfoPtr secinfo)
+{
+    int ret = -1;
+    virJSONValuePtr props = NULL;
+    const char *type;
+    char *tmp = NULL;
+
+    if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0)
+        return -1;
+
+    if (!(tmp = qemuBuildObjectCommandlineFromJSON(type, secinfo->s.iv.alias,
+                                                   props)))
+        goto cleanup;
+
+    virCommandAddArgList(cmd, "-object", tmp, NULL);
+    ret = 0;
+
+ cleanup:
+    virJSONValueFree(props);
+    VIR_FREE(tmp);
+
+    return ret;
+}
+
+
+/* qemuBuildDiskSecinfoCommandLine:
+ * @cmd: Pointer to the command string
+ * @secinfo: Pointer to a possible secinfo
+ *
+ * Adding an IV secret to the command line for an iSCSI disk currently
+ * does not work. As of qemu 2.6, in order to add the options "user=" and
+ * "password-secret=", one would have to do so using an "-iscsi" command
+ * line option. However, that requires supplying an "id=" that can be used
+ * by some "-drive" command in order to "find" the correct IQN. Unfortunately,
+ * an IQN consists of characters ':' and '/' that cannot be used for an "id=".
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskSecinfoCommandLine(virCommandPtr cmd,
+                                qemuDomainSecretInfoPtr secinfo)
+{
+    /* Not necessary for non IV secrets */
+    if (!secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_TYPE_IV)
+        return 0;
+
+    /* For now we'll just build the IV. In the future more may be required
+     * in order to support iSCSI */
+    return qemuBuildSecretIVCommandLine(cmd, secinfo);
+}
+
+
 /* qemuBuildGeneralSecinfoURI:
  * @uri: Pointer to the URI structure to add to
  * @secinfo: Pointer to the secret info data (if present)
@@ -677,6 +790,10 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
         break;
 
     case VIR_DOMAIN_SECRET_INFO_TYPE_IV:
+        virBufferEscape(buf, '\\', ":", ":id=%s:auth_supported=cephx\\;none",
+                        secinfo->s.iv.username);
+        break;
+
     case VIR_DOMAIN_SECRET_INFO_TYPE_LAST:
         return -1;
     }
@@ -1083,6 +1200,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
     char *source = NULL;
     int actualType = virStorageSourceGetActualType(disk->src);
     qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+    qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
 
     if (idx < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1164,7 +1282,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
         break;
     }
 
-    if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) < 0)
+    if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0)
         goto error;
 
     if (source &&
@@ -1215,6 +1333,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
 
         virBufferEscape(&opt, ',', ",", "%s,", source);
 
+        if (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
+            secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_IV) {
+            virBufferAsprintf(&opt, "password-secret=%s,", secinfo->s.iv.alias);
+        }
+
         if (disk->src->format > 0 &&
             disk->src->type != VIR_STORAGE_TYPE_DIR)
             virBufferAsprintf(&opt, "format=%s,",
@@ -1905,6 +2028,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
         virDomainDiskDefPtr disk = def->disks[i];
         bool withDeviceArg = false;
         bool deviceFlagMasked = false;
+        qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+        qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
 
         /* Unless we have -device, then USB disks need special
            handling */
@@ -1947,6 +2072,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
             break;
         }
 
+        if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
+            return -1;
+
         virCommandAddArg(cmd, "-drive");
 
         /* Unfortunately it is not possible to use
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b174caa..3e627a5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -897,7 +897,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn,
  *
  * Returns true if we can support the encryption code, false otherwise
  */
-static bool ATTRIBUTE_UNUSED
+static bool
 qemuDomainSecretHaveEncrypt(void)
 {
 #ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
@@ -920,7 +920,7 @@ qemuDomainSecretHaveEncrypt(void)
  *
  * Returns 0 on success, -1 on failure with error message
  */
-static int ATTRIBUTE_UNUSED
+static int
 qemuDomainSecretIVSetup(virConnectPtr conn,
                         qemuDomainObjPrivatePtr priv,
                         qemuDomainSecretInfoPtr secinfo,
@@ -1030,6 +1030,44 @@ qemuDomainSecretIVSetup(virConnectPtr conn,
 }
 
 
+/* qemuDomainSecretSetup:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @secinfo: Pointer to secret info
+ * @srcalias: Alias of the disk/hostdev used to generate the secret alias
+ * @protocol: Protocol for secret
+ * @authdef: Pointer to auth data
+ *
+ * If we have the encryption API present and can support a secret object, then
+ * build the IV secret; otherwise, build the Plain secret. This is the magic
+ * decision point for utilizing the IV secrets for an RBD disk. For now iSCSI
+ * disks and hostdevs will not be able to utilize this mechanism. For details,
+ * see qemuBuildDiskSecinfoCommandLine in qemu_command.c
+ *
+ * Returns 0 on success, -1 on failure
+ */
+static int
+qemuDomainSecretSetup(virConnectPtr conn,
+                      qemuDomainObjPrivatePtr priv,
+                      qemuDomainSecretInfoPtr secinfo,
+                      const char *srcalias,
+                      virStorageNetProtocol protocol,
+                      virStorageAuthDefPtr authdef)
+{
+    if (qemuDomainSecretHaveEncrypt() &&
+        virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) &&
+        protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
+        if (qemuDomainSecretIVSetup(conn, priv, secinfo,
+                                    srcalias, protocol, authdef) < 0)
+            return -1;
+    } else {
+        if (qemuDomainSecretPlainSetup(conn, secinfo, protocol, authdef) < 0)
+            return -1;
+    }
+    return 0;
+}
+
+
 /* qemuDomainSecretDiskDestroy:
  * @disk: Pointer to a disk definition
  *
@@ -1058,7 +1096,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
  */
 int
 qemuDomainSecretDiskPrepare(virConnectPtr conn,
-                            qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
+                            qemuDomainObjPrivatePtr priv,
                             virDomainDiskDefPtr disk)
 {
     virStorageSourcePtr src = disk->src;
@@ -1075,8 +1113,8 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
         if (VIR_ALLOC(secinfo) < 0)
             return -1;
 
-        if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
-                                       src->auth) < 0)
+        if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
+                                  src->protocol, src->auth) < 0)
             goto error;
 
         diskPriv->secinfo = secinfo;
@@ -1119,7 +1157,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev)
  */
 int
 qemuDomainSecretHostdevPrepare(virConnectPtr conn,
-                               qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
+                               qemuDomainObjPrivatePtr priv,
                                virDomainHostdevDefPtr hostdev)
 {
     virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
@@ -1140,9 +1178,9 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
             if (VIR_ALLOC(secinfo) < 0)
                 return -1;
 
-            if (qemuDomainSecretPlainSetup(conn, secinfo,
-                                           VIR_STORAGE_NET_PROTOCOL_ISCSI,
-                                           iscsisrc->auth) < 0)
+            if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias,
+                                      VIR_STORAGE_NET_PROTOCOL_ISCSI,
+                                      iscsisrc->auth) < 0)
                 goto error;
 
             hostdevPriv->secinfo = secinfo;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
new file mode 100644
index 0000000..f6c0906
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-object secret,id=virtio-disk0-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-drive 'file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
+mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\
+password-secret=virtio-disk0-ivKey0,format=raw,if=none,id=drive-virtio-disk0' \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
new file mode 100644
index 0000000..ac2e942
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <auth username='myname'>
+        <secret type='ceph' usage='mycluster_myname'/>
+      </auth>
+      <source protocol='rbd' name='pool/image'>
+        <host name='mon1.example.org' port='6321'/>
+        <host name='mon2.example.org' port='6322'/>
+        <host name='mon3.example.org' port='6322'/>
+      </source>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 1616eed..2bfbf6b 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2016 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -30,6 +30,13 @@
 #include "virstring.h"
 #include "virtpm.h"
 #include "virutil.h"
+#include "virrandom.h"
+#ifdef WITH_GNUTLS
+# include <gnutls/gnutls.h>
+# if HAVE_GNUTLS_CRYPTO_H
+#  include <gnutls/crypto.h>
+# endif
+#endif
 #include <time.h>
 #include <unistd.h>
 
@@ -145,3 +152,25 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
 {
     /* nada */
 }
+
+int
+virRandomBytes(unsigned char *buf,
+               size_t buflen)
+{
+    size_t i;
+
+    for (i = 0; i < buflen; i++)
+        buf[i] = 0xff;
+
+    return 0;
+}
+
+#ifdef WITH_GNUTLS
+int
+gnutls_rnd(gnutls_rnd_level_t level ATTRIBUTE_UNUSED,
+           void *data,
+           size_t len)
+{
+    return virRandomBytes(data, len);
+#endif
+}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e41444d..1f2577a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -26,6 +26,7 @@
 # include "virstring.h"
 # include "storage/storage_driver.h"
 # include "virmock.h"
+# include "virrandom.h"
 
 # include "testutilsqemu.h"
 
@@ -330,6 +331,14 @@ static int testCompareXMLToArgvFiles(const char *xml,
         }
     }
 
+    /* Create a fake master key */
+    if (VIR_ALLOC_N(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
+        goto out;
+
+    if (virRandomBytes(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
+        goto out;
+    priv->masterKeyLen = QEMU_DOMAIN_MASTER_KEY_LEN;
+
     if (!(cmd = qemuProcessCreatePretendCmd(conn, &driver, vm, migrateURI,
                                             (flags & FLAG_FIPS), false,
                                             VIR_QEMU_PROCESS_START_COLD))) {
@@ -776,6 +785,8 @@ mymain(void)
     DO_TEST("disk-drive-network-rbd", NONE);
     DO_TEST("disk-drive-network-sheepdog", NONE);
     DO_TEST("disk-drive-network-rbd-auth", NONE);
+    DO_TEST("disk-drive-network-rbd-auth-IV",
+            QEMU_CAPS_OBJECT_SECRET);
     DO_TEST("disk-drive-network-rbd-ipv6", NONE);
     DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
     DO_TEST("disk-drive-no-boot",
-- 
2.5.5


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