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. > Signed-off-by: Arun Menon <[email protected]> > --- > 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); > } [...]
