Open Excel's password-protected workbooks again: accept a hash-size integrity key (#2712)

Since #2425 checks Agile encryption's dataIntegrity, every workbook Excel
encrypts fails with "the decrypted data failed its integrity check". The
reader keyed the HMAC with the decrypted encryptedHmacKey cut to the salt
size (16 bytes), as [MS-OFFCRYPTO] 2.3.4.14 describes it, but Excel's key
is as long as the hash: 64 bytes for SHA-512. The check now accepts a key
of either length, so spec-literal writers still open and a modified
package still fails.

The test encryptor writes the integrity key as Excel does, so every
encrypted fixture now has Excel's layout; a new test covers both key
lengths, a key of neither length, and tampering with each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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 {
@@ -464,15 +474,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)
 }