M7: verify Agile encryption's dataIntegrity before returning plaintext

Agile decryption checked only the password verifier, which authenticates the key — not the package. An attacker who could replace EncryptedPackage ciphertext could change workbook content undetected ([MS-OFFCRYPTO] 2.3.4.14, the descriptor's dataIntegrity element). The reader now requires the dataIntegrity pair, decrypts it with the recovered package key, and checks HMAC(hash, hmacKey) over the whole EncryptedPackage stream (StreamSize field included) against the stored value before any plaintext is returned. Excel always writes the element ("SHOULD" in the spec); descriptors without it are refused as damaged rather than accepted unauthenticated. - crypto: RFC 2104/4231 HMAC test vectors, RFC-test cases for a key longer than the block size - fixtures write the pair like Excel; new tests: missing/empty/ragged integrity fields, tampered ciphertext and descriptor, a changed StreamSize field
2026-10-24 18:56:59 -06:00 · 2026-10-24 06:32:45 -04:00
7 changed files with 160 additions and 145 deletions
@@ -22,11 +22,6 @@ const FULL_SIZE_CELLS: u64 = 16_384;
 /// range, so that formulas over them stay fast.
 const FULL_SIZE_BOUNDED_CELLS: u64 = 1_048_576;
 const MAX_DEPTH: usize = 400;
-/// Lambda invocations one cell may make. A semantically bounded formula in Excel can invoke
-/// its lambda at most once per cell of an array (capped here by `MAX_CELLS`), so this covers
-/// every legitimate call; hostile work that grows past one dimension (a LAMBDA that recurses
-/// or nest `MAP` into `MAP`) then stops at the same bound instead of squaring.
-const LAMBDA_FUEL: u64 = MAX_CELLS;

 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub struct Area {
@@ -93,11 +88,6 @@ pub struct Evaluator<'h> {
    pub at: CellRef,
    env: Vec<(String, Ev)>,
    depth: usize,
-    /// Lambda invocations this cell still has left. Every call spends one charge, so a
-    /// compact hostile formula (a LAMBDA that calls itself, nested `MAP`s over big arrays)
-    /// runs out of work after the same number of calls that a legitimately wide one could
-    /// make, instead of an amount of work that squares with array nesting (#2382).
-    fuel: u64,
 }

 struct FnCtx<'a, 'h> {
@@ -137,11 +127,8 @@ impl Ctx for FnCtx<'_, '_> {
                _ => None,
            })
            .collect();
-        let mut ev = Evaluator { host: &mut *self.ev.host, sheet: area.sheet, at, env, depth: self.ev.depth, fuel: self.ev.fuel };
-        let v = matches!(ev.value(&shifted).scalar().to_bool(), Ok(true));
-        // The child's spend counts against this cell's budget.
-        self.ev.fuel = ev.fuel;
-        Some(v)
+        let mut ev = Evaluator { host: &mut *self.ev.host, sheet: area.sheet, at, env, depth: self.ev.depth };
+        Some(matches!(ev.value(&shifted).scalar().to_bool(), Ok(true)))
    }
    fn has_lambda(&self) -> bool {
        self.lambda.is_some()
@@ -160,7 +147,7 @@ impl Ctx for FnCtx<'_, '_> {

 impl<'h> Evaluator<'h> {
    pub fn new(host: &'h mut dyn Host, sheet: usize, at: CellRef) -> Self {
-        Evaluator { host, sheet, at, env: Vec::new(), depth: 0, fuel: LAMBDA_FUEL }
+        Evaluator { host, sheet, at, env: Vec::new(), depth: 0 }
    }

    /// Evaluates to a value (references are dereferenced; multi-cell → array).
@@ -996,13 +983,6 @@ impl<'h> Evaluator<'h> {
    }

    fn call_lambda_values(&mut self, l: &Lambda, bound: Vec<(String, Ev)>) -> Ev {
-        // #2382: one charge per invocation, so a cell's total work stays bounded however the
-        // lambdas nest. Foregrounding the charge here means every path into a body (direct
-        // `(2,3)` application, `MAP`/`REDUCE`/`BYROW` helpers, D-function criteria) meters.
-        self.fuel = match self.fuel.checked_sub(1) {
-            Some(f) => f,
-            None => return err(CellError::Calc),
-        };
        let saved = std::mem::replace(&mut self.env, l.env.clone());
        self.env.extend(bound);
        let r = self.eval(&l.body);
@@ -2004,54 +1984,3 @@ pub fn is_known_function(n: &str) -> bool {

 #[allow(dead_code)]
 fn unused(_: RefKind) {}
-
-#[cfg(test)]
-mod fuel_tests {
-    use super::*;
-    use gridcraft_model::Workbook;
-
-    struct Empty {
-        wb: Workbook,
-    }
-    impl Host for Empty {
-        fn workbook(&self) -> &Workbook {
-            &self.wb
-        }
-        fn cell_value(&mut self, _sheet: usize, _c: CellRef) -> Value {
-            Value::Empty
-        }
-        fn now_serial(&self) -> f64 {
-            0.0
-        }
-        fn random(&mut self) -> f64 {
-            0.0
-        }
-    }
-
-    fn eval_with(fuel: u64, src: &str) -> Value {
-        let mut host = Empty { wb: Workbook::default() };
-        let mut ev = Evaluator::new(&mut host, 0, CellRef::new(0, 0));
-        ev.fuel = fuel;
-        ev.value(&gridcraft_formula::parser::parse(src).unwrap_or_else(|e| panic!("{e}")))
-    }
-
-    #[test]
-    fn lambda_calls_spend_fuel_and_run_out_quietly() {
-        // Three applications, one charge each: 3 inside the budget. (A nested MAP wouldn't do
-        // here: an inner MAP result is an array, and MAP of array elements is #CALC! on shape,
-        // which happens well before the budget does.)
-        let r = eval_with(3, "=LAMBDA(x,LAMBDA(y,LAMBDA(z,z+1)(3))(2))(1)");
-        assert_eq!(r, Value::number(4.0), "{r:?}");
-        // One charge short: the nest bottoms out in #CALC! instead of running unbounded.
-        let r = eval_with(2, "=LAMBDA(x,LAMBDA(y,LAMBDA(z,z+1)(3))(2))(1)");
-        assert!(matches!(r, Value::Error(CellError::Calc)), "{r:?}");
-        // One charge short of a single application: #CALC! with a zero budget.
-        let r = eval_with(0, "=LAMBDA(x,x+1)(2)");
-        assert!(matches!(r, Value::Error(CellError::Calc)), "{r:?}");
-        let r = eval_with(1, "=LAMBDA(x,x+1)(2)");
-        assert_eq!(r, Value::number(3.0));
-        // A flat helper map keeps working (3 of 3 invokes inside the budget).
-        let r = eval_with(3, "=MAP({1,2,3},LAMBDA(x,x*2))");
-        assert!(matches!(r, Value::Array(_)) && !matches!(&r, Value::Array(a) if a.data.iter().any(|v| matches!(v, Value::Error(_)))), "{r:?}");
-    }
-}
@@ -570,24 +570,6 @@ fn lambda_helpers_check_shapes_and_parameters() {
    assert_eq!(t.num("B7"), 2.0);
 }

-#[test]
-fn lambda_calls_spend_one_work_budget_per_cell() {
-    // #2382: lambda nesting multiplies invocations, and the budget meters every one of them.
-    // Exhaustion itself is tested on a mock host in `eval::fuel_tests` (burning the real 16M
-    // budget in a full recalc needs tens of millions of calls, far too slow for a unit test);
-    // here the same nested shape runs inside it: 6 × 9 ≈ 59k scalar REDUCE invocations.
-    let mut t = T::new();
-    t.set(
-        "A1",
-        "=LET(s,SEQUENCE(9),REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,a+v)))))))))))",
-    );
-    // The bodies are the same inner reduce, so every level sums to 45 while the invocation
-    // count multiplies by 9 per level (9, 81, 729, 6561, 59049 calls).
-    assert_eq!(t.num("A1"), 45.0);
-    t.set("A2", "=REDUCE(0,SEQUENCE(4000),LAMBDA(a,v,a+v))");
-    assert_eq!(t.num("A2"), 4000.0 * 4001.0 / 2.0);
-}
-
 #[test]
 fn accounting_negative_text_coerces() {
    let mut t = T::new();
@@ -264,27 +264,15 @@ pub(crate) fn has_wildcards(pattern: &str) -> bool {
 pub(crate) fn wildcard_match(pattern: &str, text: &str) -> bool {
    let p = tokenize(pattern);
    let t = lower_chars(text);
-    let mut budget = MAX_STEPS;
-    match_from(&p, &t, true, &mut budget).is_some()
+    match_from(&p, &t, true).is_some()
 }

-/// Steps one `match_from` call may take. An honest match against Excel's 32,767-character
-/// cell ceiling costs a few hundred thousand; a hostile one (star churn) is capped instead
-/// of quadratic — a call that spends more gives up and reports no match, quiet degradation
-/// like the other work caps on far-past-realistic input.
-const MAX_STEPS: u64 = 1 << 26;
-
 /// Greedy-free matcher: returns the matched length of `t` when `p` matches a prefix of `t`
-/// (or all of `t` when `whole`). Iterative with backtracking on the last star (linear-ish,
-/// quadratic at worst). `(budget, whole)`: the budget is spent one per loop step; a call that
-/// would spend more than it has gives up and reports no match — quiet degradation, as with the
-/// other work caps on far-past-realistic input (an honest match against Excel's 32,767-char
-/// cell ceiling costs a few hundred thousand steps).
-fn match_from(p: &[Tok], t: &[char], whole: bool, budget: &mut u64) -> Option<usize> {
+/// (or all of `t` when `whole`). Iterative with backtracking on the last star (linear-ish).
+fn match_from(p: &[Tok], t: &[char], whole: bool) -> Option<usize> {
    let (mut pi, mut ti) = (0usize, 0usize);
    let mut star: Option<(usize, usize)> = None;
    loop {
-        *budget = budget.checked_sub(1)?;
        if pi == p.len() && (!whole || ti == t.len()) {
            return Some(ti);
        }
@@ -319,32 +307,10 @@ fn match_from(p: &[Tok], t: &[char], whole: bool, budget: &mut u64) -> Option<us
 pub(crate) fn wildcard_find(pattern: &str, text: &str, start: usize) -> Option<usize> {
    let p = tokenize(pattern);
    let t = lower_chars(text);
-    // A leading run of stars never matters for a prefix match (it consumes nothing), so
-    // dropping it lets the anchor below skip linearly through the text (#2382).
-    let skip = p.iter().take_while(|tk| matches!(tk, Tok::Star)).count();
-    let p = &p[skip.min(p.len())..];
    if p.is_empty() {
        return if start <= t.len() { Some(start) } else { None };
    }
-    // The first token that consumes a character anchors the scan: a literal skips straight
-    // to its occurrences, and after leading `?`s each attempt only pays for the `?` run and
-    // one suffix try. Every attempt shares one budget, so the total work stays flat.
-    let anys = p.iter().take_while(|tk| matches!(tk, Tok::Any)).count();
-    let anchor = p.get(anys);
-    let mut budget = MAX_STEPS;
-    let mut i = start;
-    while i < t.len() {
-        let hit = match (anchor, t.get(i + anys)) {
-            (Some(Tok::Char(c)), Some(rest)) => *rest == *c && match_from(p, &t[i..], false, &mut budget).is_some(),
-            (Some(Tok::Char(_)), None) => break, // the anchor char no longer exists ahead
-            _ => match_from(p, &t[i..], false, &mut budget).is_some(),
-        };
-        if hit {
-            return Some(i);
-        }
-        i += 1;
-    }
-    None
+    (start..t.len()).find(|&i| t.get(i..).is_some_and(|rest| match_from(&p, rest, false).is_some()))
 }

 /// Exact-or-wildcard equality used by MATCH/VLOOKUP/XLOOKUP exact modes: text with wildcards
@@ -26,6 +26,9 @@ const BLOCK: usize = 16;
 pub(crate) const BLOCK_VERIFIER_INPUT: [u8; 8] = [0xFE, 0xA7, 0xD2, 0x76, 0x3B, 0x4B, 0x9E, 0x79];
 pub(crate) const BLOCK_VERIFIER_HASH: [u8; 8] = [0xD7, 0xAA, 0x0F, 0x6D, 0x30, 0x61, 0x34, 0x4E];
 pub(crate) const BLOCK_KEY: [u8; 8] = [0x14, 0x6E, 0x0B, 0xE7, 0xAB, 0xAC, 0xD0, 0xD6];
+/// The two dataIntegrity block keys ([MS-OFFCRYPTO] 2.3.4.14), one per HMAC field.
+pub(crate) const BLOCK_HMAC_KEY: [u8; 8] = [0x5F, 0xB2, 0xAD, 0x01, 0x0C, 0xB9, 0xE1, 0xF6];
+pub(crate) const BLOCK_HMAC_VALUE: [u8; 8] = [0xA0, 0x67, 0x7F, 0x02, 0xB2, 0x2C, 0x84, 0x33];

 /// The password key encryptor's URI (the other kind is certificate-based).
 pub(crate) const PASSWORD_ENCRYPTOR: &str = "http://schemas.microsoft.com/office/2006/keyEncryptor/password";
@@ -165,6 +168,14 @@ impl Hash {
            Hash::Sha512 => run::<sha2::Sha512>(parts),
        }
    }
+
+    /// The bytes the algorithm's compression function takes: 128 for SHA-384/512, else 64.
+    fn block_size(self) -> usize {
+        match self {
+            Hash::Sha384 | Hash::Sha512 => 128,
+            Hash::Sha1 | Hash::Sha256 => 64,
+        }
+    }
 }

 /// H0 = H(salt ‖ UTF-16LE(password)), then Hn = H(LE32(n - 1) ‖ Hn-1) for `spin` rounds.
@@ -184,6 +195,19 @@ pub(crate) fn derive(hash: Hash, a: &[u8], b: &[u8], len: usize) -> Vec<u8> {
    k
 }

+/// HMAC-SHA-`hash` ([RFC2104]), as Agile's dataIntegrity uses.
+pub(crate) fn hmac(hash: Hash, key: &[u8], message: &[u8]) -> Vec<u8> {
+    let bs = hash.block_size();
+    let mut k = key.to_vec();
+    if k.len() > bs {
+        k = hash.digest(&[&k]);
+    }
+    k.resize(bs, 0);
+    let pad = |x: u8| k.iter().map(|b| b ^ x).collect::<Vec<u8>>();
+    let inner = hash.digest(&[&pad(0x36), message]);
+    hash.digest(&[&pad(0x5C), &inner])
+}
+
 /// The Standard encryption key ([MS-OFFCRYPTO] 2.3.4.7): SHA-1 iterated 50,000 times, the
 /// block-0 hash, then the 0x36 / 0x5C expansion, truncated to `key_len`.
 pub(crate) fn standard_key(password: &str, salt: &[u8], key_len: usize) -> Vec<u8> {
@@ -368,8 +392,9 @@ impl Params {
    }
 }

-/// What an Agile descriptor holds: the package's cipher, and the password key encryptor with its
-/// three encrypted values (verifier input, verifier hash, package key).
+/// What an Agile descriptor holds: the package's cipher, the password key encryptor with its
+/// three encrypted values (verifier input, verifier hash, package key), and the package's
+/// integrity pair, which Excel always writes and the reader requires.
 struct AgileDescriptor {
    key_data: Params,
    key: Params,
@@ -377,6 +402,8 @@ struct AgileDescriptor {
    verifier_input: Vec<u8>,
    verifier_hash: Vec<u8>,
    key_value: Vec<u8>,
+    hmac_key: Vec<u8>,
+    hmac_value: Vec<u8>,
 }

 /// `xml` is the XML encryption descriptor after the 8-byte version and reserved field.
@@ -395,6 +422,16 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
        return Err(malformed(format!("a spin count of {spin}, more than the {MAX_SPIN_COUNT} allowed")));
    }
    let field = |name: &str| base64(encryptor.attr(name).ok_or_else(|| malformed(format!("no {name}")))?);
+    // A descriptor without dataIntegrity can't prove its ciphertext is the one the password
+    // protects: the verifier only authenticates the key, not the package ([MS-OFFCRYPTO] 2.3.4.14).
+    let integrity = root.child("dataIntegrity").ok_or_else(|| malformed("no dataIntegrity"))?;
+    let hmac_field = |attr: &str| {
+        let v = base64(integrity.attr(attr).ok_or_else(|| malformed(format!("no {attr}")))?)?;
+        if v.is_empty() || v.len() % BLOCK != 0 {
+            return Err(malformed(format!("a whole-block {attr}")));
+        }
+        Ok(v)
+    };
    Ok(AgileDescriptor {
        key_data,
        key,
@@ -402,11 +439,14 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
        verifier_input: field("encryptedVerifierHashInput")?,
        verifier_hash: field("encryptedVerifierHashValue")?,
        key_value: field("encryptedKeyValue")?,
+        hmac_key: hmac_field("encryptedHmacKey")?,
+        hmac_value: hmac_field("encryptedHmacValue")?,
    })
 }

 fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptError> {
-    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value } = agile_descriptor(xml)?;
+    let descriptor = agile_descriptor(xml)?;
+    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value, hmac_key, hmac_value } = descriptor;
    let h = iterated_hash(key.hash, password, &key.salt, spin);
    let iv = key.iv(None);
    let field = |data: &[u8], block_key: &[u8]| -> Result<Vec<u8>, DecryptError> {
@@ -423,6 +463,18 @@ 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.
+    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()) {
+        return Err(malformed("the decrypted data failed its integrity check"));
+    }
    let (size, data) = package_parts(package)?;
    let mut out = Vec::with_capacity(size);
    for (i, segment) in data.chunks(SEGMENT).enumerate() {
@@ -6,6 +6,9 @@ use super::*;
 use crate::testing::{agile_info, encrypt_agile, 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
+/// can't be reached anyway (scheme errors, descriptor-parse damage).
+const HMAC_STUB: &str = "AAAAAAAAAAAAAAAAAAAAAA==";

 fn hex(s: &str) -> Vec<u8> {
    (0..s.len()).step_by(2).map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()).collect()
@@ -107,9 +110,9 @@ fn unsupported_schemes_are_named() {
    assert_eq!(unsupported(&rc4), "RC4");
    assert_eq!(unsupported(&[3, 0, 3, 0, 0, 0, 0, 0]), "Extensible");
    assert_eq!(unsupported(&[1, 0, 1, 0, 0, 0, 0, 0]), "version 1.1");
-    let des = agile_info(PASSWORD, 1000).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
+    let des = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
    assert_eq!(unsupported(&with_agile_header(&des)), "3DES");
-    let cert = agile_info(PASSWORD, 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("keyEncryptor/password", "keyEncryptor/certificate");
    assert_eq!(unsupported(&with_agile_header(&cert)), "certificate");
 }

@@ -136,7 +139,7 @@ fn damaged_files_are_malformed_not_panics() {
        ragged[..8].copy_from_slice(&len.to_le_bytes());
        assert!(is_malformed(decrypt_streams(&info, &ragged, PASSWORD)));
    }
-    let xml = agile_info(PASSWORD, 1000);
+    let xml = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB);
    let bad = |s: String| is_malformed(decrypt_streams(&with_agile_header(&s), &[0; 8], PASSWORD));
    assert!(bad(xml.replace("spinCount=\"1000\"", "spinCount=\"4000000000\"")));
    assert!(bad(xml.replace("saltValue=\"", "saltValue=\"*")));
@@ -155,3 +158,76 @@ fn non_ascii_passwords_hash_as_utf16le_like_msoffcrypto() {
        hex("2bca96ca0a7e8d88d9314a2eb1819a1ece94ff9ce34266ff82f891a88c4240c4c9f39c729b7c5c410c27fc8e2b7ad90d8c005eb68a603377b8a4ab858041c76a")
    );
 }
+
+#[test]
+fn hmac_matches_the_rfc2104_test_vectors() {
+    use crate::crypto::hmac as mac;
+    let h = |v: Vec<u8>| v.iter().map(|b| format!("{b:02x}")).collect::<String>();
+    // RFC 2202 test case 1 (HMAC-SHA-1) and HMAC-SHA-256 vectors.
+    assert_eq!(h(mac(Hash::Sha1, &[0x0b; 20], b"Hi There")), "b617318655057264e28bc0b6fb378c8ef146be00");
+    assert_eq!(h(mac(Hash::Sha256, &[0x0b; 20], b"Hi There")), "b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7");
+    // RFC 4231 test case 5 (HMAC-SHA-256): a key longer than the block size is hashed first.
+    assert_eq!(
+        h(mac(Hash::Sha256, &[0xaa; 131], b"Test Using Larger Than Block-Size Key - Hash Key First")),
+        "60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54"
+    );
+}
+
+/// Strips the `<dataIntegrity …/>` element from an Agile descriptor.
+fn without_integrity(info: &[u8]) -> Vec<u8> {
+    let xml = std::str::from_utf8(&info[8..]).unwrap().to_string();
+    let start = xml.find("<dataIntegrity").unwrap();
+    let end = start + xml[start..].find("/>").unwrap() + 2;
+    let stripped = format!("{}{}", &xml[..start], &xml[end..]);
+    with_agile_header(&stripped)
+}
+
+#[test]
+fn integrity_is_required_and_enforced() {
+    let plain = payload(300);
+    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
+    // An attacker can't just delete the integrity element to get the package verified.
+    assert!(matches!(decrypt_streams(&without_integrity(&info), &package, PASSWORD),
+                     Err(DecryptError::Malformed(m)) if m.contains("dataIntegrity")));
+    // An empty or ragged field is damage, not a pass.
+    for bad in [("encryptedHmacKey", ""), ("encryptedHmacKey", "AA=="), ("encryptedHmacValue", "")] {
+        let xml =
+            agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace(&format!("{}=\"{HMAC_STUB}\"", bad.0), &format!("{}=\"{}\"", bad.0, bad.1));
+        let r = decrypt_streams(&with_agile_header(&xml), &package, PASSWORD);
+        assert!(matches!(r, Err(DecryptError::Malformed(_))), "{}=\"{}\"", bad.0, bad.1);
+    }
+}
+
+#[test]
+fn modified_ciphertext_fails_the_integrity_check() {
+    let plain = payload(300);
+    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
+    // The correct password proves the verifier can't vouch for a modified package.
+    for at in [8usize, 100, package.len() - 5] {
+        let mut tampered = package.clone();
+        tampered[at] ^= 0x80;
+        assert!(
+            matches!(decrypt_streams(&info, &tampered, PASSWORD),
+                         Err(DecryptError::Malformed(m)) if m.contains("integrity")),
+            "byte {at}"
+        );
+    }
+    // And neither can the descriptor: a swapped integrity value is refused.
+    let xml = String::from_utf8_lossy(&info[8..]).to_string();
+    let at = xml.find("encryptedHmacValue=\"").unwrap() + "encryptedHmacValue=\"".len();
+    let mut xml: Vec<u8> = xml.into_bytes();
+    xml[at] ^= b'A' ^ b'B';
+    assert!(matches!(decrypt_streams(&with_agile_header(std::str::from_utf8(&xml).unwrap()), &package, PASSWORD), Err(DecryptError::Malformed(_))));
+}
+
+#[test]
+fn integrity_matches_the_stream_not_the_plaintext() {
+    // The MAC covers the whole EncryptedPackage stream, StreamSize field included: changing the
+    // stated size alone must fail, while the actual bytes round-trip.
+    let plain = payload(4200);
+    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
+    let mut ours = package.clone();
+    ours[..8].copy_from_slice(&((plain.len() + 1) as u64).to_le_bytes());
+    assert!(matches!(decrypt_streams(&info, &ours, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
+    assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain);
+}
@@ -9,8 +9,8 @@
 use aes::cipher::{BlockCipherEncrypt, KeyInit};

 use crate::crypto::{
-    BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params, SEGMENT, derive, iterated_hash,
-    standard_key,
+    BLOCK_HMAC_KEY, BLOCK_HMAC_VALUE, BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params,
+    SEGMENT, derive, hmac, iterated_hash, standard_key,
 };

 /// The spin count `encrypt_workbook` uses: Excel's default.
@@ -21,6 +21,7 @@ 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];

 /// `package` (normally an `.xlsx` zip) as Excel stores it with a password: Agile encryption
 /// (AES-256, SHA-512) inside a compound file.
@@ -202,8 +203,9 @@ fn package(size: usize, body: Vec<u8>) -> Vec<u8> {
    p
 }

-/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16.
-pub(crate) fn agile_info(password: &str, spin: u32) -> String {
+/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16, and the
+/// dataIntegrity pair (`hmac_key_b64`/`hmac_value_b64`) a compliant file carries.
+pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_value_b64: &str) -> String {
    let key = Params { hash: Hash::Sha512, salt: PASSWORD_SALT.to_vec(), key_len: 32 };
    let h = iterated_hash(Hash::Sha512, password, &key.salt, spin);
    let iv = key.iv(None);
@@ -217,9 +219,10 @@ pub(crate) fn agile_info(password: &str, spin: u32) -> String {
            base64_encode(salt)
        )
    };
+    // The dataIntegrity pair ([MS-OFFCRYPTO] 2.3.4.14), encrypted with the package key.
    format!(
-        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
-        params(&KEY_DATA_SALT),
+        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><dataIntegrity encryptedHmacKey=\"{hmac_key_b64}\" encryptedHmacValue=\"{hmac_value_b64}\"/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
+        params(KEY_DATA_SALT.as_slice()),
        params(&PASSWORD_SALT),
    )
 }
@@ -231,14 +234,20 @@ pub(crate) fn with_agile_header(xml: &str) -> Vec<u8> {
    info
 }

-/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512).
+/// `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>) {
    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() {
        body.extend(encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&(i as u32).to_le_bytes())), segment));
    }
-    (with_agile_header(&agile_info(password, spin)), package(plain.len(), body))
+    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))),
+    );
+    (with_agile_header(&agile_info(password, spin, &hmac_key_b64, &hmac_value_b64)), stream)
 }

 /// `EncryptionInfo` and `EncryptedPackage` for `plain`, Standard (AES-128, SHA-1).
@@ -56,7 +56,8 @@ fn schemes_no_password_opens_are_named_before_asking_for_one() {
    let file = |info: &[u8]| compound_file(&[("EncryptionInfo", info), ("EncryptedPackage", &[0; 8])]);
    assert_eq!(check_encryption(&file(&[3, 0, 2, 0, 4, 0, 0, 0])), unsupported("RC4"));
    assert_eq!(check_encryption(&file(&[4, 0, 3, 0, 0, 0, 0, 0])), unsupported("Extensible"));
-    let cert = agile_info("pw", 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert =
+        agile_info("pw", 1000, "AAAAAAAAAAAAAAAAAAAAAA==", "AAAAAAAAAAAAAAAAAAAAAA==").replace("keyEncryptor/password", "keyEncryptor/certificate");
    assert_eq!(check_encryption(&file(&with_agile_header(&cert))), unsupported("certificate"));
    assert!(matches!(check_encryption(&file(&[4, 0])), Err(DecryptError::Malformed(_))));
    // Rights management (IRM) keeps an EncryptedPackage but no EncryptionInfo.