[libvirt] [PATCH v2 13/14] secret: Introduce virSecretObjGetValue and virSecretObjGetValueSize

Introduce the final accessor's to _virSecretObject data and move the structure from virsecretobj.h to virsecretobj.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&gt; --- src/conf/virsecretobj.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/virsecretobj.h | 18 +++++----- src/libvirt_private.syms | 4 +++ src/secret/secret_driver.c | 50 ++++----------------------- 4 files changed, 105 insertions(+), 52 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 8854a32..4f28392 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -36,6 +36,15 @@ VIR_LOG_INIT("conf.virsecretobj"); +struct _virSecretObj { + virObjectLockable parent; + char *configFile; + char *base64File; + virSecretDefPtr def; + unsigned char *value; /* May be NULL */ + size_t value_size; +}; + static virClassPtr virSecretObjClass; static virClassPtr virSecretObjListClass; static void virSecretObjDispose(void *obj); @@ -755,6 +764,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; +} + + static int virSecretLoadValidateUUID(virSecretDefPtr def, const char *file) diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h index a9b3103..fa45b42 100644 --- a/src/conf/virsecretobj.h +++ b/src/conf/virsecretobj.h @@ -28,15 +28,6 @@ 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); @@ -105,6 +96,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); + int virSecretLoadAllConfigs(virSecretObjListPtr secrets, const char *configDir); #endif /* __VIRSECRETOBJ_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2e20b3..ad5c382 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -900,6 +900,8 @@ virSecretObjDeleteConfig; virSecretObjDeleteData; virSecretObjEndAPI; virSecretObjGetDef; +virSecretObjGetValue; +virSecretObjGetValueSize; virSecretObjListAdd; virSecretObjListExport; virSecretObjListFindByUsage; @@ -911,6 +913,8 @@ virSecretObjListRemove; virSecretObjSaveConfig; virSecretObjSaveData; virSecretObjSetDef; +virSecretObjSetValue; +virSecretObjSetValueSize; # cpu/cpu.h diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 61de3cb..bbfb382 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -312,16 +312,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; @@ -329,40 +324,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; } @@ -385,14 +357,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", @@ -400,10 +364,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.5