From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6C82A1FF13C for ; Thu, 02 Apr 2026 01:27:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 36B6D32D9; Thu, 2 Apr 2026 01:28:14 +0200 (CEST) Message-ID: Date: Thu, 2 Apr 2026 01:27:39 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-backup 04/20] pbs-config: implement encryption key config handling To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260401075521.176354-1-c.ebner@proxmox.com> <20260401075521.176354-5-c.ebner@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20260401075521.176354-5-c.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775086002120 X-SPAM-LEVEL: Spam detection results: 0 AWL -2.499 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_MAILER 2 Automated Mailer Tag Left in Email RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: PEWDGFE7LAUZARPDLSN5VD7XM5EYQAZZ X-Message-ID-Hash: PEWDGFE7LAUZARPDLSN5VD7XM5EYQAZZ X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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"); > + } > +}