[RFC v2 2/5] secret: Set up default encrypted secret key for the virtsecretd service

Arun Menon via Devel posted 5 patches 17 hours ago
[RFC v2 2/5] secret: Set up default encrypted secret key for the virtsecretd service
Posted by Arun Menon via Devel 17 hours ago
This commit sets the foundation for encrypting the libvirt secrets by providing a
secure way to pass a secret encryption key to the virtsecretd service.

A random secret key is generated using the new virt-secret-init-encryption
service. This key can be consumed by the virtsecretd service.

By using the "Before=" directive in the new secret-init-encryption
service and using "Requires=" directive in the virtsecretd service,
we make sure that the daemon is run only after we have an encrypted
secret key file generated and placed in /var/lib/libvirt/secrets.
The virtsecretd service can then read the key from CREDENTIALS_DIRECTORY. [1]

This setup therefore provides a default key out-of-the-box for initial use.
A subsequent commit will introduce the logic for virtsecretd
to access and use this key via the $CREDENTIALS_DIRECTORY environment variable. [2]

[1] https://www.freedesktop.org/software/systemd/man/latest/systemd-creds.html
[2] https://systemd.io/CREDENTIALS/

Signed-off-by: Arun Menon <armenon@redhat.com>
---
 libvirt.spec.in                         |  7 +++++++
 src/secret/meson.build                  |  8 ++++++++
 src/secret/secret-init-encryption.in    | 11 +++++++++++
 src/secret/virtsecretd.service.extra.in |  8 ++++++++
 4 files changed, 34 insertions(+)
 create mode 100644 src/secret/secret-init-encryption.in

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 79738bd7bb..fa477db031 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1889,13 +1889,16 @@ exit 0
 %pre daemon-driver-secret
 %libvirt_sysconfig_pre virtsecretd
 %libvirt_systemd_unix_pre virtsecretd
+%libvirt_systemd_oneshot_pre virt-secret-init-encryption
 
 %posttrans daemon-driver-secret
 %libvirt_sysconfig_posttrans virtsecretd
 %libvirt_systemd_unix_posttrans virtsecretd
+%libvirt_systemd_unix_posttrans virt-secret-init-encryption
 
 %preun daemon-driver-secret
 %libvirt_systemd_unix_preun virtsecretd
+%libvirt_systemd_unix_preun virt-secret-init-encryption
 
 %pre daemon-driver-storage-core
 %libvirt_sysconfig_pre virtstoraged
@@ -2247,9 +2250,13 @@ exit 0
 %{_datadir}/augeas/lenses/virtsecretd.aug
 %{_datadir}/augeas/lenses/tests/test_virtsecretd.aug
 %{_unitdir}/virtsecretd.service
+%{_unitdir}/virt-secret-init-encryption.service
 %{_unitdir}/virtsecretd.socket
 %{_unitdir}/virtsecretd-ro.socket
 %{_unitdir}/virtsecretd-admin.socket
+%{_unitdir}/virt-secret-init-encryption.socket
+%{_unitdir}/virt-secret-init-encryption-ro.socket
+%{_unitdir}/virt-secret-init-encryption-admin.socket
 %attr(0755, root, root) %{_sbindir}/virtsecretd
 %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/secrets/
 %ghost %dir %attr(0700, root, root) %{_rundir}/libvirt/secrets/
diff --git a/src/secret/meson.build b/src/secret/meson.build
index 3b859ea7b4..d8861fcbcd 100644
--- a/src/secret/meson.build
+++ b/src/secret/meson.build
@@ -42,6 +42,14 @@ if conf.has('WITH_SECRETS')
     ],
   }
 
