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

Arun Menon via Devel posted 5 patches 2 months, 2 weeks ago
There is a newer version of this series
[RFC v2 2/5] secret: Set up default encrypted secret key for the virtsecretd service
Posted by Arun Menon via Devel 2 months, 2 weeks 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 months, 2 weeks 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 2 months, 2 weeks 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 :|