On Thu, Nov 13, 2025 at 10:47:52PM +0530, Arun Menon wrote: > Hi Daniel, > > Thanks for the review. > > On Thu, Nov 13, 2025 at 03:01:21PM +0000, Daniel P. Berrangé wrote: > > On Thu, Nov 13, 2025 at 07:02:21PM +0530, Arun Menon wrote: > > > This commit sets the foundation for encrypting the libvirt secrets by > > > providing a > > > secure way to pass a master encryption key to the virtsecretd service. > > > > > > Add a default, pre-generated, master encryption key to the credentials, > > > that > > > can be consumed by the virtsecretd service. > > > By using the "SetCredentialEncrypted=" directive, we make sure that > > > passing > > > data to the service is secure. > > > 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. > > > Users can customize this setting, by replacing the default encrypted > > > string > > > with their own. A subsequent commit will introduce the logic for > > > virtsecretd > > > to access and use this key via the $CREDENTIALS_DIRECTORY environment > > > variable. [2] > > > > > > In order to add the default encryption key, a random 32 byte key was > > > generated > > > and encrypted: > > > dd if=/dev/urandom of=/tmp/master.key bs=1 count=32 > > > systemd-creds encrypt --name=master-encryption-key -p /tmp/master.key - > > > > > > This generates a SetCredentialEncrypted= line suitable for inclusion in > > > the unit > > > file. > > > > > > [1] > > > https://www.freedesktop.org/software/systemd/man/latest/systemd-creds.html > > > [2] https://systemd.io/CREDENTIALS/ > > > > > > Signed-off-by: Arun Menon <[email protected]> > > > --- > > > src/secret/virtsecretd.service.extra.in | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/src/secret/virtsecretd.service.extra.in > > > b/src/secret/virtsecretd.service.extra.in > > > index 1fc8c672f7..0f65bc3bb1 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. > > > +# > > > +[Service] > > > +Environment=MASTER_ENCRYPTION_KEY=%d/master-encryption-key > > > +SetCredentialEncrypted=master-encryption-key: \ > > > + > > > Whxqht+dQJax1aZeCGLxmiAAAAABAAAADAAAABAAAAD9m5CsEfoZf8Lj/dQAAAAAFSvJ7 \ > > > + > > > eSEmqQthu+A4Eqn4vEKp6jx7ScbcM98bcW5Do0K9V0eTPWD+eNJJrB+xS/MAklo3rkf0S \ > > > + 7n7rXk8SQZ0FQ5Uv8ZoOuidWPHHiLZGS9bxAJwTZvN/VX+pe+biC16 > > > +LoadCredentialEncrypted=master-encryption-key > > > > We cannot ship with a hardcoded default secret - that is an instant CVE. > > > > We need to have an 'ExecStartPre=' directive that auto-generates a secret > > on first use. > > > > ExecStartPre=test -f /var/lib/libvirt/secrets/encryption.key || (dd > > if=/dev/urandom status=none bs=1 count=32 | systemd-creds encrypt > > --name=master-encryption-key - /var/lib/libvirt/secrets/encryption.key) > > > > Also, bs=32 count=1 is preferrable to bs=1 count=32 as we don't > > want to be doing single byte reads & writes. > > > > and then use > > > > > > LoadCredentialEncrypted=master-encryption-key:/var/lib/libvirt/secrets/encryption.key > > > > > > Also, lets call this 'secrets-encryption-key', and likewise > > SECRETS_ENCRYPTION_KEY as the env variable. > > Yes, will do. > > I wanted to do exactly this, but it turns out that there is a precedence in > execution. > Meaning "ExecStartPre" is executed at a later stage than > "LoadCredentialEncrypted". > As a result, LoadCredentialEncrypted does not get the value that was set in > ExecStartPre. I did not know about this,
Urgh, that's annoying. > We can remove the line and add it in the readme docs so that the reader knows > how to > set up encryption using the service file. The user can create a secret key > however > they want, and that way it will not be tied to the service unit file. No, better if we do it with a separate unit file Create a virtsecretd-init-encryption.service, and have that listed as a dependency of virtsecretd.service. In that we can use ConditionPathExists=!/var/run/libvirt/secrets/encryption.cred soo that it is only launched if the encrypted credential has not yet been created. That'll effectively make the new service a one-shot thing. 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 :|