+  virt_daemon_units += {
+    'service': 'virt-secret-init-encryption',
+    'name': 'secret-init-encryption',
+    'service_in': files('secret-init-encryption.in'),
+    'service_extra_in': [],
+    'socket_extra_in': [],
+  }
+
   openrc_init_files += {
     'name': 'virtsecretd',
     'in_file': files('virtsecretd.init.in'),
diff --git a/src/secret/secret-init-encryption.in b/src/secret/secret-init-encryption.in
new file mode 100644
index 0000000000..4dc00e5bbb
--- /dev/null
+++ b/src/secret/secret-init-encryption.in
@@ -0,0 +1,11 @@
+[Unit]
+Before=virtsecretd.service
+ConditionPathExists=!/var/lib/libvirt/secrets/secrets-encryption-key
+
+[Service]
+Type=oneshot
+ExecStart=/usr/bin/sh -c 'test -f /var/lib/libvirt/secrets/secrets-encryption-key || (dd if=/dev/urandom status=none bs=32 count=1 | systemd-creds encrypt --name=secrets-encryption-key - /var/lib/libvirt/secrets/secrets-encryption-key)'
+ExecStart=-/usr/bin/chmod 0400 /var/lib/libvirt/secrets/secrets-encryption-key
+
+[Install]
+WantedBy=multi-user.target
diff --git a/src/secret/virtsecretd.service.extra.in b/src/secret/virtsecretd.service.extra.in
index 1fc8c672f7..1df9163814 100644
--- a/src/secret/virtsecretd.service.extra.in
+++ b/src/secret/virtsecretd.service.extra.in
@@ -1,2 +1,10 @@
 # The contents of this unit will be merged into a base template.
 # Additional units might be merged as well. See meson.build for details.
+#
+[Unit]
+Requires=virt-secret-init-encryption.service
+After=virt-secret-init-encryption.service
+
+[Service]
+LoadCredentialEncrypted=secrets-encryption-key:/var/lib/libvirt/secrets/secrets-encryption-key
+Environment=SECRETS_ENCRYPTION_KEY=%d/secrets-encryption-key
-- 
2.51.1
Re: [RFC v2 2/5] secret: Set up default encrypted secret key for the virtsecretd service
Posted by Peter Krempa via Devel 2 hours ago
On Thu, Nov 20, 2025 at 22:23:43 +0530, Arun Menon via Devel wrote:
> This commit sets the foundation for encrypting the libvirt secrets by providing a
> secure way to pass a secret encryption key to the virtsecretd service.
> 
> A random secret key is generated using the new virt-secret-init-encryption
> service. This key can be consumed by the virtsecretd service.
> 
> By using the "Before=" directive in the new secret-init-encryption
> service and using "Requires=" directive in the virtsecretd service,
> we make sure that the daemon is run only after we have an encrypted
> secret key file generated and placed in /var/lib/libvirt/secrets.
> The virtsecretd service can then read the key from CREDENTIALS_DIRECTORY. [1]
> 
> This setup therefore provides a default key out-of-the-box for initial use.
> A subsequent commit will introduce the logic for virtsecretd
> to access and use this key via the $CREDENTIALS_DIRECTORY environment variable. [2]
> 
> [1] https://www.freedesktop.org/software/systemd/man/latest/systemd-creds.html
> [2] https://systemd.io/CREDENTIALS/
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  libvirt.spec.in                         |  7 +++++++
>  src/secret/meson.build                  |  8 ++++++++
>  src/secret/secret-init-encryption.in    | 11 +++++++++++
>  src/secret/virtsecretd.service.extra.in |  8 ++++++++
>  4 files changed, 34 insertions(+)
>  create mode 100644 src/secret/secret-init-encryption.in
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 79738bd7bb..fa477db031 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1889,13 +1889,16 @@ exit 0
>  %pre daemon-driver-secret
>  %libvirt_sysconfig_pre virtsecretd
>  %libvirt_systemd_unix_pre virtsecretd
> +%libvirt_systemd_oneshot_pre virt-secret-init-encryption
>  
>  %posttrans daemon-driver-secret
>  %libvirt_sysconfig_posttrans virtsecretd
>  %libvirt_systemd_unix_posttrans virtsecretd
> +%libvirt_systemd_unix_posttrans virt-secret-init-encryption
>  
>  %preun daemon-driver-secret
>  %libvirt_systemd_unix_preun virtsecretd
> +%libvirt_systemd_unix_preun virt-secret-init-encryption
>  
>  %pre daemon-driver-storage-core
>  %libvirt_sysconfig_pre virtstoraged
> @@ -2247,9 +2250,13 @@ exit 0
>  %{_datadir}/augeas/lenses/virtsecretd.aug
>  %{_datadir}/augeas/lenses/tests/test_virtsecretd.aug
>  %{_unitdir}/virtsecretd.service
> +%{_unitdir}/virt-secret-init-encryption.service
>  %{_unitdir}/virtsecretd.socket
>  %{_unitdir}/virtsecretd-ro.socket
>  %{_unitdir}/virtsecretd-admin.socket
> +%{_unitdir}/virt-secret-init-encryption.socket
> +%{_unitdir}/virt-secret-init-encryption-ro.socket
> +%{_unitdir}/virt-secret-init-encryption-admin.socket

Looking at the change to src/secret/virtsecretd.service.extra.in 
these *.socket files are not actually referenced as a dependancy. Why
are you creating those?

Are they created by our build system and this is just a hack to make
RPM builds work?


>  %attr(0755, root, root) %{_sbindir}/virtsecretd
>  %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/secrets/
>  %ghost %dir %attr(0700, root, root) %{_rundir}/libvirt/secrets/
> diff --git a/src/secret/meson.build b/src/secret/meson.build
> index 3b859ea7b4..d8861fcbcd 100644
> --- a/src/secret/meson.build
> +++ b/src/secret/meson.build
> @@ -42,6 +42,14 @@ if conf.has('WITH_SECRETS')
>      ],
>    }
>  
> +  virt_daemon_units += {
> +    'service': 'virt-secret-init-encryption',
> +    'name': 'secret-init-encryption',
> +    'service_in': files('secret-init-encryption.in'),
> +    'service_extra_in': [],
> +    'socket_extra_in': [],
> +  }

