5:13 a.m.
As requested in:
https://www.redhat.com/archives/libvir-list/2020-March/msg00377.html
this is an incremental refactor of qemuDomainSecretAESSetup. Posted only
as an individual refactor to omit re-sending the 26 other patches from
the original series.
Peter Krempa (5):
qemuDomainSecretInfo: Register autoptr cleanup function
qemuDomainSecretAESSetup: Automatically free non-secret locals
qemuDomainSecretAESSetup: Allocate and return 'secinfo' here
qemuDomainSecretAESSetup: Split out lookup of secret data
Remove qemuDomainSecretInfoNew
src/qemu/qemu_domain.c | 178 ++++++++++++++++++-----------------------
src/qemu/qemu_domain.h | 2 +
2 files changed, 82 insertions(+), 98 deletions(-)
--
2.24.1
5:13 a.m.
New subject: [PATCH v2 1/5] qemuDomainSecretInfo: Register autoptr cleanup function
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 835321f54d..ad7ed3b9f0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1042,6 +1042,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
5:13 a.m.
New subject: [PATCH v2 2/5] qemuDomainSecretAESSetup: Automatically free non-secret locals
Use g_autofree for the ciphertext and init vector as they are not
secret and thus don't have to be cleared and use g_new0 to allocate the
iv for parity.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7c962fb062..e33d3099d6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1536,11 +1536,11 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
{
g_autoptr(virConnect) conn = virGetConnectSecret();
int ret = -1;
- uint8_t *raw_iv = 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)
@@ -1550,14 +1550,13 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
secinfo->s.aes.username = g_strdup(username);
if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias, isLuks)))
- goto cleanup;
+ return -1;
- 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 -1;
/* Encode the IV and save that since qemu will need it */
secinfo->s.aes.iv = g_base64_encode(raw_iv, ivlen);
@@ -1583,9 +1582,7 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
ret = 0;
cleanup:
- VIR_DISPOSE_N(raw_iv, ivlen);
VIR_DISPOSE_N(secret, secretlen);
- VIR_DISPOSE_N(ciphertext, ciphertextlen);
return ret;
}
--
2.24.1
5:13 a.m.
New subject: [PATCH v2 3/5] qemuDomainSecretAESSetup: Allocate and return 'secinfo' here
Rather than passing in an empty qemuDomainSecretInfoPtr allocate it
in this function and return it. This is done by absorbing the check from
qemuDomainSecretInfoNew and removing the internals of
qemuDomainSecretInfoNew.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 53 ++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e33d3099d6..e83301d84e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1521,21 +1521,20 @@ qemuDomainSecretPlainSetup(qemuDomainSecretInfoPtr secinfo,
* @seclookupdef: Pointer to seclookupdef data
* @isLuks: True/False for is for luks (alias generation)
*
- * Taking a secinfo, fill in the AES specific information using the
+ * Encrypts a secret looked up via @seclookupdef for use with qemu.
*
- * Returns 0 on success, -1 on failure with error message
+ * Returns qemuDomainSecretInfoPtr filled with the necessary information.
*/
-static int
+static qemuDomainSecretInfoPtr
qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
- qemuDomainSecretInfoPtr secinfo,
const char *srcalias,
virSecretUsageType usageType,
const char *username,
virSecretLookupTypeDefPtr seclookupdef,
bool isLuks)
{
+ g_autoptr(qemuDomainSecretInfo) secinfo = NULL;
g_autoptr(virConnect) conn = virGetConnectSecret();
- int ret = -1;
g_autofree uint8_t *raw_iv = NULL;
size_t ivlen = QEMU_DOMAIN_AES_IV_LEN;
uint8_t *secret = NULL;
@@ -1544,19 +1543,27 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
size_t ciphertextlen = 0;
if (!conn)
- return -1;
+ return NULL;
+
+ 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.username = g_strdup(username);
if (!(secinfo->s.aes.alias = qemuDomainGetSecretAESAlias(srcalias, isLuks)))
- return -1;
+ return NULL;
raw_iv = g_new0(uint8_t, ivlen);
/* Create a random initialization vector */
if (virRandomBytes(raw_iv, ivlen) < 0)
- return -1;
+ return NULL;
/* Encode the IV and save that since qemu will need it */
secinfo->s.aes.iv = g_base64_encode(raw_iv, ivlen);
@@ -1564,13 +1571,13 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
/* Grab the unencoded secret */
if (virSecretGetSecretString(conn, seclookupdef, usageType,
&secret, &secretlen) < 0)
- goto cleanup;
+ goto error;
if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC,
priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN,
raw_iv, ivlen, secret, secretlen,
&ciphertext, &ciphertextlen) < 0)
- goto cleanup;
+ goto error;
/* Clear out the secret */
memset(secret, 0, secretlen);
@@ -1579,11 +1586,11 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
secinfo->s.aes.ciphertext = g_base64_encode(ciphertext,
ciphertextlen);
- ret = 0;
+ return g_steal_pointer(&secinfo);
- cleanup:
+ error:
VIR_DISPOSE_N(secret, secretlen);
- return ret;
+ return NULL;
}
@@ -1655,24 +1662,8 @@ qemuDomainSecretInfoNew(qemuDomainObjPrivatePtr priv,
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;
+ return qemuDomainSecretAESSetup(priv, srcAlias, usageType, username,
+ lookupDef, isLuks);
}
--
2.24.1
5:13 a.m.
New subject: [PATCH v2 4/5] qemuDomainSecretAESSetup: Split out lookup of secret data
Split out the lookup of the secret from the secret driver into
qemuDomainSecretAESSetupFromSecret so that we can also instantiate
secret objects in qemu with data from other sources.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 87 ++++++++++++++++++++++++++----------------
1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e83301d84e..ba80bb67d2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1514,37 +1514,28 @@ 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
*
- * Encrypts a secret looked up via @seclookupdef for use with qemu.
+ * Encrypts @secret for use with qemu.
*
* Returns qemuDomainSecretInfoPtr filled with the necessary information.
*/
static qemuDomainSecretInfoPtr
qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
- const char *srcalias,
- virSecretUsageType usageType,
+ const char *alias,
const char *username,
- virSecretLookupTypeDefPtr seclookupdef,
- bool isLuks)
+ uint8_t *secret,
+ size_t secretlen)
{
g_autoptr(qemuDomainSecretInfo) secinfo = NULL;
- g_autoptr(virConnect) conn = virGetConnectSecret();
g_autofree uint8_t *raw_iv = NULL;
size_t ivlen = QEMU_DOMAIN_AES_IV_LEN;
- uint8_t *secret = NULL;
- size_t secretlen = 0;
g_autofree uint8_t *ciphertext = NULL;
size_t ciphertextlen = 0;
- if (!conn)
- return NULL;
-
if (!qemuDomainSupportsEncryptedSecret(priv)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("encrypted secrets are not supported"));
@@ -1554,11 +1545,9 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
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)))
- return NULL;
-
raw_iv = g_new0(uint8_t, ivlen);
/* Create a random initialization vector */
@@ -1568,29 +1557,61 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
/* 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 error;
-
if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC,
priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN,
raw_iv, ivlen, secret, secretlen,
&ciphertext, &ciphertextlen) < 0)
- goto error;
-
- /* 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);
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);
- error:
VIR_DISPOSE_N(secret, secretlen);
- return NULL;
+
+ return secinfo;
}
@@ -1662,8 +1683,8 @@ qemuDomainSecretInfoNew(qemuDomainObjPrivatePtr priv,
virSecretLookupTypeDefPtr lookupDef,
bool isLuks)
{
- return qemuDomainSecretAESSetup(priv, srcAlias, usageType, username,
- lookupDef, isLuks);
+ return qemuDomainSecretAESSetupFromSecret(priv, srcAlias, usageType, username,
+ lookupDef, isLuks);
}
--
2.24.1
5:13 a.m.
New subject: [PATCH v2 5/5] Remove qemuDomainSecretInfoNew
Replace it by a direct call to qemuDomainSecretAESSetupFromSecret.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 53 +++++++++++-------------------------------
1 file changed, 13 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ba80bb67d2..f74a1b6fb3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1661,33 +1661,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)
-{
- return qemuDomainSecretAESSetupFromSecret(priv, srcAlias, usageType, username,
- lookupDef, isLuks);
-}
-
-
/**
* qemuDomainSecretInfoTLSNew:
* @priv: pointer to domain private object
@@ -1714,9 +1687,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);
}
@@ -1806,11 +1779,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)
@@ -1818,11 +1791,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;
}
--
2.24.1
6:55 a.m.
On a Monday in 2020, Peter Krempa wrote:
As requested in:
https://www.redhat.com/archives/libvir-list/2020-March/msg00377.html
this is an incremental refactor of qemuDomainSecretAESSetup. Posted only
as an individual refactor to omit re-sending the 26 other patches from
the original series.
Peter Krempa (5):
qemuDomainSecretInfo: Register autoptr cleanup function
qemuDomainSecretAESSetup: Automatically free non-secret locals
qemuDomainSecretAESSetup: Allocate and return 'secinfo' here
qemuDomainSecretAESSetup: Split out lookup of secret data
Remove qemuDomainSecretInfoNew
src/qemu/qemu_domain.c | 178 ++++++++++++++++++-----------------------
src/qemu/qemu_domain.h | 2 +
2 files changed, 82 insertions(+), 98 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1744
days inactive
1744
days old
6 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa