Since we now have the functionality to provide the secrets driver
with an encryption key, and the newly introduced attribute to store
the encryption scheme across driver restarts, we can use the key to encrypt
secrets. While loading the secrets, we check whether the secret is
encrypted or not and accordingly get the value.

While the stored encryption scheme ensures the driver can
successfully load secrets after a restart,
If the user changes the encryption key between driver restarts,
any secrets encrypted with the previous key will become permanently
inaccessible upon the next restart. Users must ensure key consistency
to maintain access to existing encrypted secrets.

Signed-off-by: Arun Menon <armenon@redhat.com>
---
 src/conf/virsecretobj.c    | 165 ++++++++++++++++++++++++++++++-------
 src/conf/virsecretobj.h    |  10 ++-
 src/secret/secret_driver.c |  23 ++++--
 3 files changed, 157 insertions(+), 41 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 66270e2751..37b2db960f 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -31,6 +31,10 @@
 #include "virhash.h"
 #include "virlog.h"
 #include "virstring.h"
+#include "virsecret.h"
+#include "virrandom.h"
+#include "vircrypto.h"
+#include "virsecureerase.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECRET
 
@@ -328,6 +332,8 @@ virSecretObjListAdd(virSecretObjList *secrets,
     virSecretObj *obj;
     virSecretDef *objdef;
     virSecretObj *ret = NULL;
+    const char *encryptionScheme = NULL;
+    const char *encryptionSchemeExt = NULL;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     virObjectRWLockWrite(secrets);
@@ -379,10 +385,26 @@ virSecretObjListAdd(virSecretObjList *secrets,
             goto cleanup;
 
         /* Generate the possible configFile and base64File strings
-         * using the configDir, uuidstr, and appropriate suffix
+         * using the configDir, uuidstr, and appropriate encryption scheme
          */
-        if (!(obj->configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
-            !(obj->base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
+        if ((*newdef)->encryption_scheme != VIR_SECRET_ENCRYPTION_SCHEME_NONE
+            && (*newdef)->encryption_scheme != -1) {
+            encryptionScheme = virSecretEncryptionSchemeTypeToString((*newdef)->encryption_scheme);
+            if (!encryptionScheme) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("unknown secret encryption scheme %1$d"), (*newdef)->encryption_scheme);
+                goto cleanup;
+            }
+            encryptionSchemeExt = g_strconcat(".", encryptionScheme, NULL);
+            if (!(obj->base64File = virFileBuildPath(configDir, uuidstr, encryptionSchemeExt))) {
+                goto cleanup;
+            }
+        } else {
+            if (!(obj->base64File = virFileBuildPath(configDir, uuidstr, ".base64"))) {
+                goto cleanup;
+            }
+        }
+        if (!(obj->configFile = virFileBuildPath(configDir, uuidstr, ".xml")))
             goto cleanup;
 
         if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)
@@ -397,6 +419,7 @@ virSecretObjListAdd(virSecretObjList *secrets,
  cleanup:
     virSecretObjEndAPI(&obj);
     virObjectRWUnlock(secrets);
+    g_clear_pointer((gpointer *)&encryptionSchemeExt, g_free);
     return ret;
 }
 
@@ -680,17 +703,49 @@ virSecretObjSaveConfig(virSecretObj *obj)
     return 0;
 }
 
-
 int
-virSecretObjSaveData(virSecretObj *obj)
+virSecretObjSaveData(virSecretObj *obj,
+                     virSecretDaemonConfig *driverConfig)
 {
     g_autofree char *base64 = NULL;
+    g_autofree uint8_t *encryptedValue = NULL;
+    size_t encryptedValueLen = 0;
+    size_t base64Len = 0;
+    uint8_t iv[16] = { 0 };
 
     if (!obj->value)
         return 0;
 
-    base64 = g_base64_encode(obj->value, obj->value_size);
-
+    if (obj->def->encryption_scheme == VIR_SECRET_ENCRYPTION_SCHEME_NONE
+        || obj->def->encryption_scheme == -1) {
+        base64 = g_base64_encode(obj->value, obj->value_size);
+    } else {
+        if (driverConfig == NULL || driverConfig->secrets_encryption_key == NULL) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("cannot encrypt secret value without encryption key"));
+            return -1;
+        }
+        if (virRandomBytes(iv, sizeof(iv)) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate random IV"));
+            return -1;
+        }
+        if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC,
+                                 driverConfig->secrets_encryption_key, driverConfig->secretsKeyLen,
+                                 iv, sizeof(iv),
+                                 (uint8_t *)obj->value, obj->value_size,
+                                 &encryptedValue, &encryptedValueLen) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to encrypt secret value"));
+            return -1;
+        }
+        base64Len = sizeof(iv) + encryptedValueLen;
+        base64 = g_new0(char, base64Len);
+        memcpy(base64, iv, sizeof(iv));
+        memcpy(base64 + sizeof(iv), encryptedValue, encryptedValueLen);
+        /* Now the secret is encrypted and stored on disk. However,
+         * we did not change anything in the obj->value. This is done on
+         * purpose, as SecretObjGetValue should be able to read it as is.
+         * This will indeed be a base64 encoded secret*/
+    }
     if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
         return -1;
 
