11:39 a.m.
Introduce the final accessor's to _virSecretObject data and move the
structure from secret_conf.h to secret_conf.c
The virSecretObjSetValue logic will handle setting both the secret
value and the value_size. Some slight adjustments to the error path
over what was in secretSetValue were made.
Additionally, a slight logic change in secretGetValue where we'll
check for the internalFlags and error out before checking for
and erroring out for a NULL secret->value. That way, it won't be
obvious to anyone that the secret value wasn't set rather they'll
just know they cannot get the secret value since it's private.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/secret_conf.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
src/conf/secret_conf.h | 17 +++++-----
src/libvirt_private.syms | 4 +++
src/secret/secret_driver.c | 50 ++++-----------------------
4 files changed, 104 insertions(+), 51 deletions(-)
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 0410328..4d0cb9c 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -45,6 +45,14 @@ VIR_LOG_INIT("conf.secret_conf");
VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST,
"none", "volume", "ceph", "iscsi")
+struct _virSecretObj {
+ virObjectLockable parent;
+ char *configFile;
+ char *base64File;
+ virSecretDefPtr def;
+ unsigned char *value; /* May be NULL */
+ size_t value_size;
+};
static virClassPtr virSecretObjClass;
static void virSecretObjDispose(void *obj);
@@ -790,6 +798,82 @@ virSecretObjSetDef(virSecretObjPtr secret,
}
+unsigned char *
+virSecretObjGetValue(virSecretObjPtr secret)
+{
+ unsigned char *ret = NULL;
+
+ if (!secret->value) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(secret->def->uuid, uuidstr);
+ virReportError(VIR_ERR_NO_SECRET,
+ _("secret '%s' does not have a value"),
uuidstr);
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(ret, secret->value_size) < 0)
+ goto cleanup;
+ memcpy(ret, secret->value, secret->value_size);
+
+ cleanup:
+ return ret;
+}
+
+
+int
+virSecretObjSetValue(virSecretObjPtr secret,
+ const unsigned char *value,
+ size_t value_size)
+{
+ unsigned char *old_value, *new_value;
+ size_t old_value_size;
+
+ if (VIR_ALLOC_N(new_value, value_size) < 0)
+ return -1;
+
+ old_value = secret->value;
+ old_value_size = secret->value_size;
+
+ memcpy(new_value, value, value_size);
+ secret->value = new_value;
+ secret->value_size = value_size;
+
+ if (!secret->def->ephemeral && virSecretObjSaveData(secret) < 0)
+ goto error;
+
+ /* Saved successfully - drop old value */
+ if (old_value) {
+ memset(old_value, 0, old_value_size);
+ VIR_FREE(old_value);
+ }
+
+ return 0;
+
+ error:
+ /* Error - restore previous state and free new value */
+ secret->value = old_value;
+ secret->value_size = old_value_size;
+ memset(new_value, 0, value_size);
+ VIR_FREE(new_value);
+ return -1;
+}
+
+
+size_t
+virSecretObjGetValueSize(virSecretObjPtr secret)
+{
+ return secret->value_size;
+}
+
+
+void
+virSecretObjSetValueSize(virSecretObjPtr secret,
+ size_t value_size)
+{
+ secret->value_size = value_size;
+}
+
+
void
virSecretDefFree(virSecretDefPtr def)
{
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
index ce714c1..be3bff9 100644
--- a/src/conf/secret_conf.h
+++ b/src/conf/secret_conf.h
@@ -46,14 +46,6 @@ struct _virSecretDef {
typedef struct _virSecretObj virSecretObj;
typedef virSecretObj *virSecretObjPtr;
-struct _virSecretObj {
- virObjectLockable parent;
- char *configFile;
- char *base64File;
- virSecretDefPtr def;
- unsigned char *value; /* May be NULL */
- size_t value_size;
-};
virSecretObjPtr virSecretObjNew(void);
@@ -126,6 +118,15 @@ virSecretDefPtr virSecretObjGetDef(virSecretObjPtr secret);
void virSecretObjSetDef(virSecretObjPtr secret, virSecretDefPtr def);
+unsigned char *virSecretObjGetValue(virSecretObjPtr secret);
+
+int virSecretObjSetValue(virSecretObjPtr secret,
+ const unsigned char *value, size_t value_size);
+
+size_t virSecretObjGetValueSize(virSecretObjPtr secret);
+
+void virSecretObjSetValueSize(virSecretObjPtr secret, size_t value_size);
+
void virSecretDefFree(virSecretDefPtr def);
virSecretDefPtr virSecretDefParseString(const char *xml);
virSecretDefPtr virSecretDefParseFile(const char *filename);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3a417f0..15370f6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -791,6 +791,8 @@ virSecretObjDeleteConfig;
virSecretObjDeleteData;
virSecretObjEndAPI;
virSecretObjGetDef;
+virSecretObjGetValue;
+virSecretObjGetValueSize;
virSecretObjListAdd;
virSecretObjListExport;
virSecretObjListFindByUsage;
@@ -802,6 +804,8 @@ virSecretObjListRemove;
virSecretObjSaveConfig;
virSecretObjSaveData;
virSecretObjSetDef;
+virSecretObjSetValue;
+virSecretObjSetValueSize;
virSecretUsageIDForDef;
virSecretUsageTypeFromString;
virSecretUsageTypeToString;
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 676c02e..ab58115 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -311,16 +311,11 @@ secretSetValue(virSecretPtr obj,
unsigned int flags)
{
int ret = -1;
- unsigned char *old_value, *new_value;
- size_t old_value_size;
virSecretObjPtr secret;
virSecretDefPtr def;
virCheckFlags(0, -1);
- if (VIR_ALLOC_N(new_value, value_size) < 0)
- return -1;
-
if (!(secret = secretObjFromSecret(obj)))
goto cleanup;
@@ -328,40 +323,17 @@ secretSetValue(virSecretPtr obj,
if (virSecretSetValueEnsureACL(obj->conn, def) < 0)
goto cleanup;
- old_value = secret->value;
- old_value_size = secret->value_size;
-
- memcpy(new_value, value, value_size);
- secret->value = new_value;
- secret->value_size = value_size;
- if (!def->ephemeral) {
- if (secretEnsureDirectory() < 0)
- goto cleanup;
+ if (secretEnsureDirectory() < 0)
+ goto cleanup;
- if (virSecretObjSaveData(secret) < 0)
- goto restore_backup;
- }
- /* Saved successfully - drop old value */
- if (old_value != NULL) {
- memset(old_value, 0, old_value_size);
- VIR_FREE(old_value);
- }
- new_value = NULL;
+ if (virSecretObjSetValue(secret, value, value_size) < 0)
+ goto cleanup;
ret = 0;
- goto cleanup;
-
- restore_backup:
- /* Error - restore previous state and free new value */
- secret->value = old_value;
- secret->value_size = old_value_size;
- memset(new_value, 0, value_size);
cleanup:
virSecretObjEndAPI(&secret);
- VIR_FREE(new_value);
-
return ret;
}
@@ -384,14 +356,6 @@ secretGetValue(virSecretPtr obj,
if (virSecretGetValueEnsureACL(obj->conn, def) < 0)
goto cleanup;
- if (secret->value == NULL) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(obj->uuid, uuidstr);
- virReportError(VIR_ERR_NO_SECRET,
- _("secret '%s' does not have a value"),
uuidstr);
- goto cleanup;
- }
-
if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
def->private) {
virReportError(VIR_ERR_INVALID_SECRET, "%s",
@@ -399,10 +363,10 @@ secretGetValue(virSecretPtr obj,
goto cleanup;
}
- if (VIR_ALLOC_N(ret, secret->value_size) < 0)
+ if (!(ret = virSecretObjGetValue(secret)))
goto cleanup;
- memcpy(ret, secret->value, secret->value_size);
- *value_size = secret->value_size;
+
+ *value_size = virSecretObjGetValueSize(secret);
cleanup:
virSecretObjEndAPI(&secret);
--
2.5.0