A new attribute is required, to store the encryption scheme used while
encrypting the secret. This value will be "none" if the secret is
stored in base64 format.
For backwards compatibility, the secret will not be encrypted when the
attribute itself is absent in the configuration file. In other words,
the secret will be stored on the disk in base64 encoded format.
This new attribute is essential to be stored on the disk in the xml file,
so that we can effectively decrypt the secrets while loading them.
It also allows us to add more encryption schemes in the future.
Signed-off-by: Arun Menon <armenon@redhat.com>
---
include/libvirt/libvirt-secret.h | 20 ++++++++++++++++++++
src/conf/schemas/secret.rng | 5 +++++
src/conf/secret_conf.c | 21 +++++++++++++++++++++
src/conf/secret_conf.h | 1 +
src/util/virsecret.c | 4 ++++
src/util/virsecret.h | 1 +
tests/secretxml2xmlin/usage-ceph-space.xml | 1 +
tests/secretxml2xmlin/usage-ceph.xml | 1 +
tests/secretxml2xmlin/usage-iscsi.xml | 1 +
tests/secretxml2xmlin/usage-tls.xml | 1 +
tests/secretxml2xmlin/usage-volume.xml | 1 +
tests/secretxml2xmlin/usage-vtpm.xml | 1 +
12 files changed, 58 insertions(+)
diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h
index 761437d4ad..96a4359107 100644
--- a/include/libvirt/libvirt-secret.h
+++ b/include/libvirt/libvirt-secret.h
@@ -70,6 +70,26 @@ typedef enum {
# endif
} virSecretUsageType;
+/**
+ * virSecretEncryptionSchemeType:
+ *
+ * Since: 11.10.0
+ */
+typedef enum {
+ VIR_SECRET_ENCRYPTION_SCHEME_NONE = 0, /* (Since: 11.10.0) */
+ VIR_SECRET_ENCRYPTION_SCHEME_AES256CBS = 1, /* (Since: 11.10.0) */
+# ifdef VIR_ENUM_SENTINELS
+ VIR_SECRET_ENCRYPTION_SCHEME_LAST
+ /*
+ * NB: this enum value will increase over time as new encryption schemes are
+ * added to the libvirt API. It reflects the last enncryption scheme supported
+ * by this version of the libvirt API.
+ *
+ * Since: 11.10.0
+ */
+# endif
+} virSecretEncryptionSchemeType;
+
virConnectPtr virSecretGetConnect (virSecretPtr secret);
int virConnectNumOfSecrets (virConnectPtr conn);
int virConnectListSecrets (virConnectPtr conn,
diff --git a/src/conf/schemas/secret.rng b/src/conf/schemas/secret.rng
index c90e2eb81f..ae6e62b438 100644
--- a/src/conf/schemas/secret.rng
+++ b/src/conf/schemas/secret.rng
@@ -42,6 +42,11 @@
</choice>
</element>
</optional>
+ <optional>
+ <element name="encryptionScheme">
+ <text/>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 966536599e..2fdf3f7f2c 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -131,6 +131,12 @@ virSecretParseXML(xmlXPathContext *ctxt)
g_autofree char *ephemeralstr = NULL;
g_autofree char *privatestr = NULL;
g_autofree char *uuidstr = NULL;
+ g_autofree char *encryptionScheme = NULL;
+
+ /* Encryption scheme is set to -1 to support existing xml secret configuration
+ * files. This indicates that no encryption scheme is specified in the XML
+ */
+ int type = -1;
def = g_new0(virSecretDef, 1);
@@ -170,6 +176,15 @@ virSecretParseXML(xmlXPathContext *ctxt)
if (virSecretDefParseUsage(ctxt, def) < 0)
return NULL;
+ encryptionScheme = virXPathString("string(./encryptionScheme)", ctxt);
+ if (encryptionScheme) {
+ if ((type = virSecretEncryptionSchemeTypeFromString(encryptionScheme)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown secret encryption scheme %1$d"), def->encryption_scheme);
+ return NULL;
+ }
+ }
+ def->encryption_scheme = type;
return g_steal_pointer(&def);
}
@@ -242,6 +257,7 @@ virSecretDefFormat(const virSecretDef *def)
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(&buf);
+ const char *type = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
virBufferAsprintf(&attrBuf, " ephemeral='%s' private='%s'",
@@ -257,6 +273,11 @@ virSecretDefFormat(const virSecretDef *def)
virSecretDefFormatUsage(&childBuf, def) < 0)
return NULL;
+ type = virSecretEncryptionSchemeTypeToString(def->encryption_scheme);
+ if (type != NULL) {
+ virBufferEscapeString(&childBuf, "<encryptionScheme>%s</encryptionScheme>\n",
+ type);
+ }
virXMLFormatElement(&buf, "secret", &attrBuf, &childBuf);
return virBufferContentAndReset(&buf);
}
diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
index 8f8f47933a..a12bc8e095 100644
--- a/src/conf/secret_conf.h
+++ b/src/conf/secret_conf.h
@@ -30,6 +30,7 @@ struct _virSecretDef {
char *description; /* May be NULL */
virSecretUsageType usage_type;
char *usage_id; /* May be NULL */
+ virSecretEncryptionSchemeType encryption_scheme; /* virSecretEncryptionSchemeType */
};
void virSecretDefFree(virSecretDef *def);
diff --git a/src/util/virsecret.c b/src/util/virsecret.c
index 8e74df3b93..c9d9cf2c8a 100644
--- a/src/util/virsecret.c
+++ b/src/util/virsecret.c
@@ -36,6 +36,10 @@ VIR_ENUM_IMPL(virSecretUsage,
VIR_SECRET_USAGE_TYPE_LAST,
"none", "volume", "ceph", "iscsi", "tls", "vtpm",
);
+VIR_ENUM_IMPL(virSecretEncryptionScheme,
+ VIR_SECRET_ENCRYPTION_SCHEME_LAST,
+ "none", "aes256cbc",
+);
void
virSecretLookupDefClear(virSecretLookupTypeDef *def)
diff --git a/src/util/virsecret.h b/src/util/virsecret.h
index c803f0fe33..01998e307d 100644
--- a/src/util/virsecret.h
+++ b/src/util/virsecret.h
@@ -27,6 +27,7 @@
#include "virenum.h"
VIR_ENUM_DECL(virSecretUsage);
+VIR_ENUM_DECL(virSecretEncryptionScheme);
typedef enum {
VIR_SECRET_LOOKUP_TYPE_NONE,
diff --git a/tests/secretxml2xmlin/usage-ceph-space.xml b/tests/secretxml2xmlin/usage-ceph-space.xml
index 557b12474d..2a7a177931 100644
--- a/tests/secretxml2xmlin/usage-ceph-space.xml
+++ b/tests/secretxml2xmlin/usage-ceph-space.xml
@@ -4,4 +4,5 @@
<usage type='ceph'>
<name>client.admin secret</name>
</usage>
+ <encryptionScheme>none</encryptionScheme>
</secret>
diff --git a/tests/secretxml2xmlin/usage-ceph.xml b/tests/secretxml2xmlin/usage-ceph.xml
index e880293a63..8a2501c21f 100644
--- a/tests/secretxml2xmlin/usage-ceph.xml
+++ b/tests/secretxml2xmlin/usage-ceph.xml
@@ -4,4 +4,5 @@
<usage type='ceph'>
<name>CephCephCephCeph</name>
</usage>
+ <encryptionScheme>none</encryptionScheme>
</secret>
diff --git a/tests/secretxml2xmlin/usage-iscsi.xml b/tests/secretxml2xmlin/usage-iscsi.xml
index bfc94722e0..c36a0f8661 100644
--- a/tests/secretxml2xmlin/usage-iscsi.xml
+++ b/tests/secretxml2xmlin/usage-iscsi.xml
@@ -4,4 +4,5 @@
<usage type='iscsi'>
<target>iscsitarget</target>
</usage>
+ <encryptionScheme>none</encryptionScheme>
</secret>
diff --git a/tests/secretxml2xmlin/usage-tls.xml b/tests/secretxml2xmlin/usage-tls.xml
index 88068b56e0..a021e96279 100644
--- a/tests/secretxml2xmlin/usage-tls.xml
+++ b/tests/secretxml2xmlin/usage-tls.xml
@@ -4,4 +4,5 @@
<usage type='tls'>
<name>mumblyfratz</name>
</usage>
+ <encryptionScheme>none</encryptionScheme>
</secret>
diff --git a/tests/secretxml2xmlin/usage-volume.xml b/tests/secretxml2xmlin/usage-volume.xml
index e273c57686..7f9a4e13b8 100644
--- a/tests/secretxml2xmlin/usage-volume.xml
+++ b/tests/secretxml2xmlin/usage-volume.xml
@@ -4,4 +4,5 @@
<usage type='volume'>
<volume>/var/lib/libvirt/images/image.img</volume>
</usage>
+ <encryptionScheme>none</encryptionScheme>
</secret>
diff --git a/tests/secretxml2xmlin/usage-vtpm.xml b/tests/secretxml2xmlin/usage-vtpm.xml
index 5baff3034d..f9b801f765 100644
--- a/tests/secretxml2xmlin/usage-vtpm.xml
+++ b/tests/secretxml2xmlin/usage-vtpm.xml
@@ -4,4 +4,5 @@
<usage type='vtpm'>
<name>vTPMvTPMvTPM</name>
</usage>
+ <encryptionScheme>aes256cbc</encryptionScheme>
</secret>
--
2.51.1On Thu, Nov 20, 2025 at 22:23:45 +0530, Arun Menon via Devel wrote:
> A new attribute is required, to store the encryption scheme used while
> encrypting the secret. This value will be "none" if the secret is
> stored in base64 format.
>
> For backwards compatibility, the secret will not be encrypted when the
> attribute itself is absent in the configuration file. In other words,
> the secret will be stored on the disk in base64 encoded format.
>
> This new attribute is essential to be stored on the disk in the xml file,
> so that we can effectively decrypt the secrets while loading them.
> It also allows us to add more encryption schemes in the future.
Storing it in the XML also gives the user the possibility to control the
encryption. Do we want that? The only reason I see for a user to
configure this field is to ensure that secrets are "downgraded" to an
older encryption scheme, but I'm not sure if that is actually useful.
Shouldn't this remain an implementation detail and thus not exposed to
the user? In such case you could tell them apart e.g. using a filename
suffix.
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> include/libvirt/libvirt-secret.h | 20 ++++++++++++++++++++
> src/conf/schemas/secret.rng | 5 +++++
> src/conf/secret_conf.c | 21 +++++++++++++++++++++
> src/conf/secret_conf.h | 1 +
> src/util/virsecret.c | 4 ++++
> src/util/virsecret.h | 1 +
> tests/secretxml2xmlin/usage-ceph-space.xml | 1 +
> tests/secretxml2xmlin/usage-ceph.xml | 1 +
> tests/secretxml2xmlin/usage-iscsi.xml | 1 +
> tests/secretxml2xmlin/usage-tls.xml | 1 +
> tests/secretxml2xmlin/usage-volume.xml | 1 +
> tests/secretxml2xmlin/usage-vtpm.xml | 1 +
> 12 files changed, 58 insertions(+)
>
> diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h
> index 761437d4ad..96a4359107 100644
> --- a/include/libvirt/libvirt-secret.h
> +++ b/include/libvirt/libvirt-secret.h
> @@ -70,6 +70,26 @@ typedef enum {
> # endif
> } virSecretUsageType;
>
> +/**
> + * virSecretEncryptionSchemeType:
> + *
> + * Since: 11.10.0
Note that libvirt will enter freeze on the 25th of November. You'll need
to use 12.0.0 after that.
> + */
> +typedef enum {
> + VIR_SECRET_ENCRYPTION_SCHEME_NONE = 0, /* (Since: 11.10.0) */
> + VIR_SECRET_ENCRYPTION_SCHEME_AES256CBS = 1, /* (Since: 11.10.0) */
AES256CBC I presume
> +# ifdef VIR_ENUM_SENTINELS
> + VIR_SECRET_ENCRYPTION_SCHEME_LAST
> + /*
> + * NB: this enum value will increase over time as new encryption schemes are
> + * added to the libvirt API. It reflects the last enncryption scheme supported
> + * by this version of the libvirt API.
> + *
> + * Since: 11.10.0
> + */
> +# endif
> +} virSecretEncryptionSchemeType;
Looking at the code this seems to be only the type for the variable
holding the parsed XML.
Since no API uses this users don't actually have possibility to see what
the string values represented by the enum.
> +
> virConnectPtr virSecretGetConnect (virSecretPtr secret);
> int virConnectNumOfSecrets (virConnectPtr conn);
> int virConnectListSecrets (virConnectPtr conn,
> diff --git a/src/conf/schemas/secret.rng b/src/conf/schemas/secret.rng
> index c90e2eb81f..ae6e62b438 100644
> --- a/src/conf/schemas/secret.rng
> +++ b/src/conf/schemas/secret.rng
> @@ -42,6 +42,11 @@
> </choice>
> </element>
> </optional>
> + <optional>
> + <element name="encryptionScheme">
> + <text/>
Since this is an enumeration field please enumerate all approved values
as <choice>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
> index 966536599e..2fdf3f7f2c 100644
> --- a/src/conf/secret_conf.c
> +++ b/src/conf/secret_conf.c
> @@ -131,6 +131,12 @@ virSecretParseXML(xmlXPathContext *ctxt)
> g_autofree char *ephemeralstr = NULL;
> g_autofree char *privatestr = NULL;
> g_autofree char *uuidstr = NULL;
> + g_autofree char *encryptionScheme = NULL;
> +
> + /* Encryption scheme is set to -1 to support existing xml secret configuration
> + * files. This indicates that no encryption scheme is specified in the XML
> + */
> + int type = -1;
>
> def = g_new0(virSecretDef, 1);
>
> @@ -170,6 +176,15 @@ virSecretParseXML(xmlXPathContext *ctxt)
> if (virSecretDefParseUsage(ctxt, def) < 0)
> return NULL;
>
> + encryptionScheme = virXPathString("string(./encryptionScheme)", ctxt);
> + if (encryptionScheme) {
> + if ((type = virSecretEncryptionSchemeTypeFromString(encryptionScheme)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
Use of VIR_ERR_INTERNAL_ERROR is not appropriate for errors coming from
parsing user-originated XML. Please use something more appropriate such
as VIR_ERR_XML_ERROR
> + _("Unknown secret encryption scheme %1$d"), def->encryption_scheme);
> + return NULL;
> + }
> + }
> + def->encryption_scheme = type;
The 'encryption_shceme' member is declared as ...
> diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
> index 8f8f47933a..a12bc8e095 100644
> --- a/src/conf/secret_conf.h
> +++ b/src/conf/secret_conf.h
> @@ -30,6 +30,7 @@ struct _virSecretDef {
> char *description; /* May be NULL */
> virSecretUsageType usage_type;
> char *usage_id; /* May be NULL */
> + virSecretEncryptionSchemeType encryption_scheme; /* virSecretEncryptionSchemeType */
> };
>
which is an enum with natural numbers and thus the compiler considers it
to be unsighed, yet tyou have the possibility to assign -1.
Also no need to state the type in the comment if you declare it as such.
> return g_steal_pointer(&def);
> }
>
> @@ -242,6 +257,7 @@ virSecretDefFormat(const virSecretDef *def)
> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
> g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(&buf);
> + const char *type = NULL;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> virBufferAsprintf(&attrBuf, " ephemeral='%s' private='%s'",
> @@ -257,6 +273,11 @@ virSecretDefFormat(const virSecretDef *def)
> virSecretDefFormatUsage(&childBuf, def) < 0)
> return NULL;
>
> + type = virSecretEncryptionSchemeTypeToString(def->encryption_scheme);
> + if (type != NULL) {
> + virBufferEscapeString(&childBuf, "<encryptionScheme>%s</encryptionScheme>\n",
> + type);
'virBufferEscapeString' is NULL, tolerant on the 3rd parameter and
actually skips the whole string to format if the arg is NULL. Thus you
don't need the extra block and variable here.
> + }
> virXMLFormatElement(&buf, "secret", &attrBuf, &childBuf);
> return virBufferContentAndReset(&buf);
> }
[...]
On Fri, Nov 21, 2025 at 08:08:20AM +0100, Peter Krempa via Devel wrote: > On Thu, Nov 20, 2025 at 22:23:45 +0530, Arun Menon via Devel wrote: > > A new attribute is required, to store the encryption scheme used while > > encrypting the secret. This value will be "none" if the secret is > > stored in base64 format. > > > > For backwards compatibility, the secret will not be encrypted when the > > attribute itself is absent in the configuration file. In other words, > > the secret will be stored on the disk in base64 encoded format. > > > > This new attribute is essential to be stored on the disk in the xml file, > > so that we can effectively decrypt the secrets while loading them. > > It also allows us to add more encryption schemes in the future. > > Storing it in the XML also gives the user the possibility to control the > encryption. Do we want that? The only reason I see for a user to > configure this field is to ensure that secrets are "downgraded" to an > older encryption scheme, but I'm not sure if that is actually useful. > > Shouldn't this remain an implementation detail and thus not exposed to > the user? In such case you could tell them apart e.g. using a filename > suffix. Agreed, the file suffix is all we want - the backend impl choice should not be exposed to the end user XML. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.