@@ -733,27 +788,25 @@ virSecretObjGetValue(virSecretObj *obj)
     return ret;
 }
 
-
 int
 virSecretObjSetValue(virSecretObj *obj,
                      const unsigned char *value,
-                     size_t value_size)
+                     size_t value_size,
+                     virSecretDaemonConfig *driverConfig)
 {
     virSecretDef *def = obj->def;
     g_autofree unsigned char *old_value = NULL;
     g_autofree unsigned char *new_value = NULL;
     size_t old_value_size;
-
     new_value = g_new0(unsigned char, value_size);
 
     old_value = obj->value;
     old_value_size = obj->value_size;
-
     memcpy(new_value, value, value_size);
     obj->value = g_steal_pointer(&new_value);
     obj->value_size = value_size;
 
-    if (!def->isephemeral && virSecretObjSaveData(obj) < 0)
+    if (!def->isephemeral && virSecretObjSaveData(obj, driverConfig) < 0)
         goto error;
 
     /* Saved successfully - drop old value */
@@ -786,7 +839,6 @@ virSecretObjSetValueSize(virSecretObj *obj,
     obj->value_size = value_size;
 }
 
-
 static int
 virSecretLoadValidateUUID(virSecretDef *def,
                           const char *file)
@@ -807,11 +859,18 @@ virSecretLoadValidateUUID(virSecretDef *def,
 
 
 static int
-virSecretLoadValue(virSecretObj *obj)
+virSecretLoadValue(virSecretObj *obj,
+                   virSecretDaemonConfig *driverConfig)
 {
     int ret = -1, fd = -1;
     struct stat st;
     g_autofree char *contents = NULL;
+    g_autofree uint8_t *contents_encrypted = NULL;
+    g_autofree uint8_t *decryptedValue = NULL;
+    size_t decryptedValueLen = 0;
+    uint8_t iv[16] = { 0 };
+    uint8_t *ciphertext = NULL;
+    size_t ciphertextLen = 0;
 
     if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
         if (errno == ENOENT) {
@@ -841,25 +900,65 @@ virSecretLoadValue(virSecretObj *obj)
         goto cleanup;
     }
 
-    contents = g_new0(char, st.st_size + 1);
-
-    if (saferead(fd, contents, st.st_size) != st.st_size) {
-        virReportSystemError(errno, _("cannot read '%1$s'"),
-                             obj->base64File);
-        goto cleanup;
+    if (obj->def->encryption_scheme == VIR_SECRET_ENCRYPTION_SCHEME_NONE ||
+        obj->def->encryption_scheme == -1) {
+            contents = g_new0(char, st.st_size + 1);
+            if (saferead(fd, contents, st.st_size) != st.st_size) {
+                virReportSystemError(errno, _("cannot read '%1$s'"),
+                                     obj->base64File);
+                goto cleanup;
+            }
+            contents[st.st_size] = '\0';
+            obj->value = g_base64_decode(contents, &obj->value_size);
+            if (obj->value == NULL) {
+                virReportError(VIR_ERR_INVALID_SECRET, "%s",
+                               _("cannot decode base64 secret value"));
+                goto cleanup;
+            }
+    } else {
+        if (driverConfig->secrets_encryption_key == NULL) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("cannot decrypt secret value without encryption key"));
+            goto cleanup;
+        }
+        contents_encrypted = g_new0(uint8_t, st.st_size);
+        if (saferead(fd, contents_encrypted, st.st_size) != st.st_size) {
+            virReportSystemError(errno, _("cannot read '%1$s'"),
+                                 obj->base64File);
+            goto cleanup;
+        }
+        if ((st.st_size) < sizeof(iv)) {
+            virReportError(VIR_ERR_INVALID_SECRET, "%s",
+                           _("Encrypted secret size is invalid"));
+            goto cleanup;
+        }
+        memcpy(iv, contents_encrypted, sizeof(iv));
+        ciphertext = contents_encrypted + sizeof(iv);
+        ciphertextLen = st.st_size - sizeof(iv);
+        if (virCryptoDecryptData(VIR_CRYPTO_CIPHER_AES256CBC,
+                                 driverConfig->secrets_encryption_key, driverConfig->secretsKeyLen,
+                                 iv, sizeof(iv),
+                                 ciphertext, ciphertextLen,
+                                 &decryptedValue, &decryptedValueLen) < 0) {
+            virReportError(VIR_ERR_INVALID_SECRET, "%s",
+                           _("Decryption of secret value failed"));
+            goto cleanup;
+        }
+        g_free(obj->value);
+        obj->value = g_steal_pointer(&decryptedValue);
+        obj->value_size = decryptedValueLen;
     }
-    contents[st.st_size] = '\0';
-
-    VIR_FORCE_CLOSE(fd);
-
-    obj->value = g_base64_decode(contents, &obj->value_size);
-
     ret = 0;
 
  cleanup:
-    if (contents != NULL)
-        memset(contents, 0, st.st_size);
+    if (contents != NULL) {
+        memset(contents, 0, st.st_size+1);
+    }
+    if (contents_encrypted != NULL) {
+        memset(contents_encrypted, 0, st.st_size);
+    }
     VIR_FORCE_CLOSE(fd);
+    virSecureErase(iv, sizeof(iv));
     return ret;
 }
 
@@ -868,7 +967,8 @@ static virSecretObj *
 virSecretLoad(virSecretObjList *secrets,
               const char *file,
               const char *path,
-              const char *configDir)
+              const char *configDir,
+              virSecretDaemonConfig *driverConfig)
 {
     g_autoptr(virSecretDef) def = NULL;
     virSecretObj *obj = NULL;
@@ -882,7 +982,7 @@ virSecretLoad(virSecretObjList *secrets,
     if (!(obj = virSecretObjListAdd(secrets, &def, configDir, NULL)))
         return NULL;
 
-    if (virSecretLoadValue(obj) < 0) {
+    if (virSecretLoadValue(obj, driverConfig) < 0) {
         virSecretObjListRemove(secrets, obj);
         g_clear_pointer(&obj, virObjectUnref);
         return NULL;
@@ -894,7 +994,8 @@ virSecretLoad(virSecretObjList *secrets,
 
 int
 virSecretLoadAllConfigs(virSecretObjList *secrets,
-                        const char *configDir)
+                        const char *configDir,
+                        virSecretDaemonConfig *driverConfig)
 {
     g_autoptr(DIR) dir = NULL;
     struct dirent *de;
@@ -915,7 +1016,7 @@ virSecretLoadAllConfigs(virSecretObjList *secrets,
         if (!(path = virFileBuildPath(configDir, de->d_name, NULL)))
             continue;
 
-        if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) {
+        if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir, driverConfig))) {
             VIR_ERROR(_("Error reading secret: %1$s"),
                       virGetLastErrorMessage());
             continue;
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index 17897c5513..f49600a75c 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -23,6 +23,7 @@
 #include "internal.h"
 
 #include "secret_conf.h"
+#include "secret_config.h"
 
 typedef struct _virSecretObj virSecretObj;
 
@@ -86,7 +87,8 @@ int
 virSecretObjSaveConfig(virSecretObj *obj);
 
 int
-virSecretObjSaveData(virSecretObj *obj);
+virSecretObjSaveData(virSecretObj *obj,
+                     virSecretDaemonConfig *driverConfig);
 
 virSecretDef *
 virSecretObjGetDef(virSecretObj *obj);
@@ -101,7 +103,8 @@ virSecretObjGetValue(virSecretObj *obj);
 int
 virSecretObjSetValue(virSecretObj *obj,
                      const unsigned char *value,
-                     size_t value_size);
+                     size_t value_size,
+                     virSecretDaemonConfig *driverConfig);
 
 size_t
 virSecretObjGetValueSize(virSecretObj *obj);
@@ -112,4 +115,5 @@ virSecretObjSetValueSize(virSecretObj *obj,
 
 int
 virSecretLoadAllConfigs(virSecretObjList *secrets,
-                        const char *configDir);
+                        const char *configDir,
+                        virSecretDaemonConfig *cfg);
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 04c3ca49f1..c0cac39a28 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -30,6 +30,7 @@
 #include "virlog.h"
 #include "viralloc.h"
 #include "secret_conf.h"
+#include "secret_config.h"
 #include "virsecretobj.h"
 #include "secret_driver.h"
 #include "virthread.h"
@@ -42,6 +43,10 @@
 #include "secret_event.h"
 #include "virutil.h"
 #include "virinhibitor.h"
+#include "virfile.h"
+#include "virrandom.h"
+#include "vircrypto.h"
+#include "virsecureerase.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECRET
 
@@ -70,6 +75,8 @@ struct _virSecretDriverState {
 
     /* Immutable pointer, self-locking APIs */
     virInhibitor *inhibitor;
+
+    virSecretDaemonConfig *config;
 };
 
 static virSecretDriverState *driver;
@@ -224,7 +231,7 @@ secretDefineXML(virConnectPtr conn,
 
     if (!objDef->isephemeral) {
         if (backup && backup->isephemeral) {
-            if (virSecretObjSaveData(obj) < 0)
+            if (virSecretObjSaveData(obj, driver->config) < 0)
                 goto restore_backup;
         }
 
@@ -307,7 +314,6 @@ secretGetXMLDesc(virSecretPtr secret,
     return ret;
 }
 
-
 static int
 secretSetValue(virSecretPtr secret,
                const unsigned char *value,
@@ -327,8 +333,7 @@ secretSetValue(virSecretPtr secret,
     def = virSecretObjGetDef(obj);
     if (virSecretSetValueEnsureACL(secret->conn, def) < 0)
         goto cleanup;
-
-    if (virSecretObjSetValue(obj, value, value_size) < 0)
+    if (virSecretObjSetValue(obj, value, value_size, driver->config) < 0)
         goto cleanup;
 
     event = virSecretEventValueChangedNew(def->uuid,
@@ -454,6 +459,7 @@ secretStateCleanupLocked(void)
     VIR_FREE(driver->configDir);
 
     virObjectUnref(driver->secretEventState);
+    virObjectUnref(driver->config);
     virInhibitorFree(driver->inhibitor);
 
     if (driver->lockFD != -1)
@@ -518,6 +524,8 @@ secretStateInitialize(bool privileged,
                              driver->stateDir);
         goto error;
     }
+    if (!(driver->config = virSecretDaemonConfigNew(driver->privileged)))
+        goto error;
 
     driver->inhibitor = virInhibitorNew(
         VIR_INHIBITOR_WHAT_NONE,
@@ -534,7 +542,7 @@ secretStateInitialize(bool privileged,
     if (!(driver->secrets = virSecretObjListNew()))
         goto error;
 
-    if (virSecretLoadAllConfigs(driver->secrets, driver->configDir) < 0)
+    if (virSecretLoadAllConfigs(driver->secrets, driver->configDir, driver->config) < 0)
         goto error;
 
     return VIR_DRV_STATE_INIT_COMPLETE;
@@ -553,7 +561,10 @@ secretStateReload(void)
     if (!driver)
         return -1;
 
-    ignore_value(virSecretLoadAllConfigs(driver->secrets, driver->configDir));
+    if (!(driver->config = virSecretDaemonConfigNew(driver->privileged)))
+        return -1;
+
+    ignore_value(virSecretLoadAllConfigs(driver->secrets, driver->configDir, driver->config));
 
     return 0;
 }
-- 
2.51.1

On Thu, Nov 20, 2025 at 22:23:46 +0530, Arun Menon via Devel wrote:
> Since we now have the functionality to provide the secrets driver
> with an encryption key, and the newly introduced attribute to store
> the encryption scheme across driver restarts, we can use the key to encrypt
> secrets. While loading the secrets, we check whether the secret is
> encrypted or not and accordingly get the value.
> 
> While the stored encryption scheme ensures the driver can
> successfully load secrets after a restart,
> If the user changes the encryption key between driver restarts,
> any secrets encrypted with the previous key will become permanently
> inaccessible upon the next restart. Users must ensure key consistency
> to maintain access to existing encrypted secrets.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  src/conf/virsecretobj.c    | 165 ++++++++++++++++++++++++++++++-------
>  src/conf/virsecretobj.h    |  10 ++-
>  src/secret/secret_driver.c |  23 ++++--
>  3 files changed, 157 insertions(+), 41 deletions(-)


All changes related to adding of the secret driver config, parsing etc
ought to be either part of the commit that adds the config parser or a
separate commit after it, but not intermixed with the implementation of
encrypting the individual secrets.

Based on the feedback to 4/5 we also don't want to store the secret type
in the XML so I'll not comment on anything related to that.,

> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 66270e2751..37b2db960f 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c



> @@ -328,6 +332,8 @@ virSecretObjListAdd(virSecretObjList *secrets,
>      virSecretObj *obj;
>      virSecretDef *objdef;
>      virSecretObj *ret = NULL;
> +    const char *encryptionScheme = NULL;
> +    const char *encryptionSchemeExt = NULL;

You declare these as const, then assign string copies into them and then
use g_free on this. Use normal strings instead and auto-free them.

>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>  
>      virObjectRWLockWrite(secrets);
> @@ -379,10 +385,26 @@ virSecretObjListAdd(virSecretObjList *secrets,
>              goto cleanup;
>  
>          /* Generate the possible configFile and base64File strings
> -         * using the configDir, uuidstr, and appropriate suffix
> +         * using the configDir, uuidstr, and appropriate encryption scheme
>           */
> -        if (!(obj->configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
> -            !(obj->base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
> +        if ((*newdef)->encryption_scheme != VIR_SECRET_ENCRYPTION_SCHEME_NONE
> +            && (*newdef)->encryption_scheme != -1) {
> +            encryptionScheme = virSecretEncryptionSchemeTypeToString((*newdef)->encryption_scheme);
> +            if (!encryptionScheme) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("unknown secret encryption scheme %1$d"), (*newdef)->encryption_scheme);
> +                goto cleanup;
> +            }
> +            encryptionSchemeExt = g_strconcat(".", encryptionScheme, NULL);
> +            if (!(obj->base64File = virFileBuildPath(configDir, uuidstr, encryptionSchemeExt))) {
> +                goto cleanup;
> +            }
> +        } else {
> +            if (!(obj->base64File = virFileBuildPath(configDir, uuidstr, ".base64"))) {
> +                goto cleanup;
> +            }
> +        }

You'll need to probe instead which of the suffixes we support exist  in
order from the most preffered one.

Also rename the variable holding the filename to something like
'secretValueFile'.


> +        if (!(obj->configFile = virFileBuildPath(configDir, uuidstr, ".xml")))
>              goto cleanup;
>  
>          if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)

[...]

> @@ -680,17 +703,49 @@ virSecretObjSaveConfig(virSecretObj *obj)
>      return 0;
>  }
>  
> -
>  int
> -virSecretObjSaveData(virSecretObj *obj)
> +virSecretObjSaveData(virSecretObj *obj,
> +                     virSecretDaemonConfig *driverConfig)
>  {
>      g_autofree char *base64 = NULL;
> +    g_autofree uint8_t *encryptedValue = NULL;
> +    size_t encryptedValueLen = 0;
> +    size_t base64Len = 0;
> +    uint8_t iv[16] = { 0 };
>  
>      if (!obj->value)
>          return 0;
>  
> -    base64 = g_base64_encode(obj->value, obj->value_size);
> -
> +    if (obj->def->encryption_scheme == VIR_SECRET_ENCRYPTION_SCHEME_NONE
> +        || obj->def->encryption_scheme == -1) {

coding style

> +        base64 = g_base64_encode(obj->value, obj->value_size);
> +    } else {
> +        if (driverConfig == NULL || driverConfig->secrets_encryption_key == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("cannot encrypt secret value without encryption key"));
> +            return -1;
> +        }
> +        if (virRandomBytes(iv, sizeof(iv)) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to generate random IV"));

virRandomBytes already reports an error; we usually don't overwrite them

> +            return -1;
> +        }
> +        if (virCryptoEncryptData(VIR_CRYPTO_CIPHER_AES256CBC,
> +                                 driverConfig->secrets_encryption_key, driverConfig->secretsKeyLen,
> +                                 iv, sizeof(iv),
> +                                 (uint8_t *)obj->value, obj->value_size,
> +                                 &encryptedValue, &encryptedValueLen) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to encrypt secret value"));

Same here.

> +            return -1;
> +        }
> +        base64Len = sizeof(iv) + encryptedValueLen;
> +        base64 = g_new0(char, base64Len);
> +        memcpy(base64, iv, sizeof(iv));
> +        memcpy(base64 + sizeof(iv), encryptedValue, encryptedValueLen);
> +        /* Now the secret is encrypted and stored on disk. However,
> +         * we did not change anything in the obj->value. This is done on
> +         * purpose, as SecretObjGetValue should be able to read it as is.
> +         * This will indeed be a base64 encoded secret*/

I didn't really go check why we store these as base64 but I'd suggest
that the encrypted value is still stored as base64

> +    }
>      if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
>          return -1;
>  
> @@ -733,27 +788,25 @@ virSecretObjGetValue(virSecretObj *obj)
>      return ret;
>  }
>  
> -
>  int

Spurious whitespace change. Our coding style prefers 2 empty lines
between functions.

>  virSecretObjSetValue(virSecretObj *obj,
>                       const unsigned char *value,
> -                     size_t value_size)
> +                     size_t value_size,
> +                     virSecretDaemonConfig *driverConfig)
>  {
>      virSecretDef *def = obj->def;
>      g_autofree unsigned char *old_value = NULL;
>      g_autofree unsigned char *new_value = NULL;
>      size_t old_value_size;
> -
>      new_value = g_new0(unsigned char, value_size);
>  
>      old_value = obj->value;
>      old_value_size = obj->value_size;
> -
>      memcpy(new_value, value, value_size);
>      obj->value = g_steal_pointer(&new_value);
>      obj->value_size = value_size;

All of the whitespace changes are spurious.

>  
> -    if (!def->isephemeral && virSecretObjSaveData(obj) < 0)
> +    if (!def->isephemeral && virSecretObjSaveData(obj, driverConfig) < 0)
>          goto error;
>  
>      /* Saved successfully - drop old value */
> @@ -786,7 +839,6 @@ virSecretObjSetValueSize(virSecretObj *obj,
>      obj->value_size = value_size;
>  }
>  
> -
>  static int

ditto

>  virSecretLoadValidateUUID(virSecretDef *def,
>                            const char *file)
> @@ -807,11 +859,18 @@ virSecretLoadValidateUUID(virSecretDef *def,
>  
>  
>  static int
> -virSecretLoadValue(virSecretObj *obj)
> +virSecretLoadValue(virSecretObj *obj,
> +                   virSecretDaemonConfig *driverConfig)
>  {
>      int ret = -1, fd = -1;
>      struct stat st;
>      g_autofree char *contents = NULL;
> +    g_autofree uint8_t *contents_encrypted = NULL;
> +    g_autofree uint8_t *decryptedValue = NULL;
> +    size_t decryptedValueLen = 0;
> +    uint8_t iv[16] = { 0 };
> +    uint8_t *ciphertext = NULL;
> +    size_t ciphertextLen = 0;
>  
>      if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
>          if (errno == ENOENT) {
> @@ -841,25 +900,65 @@ virSecretLoadValue(virSecretObj *obj)
>          goto cleanup;
>      }
>  
> -    contents = g_new0(char, st.st_size + 1);
> -
> -    if (saferead(fd, contents, st.st_size) != st.st_size) {
> -        virReportSystemError(errno, _("cannot read '%1$s'"),
> -                             obj->base64File);
> -        goto cleanup;
> +    if (obj->def->encryption_scheme == VIR_SECRET_ENCRYPTION_SCHEME_NONE ||
> +        obj->def->encryption_scheme == -1) {
> +            contents = g_new0(char, st.st_size + 1);
> +            if (saferead(fd, contents, st.st_size) != st.st_size) {
> +                virReportSystemError(errno, _("cannot read '%1$s'"),
> +                                     obj->base64File);
> +                goto cleanup;
> +            }
> +            contents[st.st_size] = '\0';
> +            obj->value = g_base64_decode(contents, &obj->value_size);
> +            if (obj->value == NULL) {
> +                virReportError(VIR_ERR_INVALID_SECRET, "%s",
> +                               _("cannot decode base64 secret value"));
> +                goto cleanup;
> +            }
> +    } else {
> +        if (driverConfig->secrets_encryption_key == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("cannot decrypt secret value without encryption key"));
> +            goto cleanup;
> +        }
> +        contents_encrypted = g_new0(uint8_t, st.st_size);
> +        if (saferead(fd, contents_encrypted, st.st_size) != st.st_size) {
> +            virReportSystemError(errno, _("cannot read '%1$s'"),
> +                                 obj->base64File);
> +            goto cleanup;
> +        }
> +        if ((st.st_size) < sizeof(iv)) {
> +            virReportError(VIR_ERR_INVALID_SECRET, "%s",
> +                           _("Encrypted secret size is invalid"));
> +            goto cleanup;
> +        }
> +        memcpy(iv, contents_encrypted, sizeof(iv));
> +        ciphertext = contents_encrypted + sizeof(iv);
> +        ciphertextLen = st.st_size - sizeof(iv);
> +        if (virCryptoDecryptData(VIR_CRYPTO_CIPHER_AES256CBC,
> +                                 driverConfig->secrets_encryption_key, driverConfig->secretsKeyLen,
> +                                 iv, sizeof(iv),
> +                                 ciphertext, ciphertextLen,
> +                                 &decryptedValue, &decryptedValueLen) < 0) {
> +            virReportError(VIR_ERR_INVALID_SECRET, "%s",
> +                           _("Decryption of secret value failed"));
> +            goto cleanup;
> +        }
> +        g_free(obj->value);
> +        obj->value = g_steal_pointer(&decryptedValue);
> +        obj->value_size = decryptedValueLen;
>      }
> -    contents[st.st_size] = '\0';
> -
> -    VIR_FORCE_CLOSE(fd);
> -
> -    obj->value = g_base64_decode(contents, &obj->value_size);
> -
>      ret = 0;
>  
>   cleanup:
> -    if (contents != NULL)
> -        memset(contents, 0, st.st_size);
> +    if (contents != NULL) {
> +        memset(contents, 0, st.st_size+1);
> +    }
> +    if (contents_encrypted != NULL) {
> +        memset(contents_encrypted, 0, st.st_size);
> +    }

Use virSecureErase; memset can be optimized out. Also for any existing
code.


>      VIR_FORCE_CLOSE(fd);
> +    virSecureErase(iv, sizeof(iv));
>      return ret;
>  }

[...]


> @@ -70,6 +75,8 @@ struct _virSecretDriverState {
>  
>      /* Immutable pointer, self-locking APIs */
>      virInhibitor *inhibitor;
> +
> +    virSecretDaemonConfig *config;

Please annotate this one as the other pointers are.