2:27 a.m.
I'm kind of convinced that we want to do this, but also it's a
significant change in the behaviour of the daemon, hence RFC prefix.
This stemmed from a discussion with a user who wants us to use something
more secure than base64 encoded secret values stored on a disk. They
suggested storing the values in TPM and while that might sound like a
good idea, I suggested using ephemeral secrets for the time being. Well,
because of '--timeout 120', ephemeral secrets are short lived, indeed.
Meanwhile, let me see if there's a library we could use to talk to TPM.
Michal Prívozník (1):
secret: Inhibit shutdown for ephemeral secrets
src/secret/secret_driver.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
--
2.38.2
2:27 a.m.
New subject: [PATCH 1/1] secret: Inhibit shutdown for ephemeral secrets
Our secret driver divides secrets into two groups: ephemeral
(stored only in memory) and persistent (stored on disk). Now, the
aim of ephemeral secrets is to define them shortly before being
used and then undefine them. But 'shortly before being used' is a
very vague time frame. And since we default to socket activation
and thus pass '--timeout 120' to every daemon it may happen that
just defined ephemeral secret is gone among with the virtsecretd.
This is no problem for persistent secrets as their definition
(and value) is restored when the virtsecretd starts again, but
ephemeral secrets can't be restored.
Therefore, we could view ephemeral secrets as active objects that
the daemon manages and thus inhibit automatic shutdown (just like
hypervisor daemons do when a guest is running).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/secret/secret_driver.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index bd981a8ace..c38cd6f651 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -66,6 +66,10 @@ struct _virSecretDriverState {
/* Immutable pointer, self-locking APIs */
virObjectEventState *secretEventState;
+
+ /* Immutable pointers. Caller must provide locking */
+ virStateInhibitCallback inhibitCallback;
+ void *inhibitOpaque;
};
static virSecretDriverState *driver;
@@ -86,6 +90,23 @@ secretObjFromSecret(virSecretPtr secret)
}
+static bool
+secretNumOfEphemeralSecretsHelper(virConnectPtr conn G_GNUC_UNUSED,
+ virSecretDef *def)
+{
+ return def->isephemeral;
+}
+
+
+static int
+secretNumOfEphemeralSecrets(void)
+{
+ return virSecretObjListNumOfSecrets(driver->secrets,
+ secretNumOfEphemeralSecretsHelper,
+ NULL);
+}
+
+
/* Driver functions */
static int
@@ -266,6 +287,10 @@ secretDefineXML(virConnectPtr conn,
cleanup:
virSecretDefFree(def);
virSecretObjEndAPI(&obj);
+
+ if (secretNumOfEphemeralSecrets() > 0)
+ driver->inhibitCallback(true, driver->inhibitOpaque);
+
virObjectEventStateQueue(driver->secretEventState, event);
return ret;
@@ -424,6 +449,10 @@ secretUndefine(virSecretPtr secret)
cleanup:
virSecretObjEndAPI(&obj);
+
+ if (secretNumOfEphemeralSecrets() == 0)
+ driver->inhibitCallback(false, driver->inhibitOpaque);
+
virObjectEventStateQueue(driver->secretEventState, event);
return ret;
@@ -463,8 +492,8 @@ static int
secretStateInitialize(bool privileged,
const char *root,
bool monolithic G_GNUC_UNUSED,
- virStateInhibitCallback callback G_GNUC_UNUSED,
- void *opaque G_GNUC_UNUSED)
+ virStateInhibitCallback callback,
+ void *opaque)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&mutex);
@@ -473,6 +502,8 @@ secretStateInitialize(bool privileged,
driver->lockFD = -1;
driver->secretEventState = virObjectEventStateNew();
driver->privileged = privileged;
+ driver->inhibitCallback = callback;
+ driver->inhibitOpaque = opaque;
if (root) {
driver->embeddedRoot = g_strdup(root);
--
2.38.2
2:31 a.m.
New subject: [PATCH 1/1] secret: Inhibit shutdown for ephemeral secrets
On Tue, Dec 20, 2022 at 09:27:11AM +0100, Michal Privoznik wrote:
Our secret driver divides secrets into two groups: ephemeral
(stored only in memory) and persistent (stored on disk). Now, the
aim of ephemeral secrets is to define them shortly before being
used and then undefine them. But 'shortly before being used' is a
very vague time frame. And since we default to socket activation
and thus pass '--timeout 120' to every daemon it may happen that
just defined ephemeral secret is gone among with the virtsecretd.
This is no problem for persistent secrets as their definition
(and value) is restored when the virtsecretd starts again, but
ephemeral secrets can't be restored.
Therefore, we could view ephemeral secrets as active objects that
the daemon manages and thus inhibit automatic shutdown (just like
hypervisor daemons do when a guest is running).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/secret/secret_driver.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
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 :|
3:01 a.m.
On Tue, Dec 20, 2022 at 09:27:10AM +0100, Michal Privoznik wrote:
I'm kind of convinced that we want to do this, but also it's
a
significant change in the behaviour of the daemon, hence RFC prefix.
This stemmed from a discussion with a user who wants us to use something
more secure than base64 encoded secret values stored on a disk. They
suggested storing the values in TPM and while that might sound like a
good idea, I suggested using ephemeral secrets for the time being. Well,
because of '--timeout 120', ephemeral secrets are short lived, indeed.
Meanwhile, let me see if there's a library we could use to talk to TPM.
Storing secrets in the TPM isn't viable, as it has insufficient
NVRAM for our needs. What we need todo is encrypt the secrets,
with a primary key that is in turn sealed against the TPM. This
sounds difficult, but its actually fairly trivial as we can receive
such a primary key from systemd, using its credentials mechanism.
If we assume a (encrypted) primary key in /var/lib/libvirt, then
we can put a line in virtsecretd.service:
LoadCredential=primary:/var/lib/libvirt/secret/primary.creds
When virtsecretd runs, this will result in the plain text primary
key being made available in a file under $CREDENTIALS_DIRECTORY.
We can reference this directory using %d, so for example we
change virtsecretd.service to use:
ExecStart=/usr/sbin/virtsecretd --primary-key %d/primary $VIRTSECRETD_ARGS
The problem is how do we create the original encrypted primary
key. The best trick I've come up with is to use a ExecStartPre
script:
ExecStartPre=/usr/sbin/virtsecretd-mkcreds /var/lib/libvirt/secret/primary.creds
Where virtsecretd-mkcreds contains:
#!/bin/sh
CREDS=$1
if test -f $CREDS
then
exit 0
fi
dd if=/dev/urandom bs=256 count=1 status=none | systemd-creds encrypt - $CREDS
exit 0
This creates a random key encrypting it, preferentially with the TPM2
if one is available.
It is kinda irritating that systemd can't auto-create creds itself when
a service is started, as this mkcreds script would be the same for
everyone who wants this kind of functionality.
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 :|
703
days inactive
703
days old
3 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik