From 9e0f14cab9d0d8cd0d693a096df8fc677abfc2cc Mon Sep 17 00:00:00 2001 From: truongit Date: Thu, 29 Oct 2026 14:42:37 +0700 Subject: [PATCH] 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 --- crates/xlsx/src/crypto.rs | 25 ++++++++++++++++++++----- crates/xlsx/src/crypto/tests.rs | 19 ++++++++++++++++++- crates/xlsx/src/testing.rs | 17 ++++++++++++++--- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/crates/xlsx/src/crypto.rs b/crates/xlsx/src/crypto.rs index 0a63044a0..cafe08d17 100644 --- a/crates/xlsx/src/crypto.rs +++ b/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, 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)?; diff --git a/crates/xlsx/src/crypto/tests.rs b/crates/xlsx/src/crypto/tests.rs index 5f14686e9..56ac7d8a2 100644 --- a/crates/xlsx/src/crypto/tests.rs +++ b/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"))); +} diff --git a/crates/xlsx/src/testing.rs b/crates/xlsx/src/testing.rs index 5729250bb..a82283644 100644 --- a/crates/xlsx/src/testing.rs +++ b/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 { /// `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, Vec) { + 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, Vec) { 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 } 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) }