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 <[email protected]>
> ---
> 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
>
