Re: [PATCH proxmox-backup 04/20] pbs-config: implement encryption key config handling

Thomas Lamprecht
2026-04-01T23:27:45Z

Am 01.04.26 um 09:55 schrieb Christian Ebner:
> Implements the handling for encryption key configuration and files.
>
> Individual encryption keys with the secret key material are stored in
> individual files, while the config stores duplicate key info, so the
> actual key only needs to be loaded when accessed, not for listing.
>
> Key fingerprint is compared when loading the key in order to detect
> possible mismatches.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> pbs-config/Cargo.toml | 1 +
> pbs-config/src/encryption_keys.rs | 159 ++++++++++++++++++++++++++++++
> pbs-config/src/lib.rs | 1 +
> 3 files changed, 161 insertions(+)
> create mode 100644 pbs-config/src/encryption_keys.rs
>
> diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml
> index eb81ce004..a27964cfd 100644
> --- a/pbs-config/Cargo.toml
> +++ b/pbs-config/Cargo.toml
> @@ -30,3 +30,4 @@ proxmox-uuid.workspace = true
>
> pbs-api-types.workspace = true
> pbs-buildcfg.workspace = true
> +pbs-key-config.workspace = true
> diff --git a/pbs-config/src/encryption_keys.rs b/pbs-config/src/encryption_keys.rs

> +/// Delete the encryption key from config.
> +///
> +/// Deletes the key from the config but keeps a backup of the key file.

The implementation just calls `std::fs::remove_file`, so is there really a
backup of the key kept?

> +pub fn delete_key(id: &str) -> Result<(), Error> {
> + let _lock = lock_config()?;
> + let (mut config, _digest) = config()?;
> +
> + // if the key with given id exists in config, try to remove also file on path.
> + if let Some((section_type, key)) = config.sections.get(id) {
> + if section_type == ENCRYPTION_KEYS_CFG_TYPE_ID {
> + let key = EncryptionKey::deserialize(key)
> + .map_err(|_err| format_err!("failed to parse pre-existing key"))?;
> +
> + if let Some(path) = &key.info.path {
> + std::fs::remove_file(path)?;
> + }

This ordering seems slightly risky: the key file is deleted *before* the
config is rewritten. If `replace_backup_config` fails after `remove_file`
succeeds, the key material is gone but the config still references it.

Would be probably safer to write the config first (removing the section),
then delete the file. Or was the idea here to avoid leaving a stale key
file on disk?

> + }
> +
> + config.sections.remove(id);
> +
> + let raw = CONFIG.write(ENCRYPTION_KEYS_CFG_FILENAME, &config)?;
> + replace_backup_config(ENCRYPTION_KEYS_CFG_FILENAME, raw.as_bytes())
> + } else {
> + bail!("key {id} not found in config");
> + }
> +}