[PATCH 05/30] qemu: domain: Split out encryption of secret object data

Peter Krempa pkrempa at redhat.com
Mon Mar 9 16:22:45 UTC 2020


Previously qemuDomainSecretAESSetup was looking up the secret in the
secret diver as well as encrypting it for use with qemu. Split out the
the lookup into a wrapper for this function so that we can reuse the
original internals when we don't need to look up a secret with the
secret driver. The new wrapper is called
qemuDomainSecretAESSetupFromSecret.

This refactor also changes the functions to return qemuDomainSecretInfoPtr
directly rather than filling it via an argument. This rendered
qemuDomainSecretInfoNew obsolete and thus it was deleted.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_domain.c | 180 ++++++++++++++++++-----------------------
 src/qemu/qemu_domain.h |   2 +
 2 files changed, 81 insertions(+), 101 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bd32949e9b..52d2dddede 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1513,79 +1513,100 @@ qemuDomainSecretPlainSetup(qemuDomainSecretInfoPtr secinfo,

 /* qemuDomainSecretAESSetup:
  * @priv: pointer to domain private object
- * @secinfo: Pointer to secret info
- * @srcalias: Alias of the disk/hostdev used to generate the secret alias
- * @usageType: The virSecretUsageType
- * @username: username to use for authentication (may be NULL)
- * @seclookupdef: Pointer to seclookupdef data
- * @isLuks: True/False for is for luks (alias generation)
+ * @alias: alias of the secret
+ * @username: username to use (may be NULL)
+ * @secret: secret data
+ * @secretlen: length of @secret
  *
- * Taking a secinfo, fill in the AES specific information using the
- *
- * Returns 0 on success, -1 on failure with error message
+ * Encrypts @secret to use with the domain.
  */
-static int
+static qemuDomainSecretInfoPtr
 qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
-                         qemuDomainSecretInfoPtr secinfo,
-                         const char *srcalias,
-                         virSecretUsageType usageType,
+                         const char *alias,
                          const char *username,
-                         virSecretLookupTypeDefPtr seclookupdef,
-                         bool isLuks)
+                         uint8_t *secret,
+                         size_t secretlen)
 {
-    g_autoptr(virConnect) conn = virGetConnectSecret();
-    int ret = -1;
-    uint8_t *raw_iv = NULL;
+    g_autoptr(qemuDomainSecretInfo) secinfo = NULL;
+    g_autofree uint8_t *raw_iv = NULL;
     size_t ivlen = QEMU_DOMAIN_AES_IV_LEN;
-    uint8_t *secret = NULL;
-    size_t secretlen = 0;
-    uint8_t *ciphertext = NULL;
+    g_autofree uint8_t *ciphertext = NULL;
     size_t ciphertextlen = 0;

-    if (!conn)
-        return -1;
+    if (!qemuDomainSupportsEncryptedSecret(priv)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("encrypted secrets are not supported"));
+        return NULL;
+    }
+
+    secinfo = g_new0(qemuDomainSecretInfo, 1);

     secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES;
+    secinfo->s.aes.alias = g_strdup(alias);
     secinfo->s.aes.username = g_strdup(username);

-    if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias, isLuks)))
-        goto cleanup;
-
-    if (VIR_ALLOC_N(raw_iv, ivlen) < 0)
-        goto cleanup;
+    raw_iv = g_new0(uint8_t, ivlen);

     /* Create a random initialization vector */
     if (virRandomBytes(raw_iv, ivlen) < 0)
-        goto cleanup;
+        return NULL;

     /* Encode the IV and save that since qemu will need it */
     secinfo->s.aes.iv = g_base64_encode(raw_iv, ivlen);

-    /* Grab the unencoded secret */
-    if (virSecretGetSecretString(conn, seclookupdef, usageType,
-                                 &secret, &secretlen) < 0)
-        goto cleanup;
-
     if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC,
                              priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN,
                              raw_iv, ivlen, secret, secretlen,
                              &ciphertext, &ciphertextlen) < 0)
-        goto cleanup;
-
-    /* Clear out the secret */
-    memset(secret, 0, secretlen);
+        return NULL;

     /* Now encode the ciphertext and store to be passed to qemu */
-    secinfo->s.aes.ciphertext = g_base64_encode(ciphertext,
-                                                ciphertextlen);
+    secinfo->s.aes.ciphertext = g_base64_encode(ciphertext, ciphertextlen);