So this is what is creating those .socket files. If you don't want them
and I don't see why they should exist you'll need to build this in a
different way.




> +
>    openrc_init_files += {
>      'name': 'virtsecretd',
>      'in_file': files('virtsecretd.init.in'),
> diff --git a/src/secret/secret-init-encryption.in b/src/secret/secret-init-encryption.in
> new file mode 100644
> index 0000000000..4dc00e5bbb
> --- /dev/null
> +++ b/src/secret/secret-init-encryption.in
> @@ -0,0 +1,11 @@
> +[Unit]
> +Before=virtsecretd.service
> +ConditionPathExists=!/var/lib/libvirt/secrets/secrets-encryption-key

So this shouldn't start if the file exists.

You also use full paths here, which don't respect configure options such
as 'sysconfdir' etc.

> +
> +[Service]
> +Type=oneshot
> +ExecStart=/usr/bin/sh -c 'test -f /var/lib/libvirt/secrets/secrets-encryption-key || (dd if=/dev/urandom status=none bs=32 count=1 | systemd-creds encrypt --name=secrets-encryption-key - /var/lib/libvirt/secrets/secrets-encryption-key)'

So why is this checking the existance again?

> +ExecStart=-/usr/bin/chmod 0400 /var/lib/libvirt/secrets/secrets-encryption-key

'chmod'-ing after the file is created leaves a theoretical chance to
leak the secret. I think you'll need to pre-create an empty file, chmod
it and jus then fill it with the secret.

> +
> +[Install]
> +WantedBy=multi-user.target
> diff --git a/src/secret/virtsecretd.service.extra.in b/src/secret/virtsecretd.service.extra.in
> index 1fc8c672f7..1df9163814 100644
> --- a/src/secret/virtsecretd.service.extra.in
> +++ b/src/secret/virtsecretd.service.extra.in
> @@ -1,2 +1,10 @@
>  # The contents of this unit will be merged into a base template.
>  # Additional units might be merged as well. See meson.build for details.
> +#
> +[Unit]
> +Requires=virt-secret-init-encryption.service
> +After=virt-secret-init-encryption.service
> +
> +[Service]
> +LoadCredentialEncrypted=secrets-encryption-key:/var/lib/libvirt/secrets/secrets-encryption-key
> +Environment=SECRETS_ENCRYPTION_KEY=%d/secrets-encryption-key
> -- 
> 2.51.1
>
Re: [RFC v2 2/5] secret: Set up default encrypted secret key for the virtsecretd service
Posted by Daniel P. Berrangé via Devel 11 minutes ago
On Fri, Nov 21, 2025 at 08:41:53AM +0100, Peter Krempa via Devel wrote:
> On Thu, Nov 20, 2025 at 22:23:43 +0530, Arun Menon via Devel wrote:
> > This commit sets the foundation for encrypting the libvirt secrets by providing a
> > secure way to pass a secret encryption key to the virtsecretd service.
> > 
> > A random secret key is generated using the new virt-secret-init-encryption
> > service. This key can be consumed by the virtsecretd service.
> > 
> > By using the "Before=" directive in the new secret-init-encryption
> > service and using "Requires=" directive in the virtsecretd service,
> > we make sure that the daemon is run only after we have an encrypted
> > secret key file generated and placed in /var/lib/libvirt/secrets.
> > The virtsecretd service can then read the key from CREDENTIALS_DIRECTORY. [1]
> > 
> > This setup therefore provides a default key out-of-the-box for initial use.
> > A subsequent commit will introduce the logic for virtsecretd
> > to access and use this key via the $CREDENTIALS_DIRECTORY environment variable. [2]
> > 
> > [1] https://www.freedesktop.org/software/systemd/man/latest/systemd-creds.html
> > [2] https://systemd.io/CREDENTIALS/
> > 
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > ---
> >  libvirt.spec.in                         |  7 +++++++
> >  src/secret/meson.build                  |  8 ++++++++
> >  src/secret/secret-init-encryption.in    | 11 +++++++++++
> >  src/secret/virtsecretd.service.extra.in |  8 ++++++++
> >  4 files changed, 34 insertions(+)
> >  create mode 100644 src/secret/secret-init-encryption.in
> > 
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index 79738bd7bb..fa477db031 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -1889,13 +1889,16 @@ exit 0
> >  %pre daemon-driver-secret
> >  %libvirt_sysconfig_pre virtsecretd
> >  %libvirt_systemd_unix_pre virtsecretd
> > +%libvirt_systemd_oneshot_pre virt-secret-init-encryption
> >  
> >  %posttrans daemon-driver-secret
> >  %libvirt_sysconfig_posttrans virtsecretd
> >  %libvirt_systemd_unix_posttrans virtsecretd
> > +%libvirt_systemd_unix_posttrans virt-secret-init-encryption
> >  
> >  %preun daemon-driver-secret
> >  %libvirt_systemd_unix_preun virtsecretd
> > +%libvirt_systemd_unix_preun virt-secret-init-encryption
> >  
> >  %pre daemon-driver-storage-core
> >  %libvirt_sysconfig_pre virtstoraged
> > @@ -2247,9 +2250,13 @@ exit 0
> >  %{_datadir}/augeas/lenses/virtsecretd.aug
> >  %{_datadir}/augeas/lenses/tests/test_virtsecretd.aug
> >  %{_unitdir}/virtsecretd.service
> > +%{_unitdir}/virt-secret-init-encryption.service
> >  %{_unitdir}/virtsecretd.socket
> >  %{_unitdir}/virtsecretd-ro.socket
> >  %{_unitdir}/virtsecretd-admin.socket
> > +%{_unitdir}/virt-secret-init-encryption.socket
> > +%{_unitdir}/virt-secret-init-encryption-ro.socket
> > +%{_unitdir}/virt-secret-init-encryption-admin.socket
> 
> Looking at the change to src/secret/virtsecretd.service.extra.in 
> these *.socket files are not actually referenced as a dependancy. Why
> are you creating those?
> 
> Are they created by our build system and this is just a hack to make
> RPM builds work?
> 
> 
> >  %attr(0755, root, root) %{_sbindir}/virtsecretd
> >  %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/secrets/
> >  %ghost %dir %attr(0700, root, root) %{_rundir}/libvirt/secrets/
> > diff --git a/src/secret/meson.build b/src/secret/meson.build
> > index 3b859ea7b4..d8861fcbcd 100644
> > --- a/src/secret/meson.build
> > +++ b/src/secret/meson.build
> > @@ -42,6 +42,14 @@ if conf.has('WITH_SECRETS')
> >      ],
> >    }
> >  
> > +  virt_daemon_units += {
> > +    'service': 'virt-secret-init-encryption',
> > +    'name': 'secret-init-encryption',
> > +    'service_in': files('secret-init-encryption.in'),
> > +    'service_extra_in': [],
> > +    'socket_extra_in': [],
> > +  }
> 
> So this is what is creating those .socket files. If you don't want them
> and I don't see why they should exist you'll need to build this in a
> different way.

