On Tue, Feb 13, 2024 at 05:16:06PM +0100, Michal Privoznik wrote:
> This attribute exists next to @ephemeral and @private attributes
> and controls whether the secret value is encrypted using system's
> TPM chip before stored on disk. Obviously, it's mutually
> exclusive with @ephemeral which forces us to keep the secret
> value in memory only.
>
> In the long run, we can even encrypt secret values that are kept
> in memory (so they can't be obtained by dumping virtsecretd's
> memory). But that's not what is being implemented here.
I'm not really convinced we need a new XML attribute. The storage
backend for the secret driver is an impl detail, and we should
just always use the best option we have available. If we ever do
have multiple storage backends for secrets, then it feels more
like an /etc/libvirt/secret.conf setting to sepcify which backend
to use, as a host admin choice.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> docs/formatsecret.rst | 8 ++++++--
> src/conf/schemas/secret.rng | 5 +++++
> src/conf/secret_conf.c | 17 +++++++++++++++++
> src/conf/secret_conf.h | 2 ++
> tests/secretxml2xmlin/usage-tpm-vtpm.xml | 7 +++++++
> tests/secretxml2xmltest.c | 1 +
> 6 files changed, 38 insertions(+), 2 deletions(-)
> create mode 100644 tests/secretxml2xmlin/usage-tpm-vtpm.xml
>
> diff --git a/docs/formatsecret.rst b/docs/formatsecret.rst
> index d0c37ff165..c7910aecce 100644
> --- a/docs/formatsecret.rst
> +++ b/docs/formatsecret.rst
> @@ -10,14 +10,18 @@ Secret XML
> ----------
>
> Secrets stored by libvirt may have attributes associated with them, using the
> -``secret`` element. The ``secret`` element has two optional attributes, each
> -with values '``yes``' and '``no``', and defaulting to '``no``':
> +``secret`` element. The ``secret`` element has the following optional
> +attributes, each with values '``yes``' and '``no``', and defaulting to
> +'``no``':
>
> ``ephemeral``
> This secret must only be kept in memory, never stored persistently.
> ``private``
> The value of the secret must not be revealed to any caller of libvirt, nor to
> any other node.
> +``tpm``
> + The value of the secret is stored using a key that's derived from the
> + system's TPM2 chip. This is mutually exclusive with ``ephemeral``.
>
> The top-level ``secret`` element may contain the following elements:
>
> diff --git a/src/conf/schemas/secret.rng b/src/conf/schemas/secret.rng
> index c90e2eb81f..59d825bf91 100644
> --- a/src/conf/schemas/secret.rng
> +++ b/src/conf/schemas/secret.rng
> @@ -19,6 +19,11 @@
> <ref name="virYesNo"/>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="tpm">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> <interleave>
> <optional>
> <element name="uuid">
> diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
> index 966536599e..1ab6cf5cd4 100644
> --- a/src/conf/secret_conf.c
> +++ b/src/conf/secret_conf.c
> @@ -130,6 +130,7 @@ virSecretParseXML(xmlXPathContext *ctxt)
> g_autoptr(virSecretDef) def = NULL;
> g_autofree char *ephemeralstr = NULL;
> g_autofree char *privatestr = NULL;
> + g_autofree char *tpmstr = NULL;
> g_autofree char *uuidstr = NULL;
>
> def = g_new0(virSecretDef, 1);
> @@ -150,6 +151,17 @@ virSecretParseXML(xmlXPathContext *ctxt)
> }
> }
>
> + if (virXMLPropTristateBool(ctxt->node, "tpm",
> + VIR_XML_PROP_NONE, &def->tpm) < 0) {
> + return NULL;
> + }
> +
> + if (def->tpm == VIR_TRISTATE_BOOL_YES && def->isephemeral) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("ephemeral and tpm are mutually exclusive"));
> + return NULL;
> + }
> +
> uuidstr = virXPathString("string(./uuid)", ctxt);
> if (!uuidstr) {
> if (virUUIDGenerate(def->uuid) < 0) {
> @@ -248,6 +260,11 @@ virSecretDefFormat(const virSecretDef *def)
> def->isephemeral ? "yes" : "no",
> def->isprivate ? "yes" : "no");
>
> + if (def->tpm != VIR_TRISTATE_BOOL_ABSENT) {
> + virBufferAsprintf(&attrBuf, " tpm='%s'",
> + virTristateBoolTypeToString(def->tpm));
> + }
> +
> virUUIDFormat(def->uuid, uuidstr);
> virBufferEscapeString(&childBuf, "<uuid>%s</uuid>\n", uuidstr);
> if (def->description != NULL)
> diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h
> index 8f8f47933a..8a9e382c1e 100644
> --- a/src/conf/secret_conf.h
> +++ b/src/conf/secret_conf.h
> @@ -21,11 +21,13 @@
> #pragma once
>
> #include "internal.h"
> +#include "virenum.h"
>
> typedef struct _virSecretDef virSecretDef;
> struct _virSecretDef {
> bool isephemeral;
> bool isprivate;
> + virTristateBool tpm;
> unsigned char uuid[VIR_UUID_BUFLEN];
> char *description; /* May be NULL */
> virSecretUsageType usage_type;
> diff --git a/tests/secretxml2xmlin/usage-tpm-vtpm.xml b/tests/secretxml2xmlin/usage-tpm-vtpm.xml
> new file mode 100644
> index 0000000000..b70785113c
> --- /dev/null
> +++ b/tests/secretxml2xmlin/usage-tpm-vtpm.xml
> @@ -0,0 +1,7 @@
> +<secret ephemeral='no' private='yes' tpm='yes'>
> + <uuid>46b96412-fffc-46a3-bf3d-c371f776cadb</uuid>
> + <description>vTPM secret</description>
> + <usage type='vtpm'>
> + <name>vTPMvTPMvTPM</name>
> + </usage>
> +</secret>
> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c
> index eb4d3e143c..cdd34546b2 100644
> --- a/tests/secretxml2xmltest.c
> +++ b/tests/secretxml2xmltest.c
> @@ -69,6 +69,7 @@ mymain(void)
> DO_TEST("usage-iscsi");
> DO_TEST("usage-tls");
> DO_TEST("usage-vtpm");
> + DO_TEST("usage-tpm-vtpm");
>
> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
> --
> 2.43.0
> _______________________________________________
> Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
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 :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