-    ret = 0;
+    return g_steal_pointer(&secinfo);
+}
+
+
+/**
+ * qemuDomainSecretAESSetupFromSecret:
+ * @priv: pointer to domain private object
+ * @srcalias: Alias of the disk/hostdev used to generate the secret alias
+ * @usageType: The virSecretUsageType
+ * @username: username to use for authentication (may be NULL)
+ * @seclookupdef: Pointer to seclookupdef data
+ * @isLuks: True/False for is for luks (alias generation)
+ *
+ * Looks up a secret in the secret driver based on @usageType and @seclookupdef
+ * and builds qemuDomainSecretInfoPtr from it.
+ */
+static qemuDomainSecretInfoPtr
+qemuDomainSecretAESSetupFromSecret(qemuDomainObjPrivatePtr priv,
+                                   const char *srcalias,
+                                   virSecretUsageType usageType,
+                                   const char *username,
+                                   virSecretLookupTypeDefPtr seclookupdef,
+                                   bool isLuks)
+{
+    g_autoptr(virConnect) conn = virGetConnectSecret();
+    qemuDomainSecretInfoPtr secinfo;
+    g_autofree char *alias = NULL;
+    uint8_t *secret = NULL;
+    size_t secretlen = 0;
+
+    if (!conn)
+        return NULL;
+
+    if (!(alias = qemuDomainGetSecretAESAlias(srcalias, isLuks)))
+        return NULL;
+
+    if (virSecretGetSecretString(conn, seclookupdef, usageType, &secret, &secretlen) < 0)
+        return NULL;
+
+    secinfo = qemuDomainSecretAESSetup(priv, alias, username, secret, secretlen);

- cleanup:
-    VIR_DISPOSE_N(raw_iv, ivlen);
     VIR_DISPOSE_N(secret, secretlen);
-    VIR_DISPOSE_N(ciphertext, ciphertextlen);
-    return ret;
+
+    return secinfo;
 }


@@ -1635,49 +1656,6 @@ qemuDomainSecretInfoNewPlain(virSecretUsageType usageType,
 }


-/* qemuDomainSecretInfoNew:
- * @priv: pointer to domain private object
- * @srcAlias: Alias base to use for TLS object
- * @usageType: Secret usage type
- * @username: username
- * @looupDef: lookup def describing secret
- * @isLuks: boolean for luks lookup
- *
- * Helper function to create a secinfo to be used for secinfo consumers. This
- * sets up encrypted data to be used with qemu's 'secret' object.
- *
- * Returns @secinfo on success, NULL on failure. Caller is responsible
- * to eventually free @secinfo.
- */
-static qemuDomainSecretInfoPtr
-qemuDomainSecretInfoNew(qemuDomainObjPrivatePtr priv,
-                        const char *srcAlias,
-                        virSecretUsageType usageType,
-                        const char *username,
-                        virSecretLookupTypeDefPtr lookupDef,
-                        bool isLuks)
-{
-    qemuDomainSecretInfoPtr secinfo = NULL;
-
-    if (!qemuDomainSupportsEncryptedSecret(priv)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("encrypted secrets are not supported"));
-        return NULL;
-    }
-
-    if (VIR_ALLOC(secinfo) < 0)
-        return NULL;
-
-    if (qemuDomainSecretAESSetup(priv, secinfo, srcAlias, usageType, username,
-                                 lookupDef, isLuks) < 0) {
-        g_clear_pointer(&secinfo, qemuDomainSecretInfoFree);
-        return NULL;
-    }
-
-    return secinfo;
-}
-
-
 /**
  * qemuDomainSecretInfoTLSNew:
  * @priv: pointer to domain private object
@@ -1704,9 +1682,9 @@ qemuDomainSecretInfoTLSNew(qemuDomainObjPrivatePtr priv,
     }
     seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;

-    return qemuDomainSecretInfoNew(priv, srcAlias,
-                                   VIR_SECRET_USAGE_TYPE_TLS, NULL,
-                                   &seclookupdef, false);
+    return qemuDomainSecretAESSetupFromSecret(priv, srcAlias,
+                                              VIR_SECRET_USAGE_TYPE_TLS,
+                                              NULL, &seclookupdef, false);
 }


@@ -1796,11 +1774,11 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv,
                                                             src->auth->username,
                                                             &src->auth->seclookupdef);
         } else {
-            srcPriv->secinfo = qemuDomainSecretInfoNew(priv, authalias,
-                                                       usageType,
-                                                       src->auth->username,
-                                                       &src->auth->seclookupdef,
-                                                       false);
+            srcPriv->secinfo = qemuDomainSecretAESSetupFromSecret(priv, authalias,
+                                                                  usageType,
+                                                                  src->auth->username,
+                                                                  &src->auth->seclookupdef,
+                                                                  false);
         }

         if (!srcPriv->secinfo)
@@ -1808,11 +1786,11 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv,
     }

     if (hasEnc) {
-        if (!(srcPriv->encinfo =
-              qemuDomainSecretInfoNew(priv, encalias,
-                                      VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
-                                      &src->encryption->secrets[0]->seclookupdef,
-                                      true)))
+        if (!(srcPriv->encinfo = qemuDomainSecretAESSetupFromSecret(priv, encalias,
+                                                                    VIR_SECRET_USAGE_TYPE_VOLUME,
+                                                                    NULL,
+                                                                    &src->encryption->secrets[0]->seclookupdef,
+                                                                    true)))
               return -1;
     }

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 10d6264e46..202b85e39a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1035,6 +1035,8 @@ bool qemuDomainSupportsEncryptedSecret(qemuDomainObjPrivatePtr priv);
 void qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr secinfo)
     ATTRIBUTE_NONNULL(1);

+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainSecretInfo, qemuDomainSecretInfoFree);
+
 void qemuDomainSecretInfoDestroy(qemuDomainSecretInfoPtr secinfo);

 void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
-- 
2.24.1




More information about the libvir-list mailing list