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 :|
On Fri, 21 Nov, 2025, 3:22 pm Daniel P. Berrangé, <berrange@redhat.com> wrote: > 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 :| > Do you mean that we should not be using encryption scheme at all. While loading the secrets, all we need to do is check the file name suffix and if it is not ".base64", then enter the decryption logic? Regards, Arun Menon he/him Software Engineer, Virtualization Red Hat APAC
On Fri, Nov 21, 2025 at 20:20:56 +0530, Arun Menon wrote: > On Fri, 21 Nov, 2025, 3:22 pm Daniel P. Berrangé, <berrange@redhat.com> > wrote: > > > 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. [...] > Do you mean that we should not be using encryption scheme at all. While > loading the secrets, all we need to do is check the file name suffix and if > it is not ".base64", then enter the decryption logic? You can encode which cipher was used in the suffix of the filename e.g: SECRETNAME.aes256, SECRETNAME.base64 If we ever switch to another cipher we'll use a different suffix. And when loading them look at the most recently used cipher/format first, and go back the list. You then can either remember which ciplher one was used internally, so that we use the same on output, or we'll unconditionally write out in the newest format possible. In the latter case you must ensure that the older format file was wiped once the value is written out.
On Fri, Nov 21, 2025 at 8:38 PM Peter Krempa <pkrempa@redhat.com> wrote: > On Fri, Nov 21, 2025 at 20:20:56 +0530, Arun Menon wrote: > > On Fri, 21 Nov, 2025, 3:22 pm Daniel P. Berrangé, <berrange@redhat.com> > > wrote: > > > > > 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. > > [...] > > > Do you mean that we should not be using encryption scheme at all. While > > loading the secrets, all we need to do is check the file name suffix and > if > > it is not ".base64", then enter the decryption logic? > > You can encode which cipher was used in the suffix of the filename e.g: > > SECRETNAME.aes256, SECRETNAME.base64 > > If we ever switch to another cipher we'll use a different suffix. > > And when loading them look at the most recently used cipher/format > first, and go back the list. > > You then can either remember which ciplher one was used internally, so > that we use the same on output, or we'll unconditionally write out in > the newest format possible. In the latter case you must ensure that the > older format file was wiped once the value is written out. > I see. I was only afraid of the scenario where we migrate from an older version of libvirt to the new one and the encryption scheme changes in between. A secret on the disk which was encrypted with scheme A will not be loaded automatically, because the new scheme will not be able to decrypt it.
On Fri, Nov 21, 2025 at 21:05:23 +0530, Arun Menon wrote: > On Fri, Nov 21, 2025 at 8:38 PM Peter Krempa <pkrempa@redhat.com> wrote: > > > On Fri, Nov 21, 2025 at 20:20:56 +0530, Arun Menon wrote: > > > On Fri, 21 Nov, 2025, 3:22 pm Daniel P. Berrangé, <berrange@redhat.com> [...] > > > Do you mean that we should not be using encryption scheme at all. While > > > loading the secrets, all we need to do is check the file name suffix and > > if > > > it is not ".base64", then enter the decryption logic? > > > > You can encode which cipher was used in the suffix of the filename e.g: > > > > SECRETNAME.aes256, SECRETNAME.base64 > > > > If we ever switch to another cipher we'll use a different suffix. > > > > And when loading them look at the most recently used cipher/format > > first, and go back the list. > > > > You then can either remember which ciplher one was used internally, so > > that we use the same on output, or we'll unconditionally write out in > > the newest format possible. In the latter case you must ensure that the > > older format file was wiped once the value is written out. > > > > I see. I was only afraid of the scenario where we migrate from an older > version of libvirt to the new one and the encryption scheme changes in > between. > A secret on the disk which was encrypted with scheme A will not be loaded > automatically, because the new scheme will not be able to decrypt it. Yes you definitely need to be able to tell which cipher/method was used on the actual on-disk file. But since we don't want to expose it the filename seems to be the most straightforward solution.
© 2016 - 2026 Red Hat, Inc.