Merge PR #2870

crates/xlsx/src/crypto.rs

@@ -169,6 +169,16 @@ impl Hash {
         }
     }
 
+    /// The digest's length in bytes.
+    fn digest_len(self) -> usize {
+        match self {
+            Hash::Sha1 => 20,
+            Hash::Sha256 => 32,
+            Hash::Sha384 => 48,
+            Hash::Sha512 => 64,
+        }
+    }
+
     /// The bytes the algorithm's compression function takes: 128 for SHA-384/512, else 64.
     fn block_size(self) -> usize {
         match self {
@@ -444,6 +454,8 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
     })
 }
 
+/// Decrypts an Agile package ([MS-OFFCRYPTO] 2.3.4.10): checks the password against the
+/// verifier, unwraps the package key, verifies the dataIntegrity HMAC, then decrypts the segments.
 fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptError> {
     let descriptor = agile_descriptor(xml)?;
     let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value, hmac_key, hmac_value } = descriptor;
@@ -464,15 +476,20 @@ fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptE
     let secret = field(&key_value, &BLOCK_KEY)?;
     let aes = Aes::new(secret.get(..key_data.key_len).ok_or_else(|| malformed("a short encryptedKeyValue"))?)?;
     // Package integrity, [MS-OFFCRYPTO] 2.3.4.14: both HMAC fields are encrypted with the
-    // package key (the verifier above only authenticates the password), keyed by a random
-    // salt-size array, computed over the whole EncryptedPackage stream, StreamSize field included.
+    // package key (the verifier above only authenticates the password), computed over the whole
+    // EncryptedPackage stream, StreamSize field included. The specification keys the HMAC with a
+    // random salt-size array, but Excel's key is as long as the hash (64 bytes for SHA-512), so
+    // both lengths are tried.
     let mut mac_field = hmac_key;
     aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_KEY)), &mut mac_field)?;
-    mac_field.truncate(key_data.salt.len());
     let mut declared = hmac_value;
     aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_VALUE)), &mut declared)?;
-    let mac = hmac(key_data.hash, &mac_field, package);
-    if declared.get(..mac.len()) != Some(mac.as_slice()) {
+    let verified = [key_data.hash.digest_len(), key_data.salt.len()].into_iter().any(|len| {
+        let Some(key) = mac_field.get(..len) else { return false };
+        let mac = hmac(key_data.hash, key, package);
+        declared.get(..mac.len()) == Some(mac.as_slice())
+    });
+    if !verified {
         return Err(malformed("the decrypted data failed its integrity check"));
     }
     let (size, data) = package_parts(package)?;

crates/xlsx/src/crypto/tests.rs

@@ -3,7 +3,7 @@
 //! files are built in code by `crate::testing`. No Excel-produced file is used.
 
 use super::*;
-use crate::testing::{agile_info, encrypt_agile, encrypt_standard, with_agile_header};
+use crate::testing::{agile_info, encrypt_agile, encrypt_agile_with_hmac_key, encrypt_standard, with_agile_header};
 
 const PASSWORD: &str = "Pass1234";
 /// 16 zero bytes (whole AES blocks), a stand-in for a real integrity field where the values
@@ -231,3 +231,20 @@ fn integrity_matches_the_stream_not_the_plaintext() {
     assert!(matches!(decrypt_streams(&info, &ours, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
     assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain);
 }
+
+#[test]
+fn integrity_keys_as_long_as_the_hash_or_the_salt_verify() {
+    // Excel keys the HMAC with 64 bytes (#2712); [MS-OFFCRYPTO] 2.3.4.14 describes a salt-size
+    // key. Both verify, and neither lets a modified package through.
+    let plain = payload(5000);
+    for key in [vec![7u8; 64], vec![7u8; 16]] {
+        let (info, package) = encrypt_agile_with_hmac_key(&plain, PASSWORD, 1000, &key);
+        assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain, "{}-byte key", key.len());
+        let mut tampered = package.clone();
+        tampered[100] ^= 1;
+        assert!(matches!(decrypt_streams(&info, &tampered, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
+    }
+    // Another length (a key that matches neither) fails.
+    let (info, package) = encrypt_agile_with_hmac_key(&plain, PASSWORD, 1000, &[7u8; 32]);
+    assert!(matches!(decrypt_streams(&info, &package, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
+}

crates/xlsx/src/testing.rs

@@ -21,7 +21,13 @@ const KEY_DATA_SALT: [u8; 16] = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,
 const SECRET_KEY: [u8; 32] =
     [64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95];
 const VERIFIER: [u8; 16] = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47];
-const HMAC_SALT: [u8; 16] = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127];
+/// The dataIntegrity HMAC key, as long as the hash: Excel writes 64 bytes for SHA-512, not the
+/// salt-size key [MS-OFFCRYPTO] 2.3.4.14 describes.
+const HMAC_KEY: [u8; 64] = [
+    112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140,
+    141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169,
+    170, 171, 172, 173, 174, 175,
+];
 
 /// `package` (normally an `.xlsx` zip) as Excel stores it with a password: Agile encryption
 /// (AES-256, SHA-512) inside a compound file.
@@ -237,6 +243,11 @@ pub(crate) fn with_agile_header(xml: &str) -> Vec<u8> {
 /// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512), with the
 /// dataIntegrity HMAC over the whole EncryptedPackage stream as Excel writes it.
 pub(crate) fn encrypt_agile(plain: &[u8], password: &str, spin: u32) -> (Vec<u8>, Vec<u8>) {
+    encrypt_agile_with_hmac_key(plain, password, spin, &HMAC_KEY)
+}
+
+/// [`encrypt_agile`] with the dataIntegrity HMAC keyed by `hmac_key`.
+pub(crate) fn encrypt_agile_with_hmac_key(plain: &[u8], password: &str, spin: u32, hmac_key: &[u8]) -> (Vec<u8>, Vec<u8>) {
     let key_data = Params { hash: Hash::Sha512, salt: KEY_DATA_SALT.to_vec(), key_len: 32 };
     let mut body = vec![];
     for (i, segment) in plain.chunks(SEGMENT).enumerate() {
@@ -244,8 +255,8 @@ pub(crate) fn encrypt_agile(plain: &[u8], password: &str, spin: u32) -> (Vec<u8>
     }
     let stream = package(plain.len(), body);
     let (hmac_key_b64, hmac_value_b64) = (
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_KEY)), &HMAC_SALT)),
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_VALUE)), &hmac(Hash::Sha512, &HMAC_SALT, &stream))),
+        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_KEY)), hmac_key)),
+        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_VALUE)), &hmac(Hash::Sha512, hmac_key, &stream))),
     );
     (with_agile_header(&agile_info(password, spin, &hmac_key_b64, &hmac_value_b64)), stream)
 }