From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 5F5AC1FF0C0 for ; Wed, 21 Oct 2026 18:41:28 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A7F83215F1; Wed, 21 Oct 2026 18:41:25 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup 2/2] fix: pbs-key-config: do not panic on short decrypted keys Date: Wed, 21 Oct 2026 18:41:12 +0200 Message-ID: <20261021164112.464267-3-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261021164112.464267-1-c.ebner@proxmox.com> References: <20261021164112.464267-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792600882217 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.498 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: 7B4OWID5U2NOGD7AUPHET6P5EUCPGHFD X-Message-ID-Hash: 7B4OWID5U2NOGD7AUPHET6P5EUCPGHFD X-MailFrom: c.ebner@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: KeyConfig::decrypt() insufficiently check the key length after decryption or when the key is already in plain text, potentially leading to a panic as copy_from_slice() expects the source and target slice length to match, panic'ing otherwise [0]. While this cannot be triggered for encrypted keys as long as openssl::symm::decrypt_aead() returns the expected data and fails on errors, plain keys can easily trigger the panic by loading a corrupt key file. Check for expected key size when loading a plain or encrypted key, as well as when copying to the fixed size array after decryption or copying of already plain key. By defining the expected length as constant, checks and error reporting are kept in sync. [0] https://doc.rust-lang.org/std/primitive.slice.html#method.copy_from_slice Reported-by: Martin Vlach Signed-off-by: Christian Ebner --- pbs-key-config/src/lib.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/pbs-key-config/src/lib.rs b/pbs-key-config/src/lib.rs index 3147845ca..2dd232601 100644 --- a/pbs-key-config/src/lib.rs +++ b/pbs-key-config/src/lib.rs @@ -14,6 +14,8 @@ use pbs_api_types::{Fingerprint, Kdf, KeyInfo}; use pbs_tools::crypt_config::CryptConfig; +const EXPECTED_KEY_LEN: usize = 32; + /// Key derivation function configuration #[derive(Deserialize, Serialize, Clone, Debug)] pub enum KeyDerivationConfig { @@ -34,7 +36,7 @@ pub enum KeyDerivationConfig { impl KeyDerivationConfig { /// Derive a key from provided passphrase pub fn derive_key(&self, passphrase: &[u8]) -> Result<[u8; 32], Error> { - let mut key = [0u8; 32]; + let mut key = [0u8; EXPECTED_KEY_LEN]; match self { KeyDerivationConfig::Scrypt { n, r, p, salt } => { @@ -100,7 +102,7 @@ impl From<&KeyConfig> for KeyInfo { impl KeyConfig { /// Creates a new key using random data, protected by passphrase. pub fn new(passphrase: &[u8], kdf: Kdf) -> Result<([u8; 32], Self), Error> { - let mut key = [0u8; 32]; + let mut key = [0u8; EXPECTED_KEY_LEN]; proxmox_sys::linux::fill_with_random_data(&mut key)?; let key_config = Self::with_key(&key, passphrase, kdf)?; Ok((key, key_config)) @@ -175,6 +177,18 @@ impl KeyConfig { pub fn load>(path: P) -> Result { let keydata = file_get_contents(path)?; let key_config: KeyConfig = serde_json::from_reader(&keydata[..])?; + if key_config.kdf.is_none() && key_config.data.len() != EXPECTED_KEY_LEN { + bail!( + "unexpected key length ({} != {EXPECTED_KEY_LEN})", + key_config.data.len() + ) + } else if key_config.kdf.is_some() && key_config.data.len() < 32 { + bail!( + "unexpected short data for key ({} < 32)", + key_config.data.len() + ); + } + Ok(key_config) } @@ -215,8 +229,12 @@ impl KeyConfig { raw_data.clone() }; - let mut result = [0u8; 32]; - result.copy_from_slice(&key); + let result: [u8; EXPECTED_KEY_LEN] = key.as_slice().try_into().map_err(|_err| { + format_err!( + "unexpected key length {} (expected {EXPECTED_KEY_LEN})", + key.len(), + ) + })?; let crypt_config = CryptConfig::new(result)?; let fingerprint = Fingerprint::new(crypt_config.fingerprint()); @@ -378,6 +396,19 @@ fn encrypt_decrypt_test() -> Result<(), Error> { Ok(()) } +#[test] +fn plain_key_length_check() -> Result<(), Error> { + let key: [u8; 32] = [0x11; 32]; + let empty_passphrase = || -> Result, Error> { Ok(Vec::new()) }; + + let mut key_config = KeyConfig::without_password(key).unwrap(); + assert!(key_config.decrypt(&empty_passphrase).is_ok()); + key_config.data = vec![0x11; 31]; // short raw key data + assert!(key_config.decrypt(&empty_passphrase).is_err()); + + Ok(()) +} + #[test] fn fingerprint_checks() -> Result<(), Error> { let key = KeyConfig { -- 2.47.3