On Thu, Nov 13, 2025 at 07:02:22PM +0530, Arun Menon wrote:
> A new configuration file called secrets.conf is introduced to
> let the user configure the path to the master encryption key.
> This key will be used to encrypt/decrypt the secrets in libvirt.
>
> By default the path is set to the runtime directory
> /run/libvirt/secrets, and it is commented in the config file.
> The virtsecretd driver checks if the credentials are available
> in the CREDENTIALS_DIRECTORY. In case it is not present, then the
> user is expected to provide the encryption key path in secrets.conf
>
> Add logic to parse the encryption key file and store the key.
> When systemd will start the secrets driver, it will read the secret.conf
> file and check if encrypt_data flag is set to 1. In that case, the secrets
> will be stored in encrypted format on the disk. The encryption and decryption
> logic will be added in the subsequent patches.
>
> Signed-off-by: Arun Menon <[email protected]>
> ---
> libvirt.spec.in | 1 +
> src/secret/meson.build | 7 +++
> src/secret/secret_driver.c | 96 ++++++++++++++++++++++++++++++++++++++
> src/secret/secrets.conf.in | 14 ++++++
New config files need to be accompanied with augeas files for
processing them. See the simple examples in
src/network/libvirtd_network.aug
src/network/test_libvirtd_network.aug.in
and the src/network/meson.build file for how to wire them up.
They also need to be in the RPM spec.
> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
> index 04c3ca49f1..0b415e5ef3 100644
> --- a/src/secret/secret_driver.c
> +++ b/src/secret/secret_driver.c
> @@ -42,6 +42,7 @@
> #include "secret_event.h"
> #include "virutil.h"
> #include "virinhibitor.h"
> +#include "virfile.h"
>
> #define VIR_FROM_THIS VIR_FROM_SECRET
>
> @@ -70,6 +71,17 @@ struct _virSecretDriverState {
>
> /* Immutable pointer, self-locking APIs */
> virInhibitor *inhibitor;
> +
> + /* master encryption key value from secret.conf file */
> + char *masterKeyPath;
Per the previous patch, lets call this secretsEncryptionKeyPath
> +
> + /* Indicates if the secrets are encrypted or not. 0 if not encrypted
> + * and 1 if encrypted.
> + */
Lets specify "newly written secrets" to clarify that setting this
to '1' wouldn't make it encrypt any pre-existing plain text secrets
> + int encrypt_data;
> +
> + unsigned char* masterKey;
> + size_t masterKeyLen;
> };
None of these should be in this struct. Our normal practice
is to define a virSecretDriverConfig struct to hold all
config parameters, and an pointer to that config in the
state struct. See virNetworkDriverState/Config for a
simple example.
>
> static virSecretDriverState *driver;
> @@ -307,6 +319,44 @@ secretGetXMLDesc(virSecretPtr secret,
> return ret;
> }
>
> +static bool secretGetMasterKey(uint8_t **masterKey, size_t *masterKeyLen)
> +{
> + int fd = -1;
> + struct stat st;
> +
> + if ((fd = open(driver->masterKeyPath, O_RDONLY)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot open master key
> file '%1$s'"),
> + driver->masterKeyPath);
> + return false;
> + }
> + if (fstat(fd, &st) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot stat master key
> file '%1$s'"),
> + driver->masterKeyPath);
> + VIR_FORCE_CLOSE(fd);
> + return false;
> + }
> + *masterKeyLen = st.st_size;
> + if (*masterKeyLen == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("Master encryption key file
> %1$s is empty"),
> + driver->masterKeyPath);
> + VIR_FORCE_CLOSE(fd);
> + return false;
> + }
> + *masterKey = g_new0(uint8_t, *masterKeyLen);
> + if (saferead(fd, &masterKey, *masterKeyLen) != *masterKeyLen) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot read master key
> file '%1$s'"),
> + driver->masterKeyPath);
> + VIR_FORCE_CLOSE(fd);
> + return false;
> + }
> + VIR_FORCE_CLOSE(fd);
> + if (*masterKeyLen < 32) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("Master encryption key file
> %1$s must be atleast 32 bytes"),
> + driver->masterKeyPath);
> + return false;
> + }
> + return true;
> +}
>
> static int
> secretSetValue(virSecretPtr secret,
> @@ -482,6 +532,10 @@ secretStateInitialize(bool privileged,
> void *opaque)
> {
> VIR_LOCK_GUARD lock = virLockGuardLock(&mutex);
> + g_autofree char *secretsconf = NULL;
> + g_autofree char *credentials_directory = NULL;
> + g_autofree char *master_encryption_key_path = NULL;
> + g_autoptr(virConf) conf = NULL;
>
> driver = g_new0(virSecretDriverState, 1);
>
> @@ -537,6 +591,48 @@ secretStateInitialize(bool privileged,
> if (virSecretLoadAllConfigs(driver->secrets, driver->configDir) < 0)
> goto error;
>
> + secretsconf = g_strdup_printf("%s/libvirt/secrets.conf", SYSCONFDIR);
> + credentials_directory = getenv("CREDENTIALS_DIRECTORY");
> +
> + if (credentials_directory) {
> + VIR_DEBUG("Using credentials directory from environment: %s",
> + credentials_directory);
> + master_encryption_key_path =
> g_strdup_printf("%s/master-encryption-key", credentials_directory);
> + if (access(master_encryption_key_path, R_OK) == 0) {
> + driver->masterKeyPath = g_strdup(master_encryption_key_path);
> + }
> + } else if (access(secretsconf, R_OK) == 0) {
> + if (!(conf = virConfReadFile(secretsconf, 0))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to read secrets.conf from %1$s"),
> + secretsconf);
> + goto error;
> + }
> +
> + if (virConfGetValueString(conf, "master_encryption_key",
> &driver->masterKeyPath) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to get master_encryption_key from
> %1$s"),
> + secretsconf);
> + goto error;
> + }
> + } else {
> + VIR_DEBUG("No secrets configuration found %s, skipping",
> driver->configDir);
> + driver->masterKeyPath = NULL;
> + driver->masterKeyLen = 0;
> + }
> + if (driver->masterKeyPath) {
> + if (!secretGetMasterKey(&driver->masterKey, &driver->masterKeyLen)) {
> + goto error;
> + }
> + VIR_DEBUG("Master encryption key loaded from %s",
> driver->masterKeyPath);
> + VIR_DEBUG("Master encryption key length: %zu bytes",
> driver->masterKeyLen);
> + }
> + if (virConfGetValueInt(conf, "encrypt_data", &driver->encrypt_data) < 0)
> {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to get encrypt_data from %1$s"),
> + secretsconf);
> + goto error;
> + }
> return VIR_DRV_STATE_INIT_COMPLETE;
Config file handling code should not be put directly in the state
initialization method. All loading of the config should be isolated
in separate methods, and live in secret_driver_conf.{h,c} files.
Again take a look at bridge_driver_conf.{h,c} for a simple example
of prior art.
In terms of handling the 'encrypt_data" parameter we need this
psuedo-logic:
if encrypt_data is not set
if key path exists
encrypt_data = 1
else
encrypt_data = 1
else if encrypt_data == 1
if key path does not exist
..report error...
>
> error:
> diff --git a/src/secret/secrets.conf.in b/src/secret/secrets.conf.in
> new file mode 100644
> index 0000000000..80bb9654ce
> --- /dev/null
> +++ b/src/secret/secrets.conf.in
> @@ -0,0 +1,14 @@
> +#
> +# Master configuration file for the secrets driver.
> +#
> +
> +# The master encryption key is used to override default master encryption
> +# key path. The user can create an encryption key and set the
> master_encryption_key
> +# to the path on which it resides.
> +# The key must be atleast 32-bytes long.
> +#
> +# master_encryption_key = "/run/libvirt/secrets/master.key"
> +#
> +# The encrypt_data setting is used to indicate if the encryption is on or
> off.
> +# 0 indicates off and 1 indicates on. By default it is set to on.
> +encrypt_data = 1
This setting should be commented out - the default value should be
set by the code, and should be 0 or 1, depending on whether or not
any encryption key exists on disk.
# This setting determines whether newly written secret values
# are encrypted or not. Setting this to 1 will not trigger
# encryption of existing plain text secrets on disk.
#
# Will default to 1 if the master_encryption_key path exists
# on disk, otherwise will default to 0. Uncomment this to
# force use of encryption and report an error if the key path
# is missing.
# encrypt_data = 1
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 :|
