applied: [PATCH] pbs-key-config: check the exact length of password protected key data

applied: [PATCH] pbs-key-config: check the exact length of password protected key data

[Thomas Lamprecht]

The length checks of password protected keys compared against the
plain key length, which only works because the IV and tag that precede
the encrypted key happen to add up to the same 32 bytes. The encrypted
key itself always has the plain key's length with AES-GCM, so the data
has a fixed total length that can be checked exactly, which rejects
truncated or over-long data with a clear error before the tag check
fails with a hint at a wrong password.

Name the IV and tag lengths and use them for the slicing as well.

Christian raised the mix-up while reviewing commit 17675dc28
("pbs-key-config: ensure password-protected key is long enough") and
Fabian agreed to send this as a v2 of that patch, which was already
applied in the meantime by my as I overlooked that reply.

Suggested-by: Christian Ebner <c.ebner@proxmox.com>
Link: https://lore.proxmox.com/da2cba4c-a6ce-40e9-837b-5d2fd289bdca@proxmox.com
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 pbs-key-config/src/lib.rs | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/pbs-key-config/src/lib.rs b/pbs-key-config/src/lib.rs
index 0cffdf008..d76a24d84 100644
--- a/pbs-key-config/src/lib.rs
+++ b/pbs-key-config/src/lib.rs
@@ -16,6 +16,14 @@ use pbs_tools::crypt_config::CryptConfig;
 
 const EXPECTED_KEY_LEN: usize = 32;
 
+/// Length of the AES-256-GCM IV prepended to a password protected key.
+const IV_LEN: usize = 16;
+/// Length of the AES-256-GCM tag stored after the IV of a password protected key.
+const TAG_LEN: usize = 16;
+/// Length of the data of a password protected key: IV, tag and the encrypted key, which has the
+/// same length as the plain one.
+const ENCRYPTED_KEY_LEN: usize = IV_LEN + TAG_LEN + EXPECTED_KEY_LEN;
+
 /// Key derivation function configuration
 #[derive(Deserialize, Serialize, Clone, Debug)]
 pub enum KeyDerivationConfig {
@@ -150,8 +158,8 @@ impl KeyConfig {
 
         let cipher = openssl::symm::Cipher::aes_256_gcm();
 
-        let iv = proxmox_sys::linux::random_data(16)?;
-        let mut tag = [0u8; 16];
+        let iv = proxmox_sys::linux::random_data(IV_LEN)?;
+        let mut tag = [0u8; TAG_LEN];
 
         let encrypted_key =
             openssl::symm::encrypt_aead(cipher, &derived_key, Some(&iv), b"", raw_key, &mut tag)?;
@@ -186,9 +194,9 @@ impl KeyConfig {
                 "unexpected key length ({} != {EXPECTED_KEY_LEN})",
                 key_config.data.len()
             )
-        } else if key_config.kdf.is_some() && key_config.data.len() < EXPECTED_KEY_LEN {
+        } else if key_config.kdf.is_some() && key_config.data.len() != ENCRYPTED_KEY_LEN {
             bail!(
-                "unexpected short data for key ({} < {EXPECTED_KEY_LEN})",
+                "unexpected encrypted key length ({} != {ENCRYPTED_KEY_LEN})",
                 key_config.data.len()
             );
         }
@@ -211,12 +219,15 @@ impl KeyConfig {
 
             let derived_key = kdf.derive_key(&passphrase)?;
 
-            if raw_data.len() < EXPECTED_KEY_LEN + 1 {
-                bail!("Unable to decrypt key - short data");
+            if raw_data.len() != ENCRYPTED_KEY_LEN {
+                bail!(
+                    "Unable to decrypt key - unexpected data length {} (expected {ENCRYPTED_KEY_LEN})",
+                    raw_data.len()
+                );
             }
-            let iv = &raw_data[0..16];
-            let tag = &raw_data[16..32];
-            let enc_data = &raw_data[32..];
+            let iv = &raw_data[..IV_LEN];
+            let tag = &raw_data[IV_LEN..IV_LEN + TAG_LEN];
+            let enc_data = &raw_data[IV_LEN + TAG_LEN..];
 
             let cipher = openssl::symm::Cipher::aes_256_gcm();
 
-- 
2.47.3