Yep, this virt_daemon_units was only designed for the main daemon
unit file processing.

In this case, we should just call the meson 'configure_file'
function directly to generate secret-init-encryption.service

> > +
> >    openrc_init_files += {
> >      'name': 'virtsecretd',
> >      'in_file': files('virtsecretd.init.in'),
> > diff --git a/src/secret/secret-init-encryption.in b/src/secret/secret-init-encryption.in
> > new file mode 100644
> > index 0000000000..4dc00e5bbb
> > --- /dev/null
> > +++ b/src/secret/secret-init-encryption.in
> > @@ -0,0 +1,11 @@
> > +[Unit]
> > +Before=virtsecretd.service
> > +ConditionPathExists=!/var/lib/libvirt/secrets/secrets-encryption-key
> 
> So this shouldn't start if the file exists.
> 
> You also use full paths here, which don't respect configure options such
> as 'sysconfdir' etc.
> 
> > +
> > +[Service]
> > +Type=oneshot
> > +ExecStart=/usr/bin/sh -c 'test -f /var/lib/libvirt/secrets/secrets-encryption-key || (dd if=/dev/urandom status=none bs=32 count=1 | systemd-creds encrypt --name=secrets-encryption-key - /var/lib/libvirt/secrets/secrets-encryption-key)'
> 
> So why is this checking the existance again?
> 
> > +ExecStart=-/usr/bin/chmod 0400 /var/lib/libvirt/secrets/secrets-encryption-key
> 
> 'chmod'-ing after the file is created leaves a theoretical chance to
> leak the secret. I think you'll need to pre-create an empty file, chmod
> it and jus then fill it with the secret.

Easier to use the 'umask' command first

 "umask 0066 && (dd if=... | systemd-creds .....)"

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 :|