Fix pcr_digest handling and harden error paths

[sergio-correia]

The pcr_digest configuration field was accepted but silently ignored during encryption because tpm2-policy always reads live PCR values. This series fixes that by manually constructing a trial policy session with the caller-supplied digest when pcr_digest is present, enabling sealing to predicted or future PCR values (e.g., before a kernel update). Input validation in normalize() now catches malformed configurations early with clear error messages: invalid base64, wrong digest length, out-of-range PCR IDs, and incompatible option combinations.

The series also fixes several panic paths reachable from untrusted input. On the decrypt path, get_hash_alg_from_name() panicked on unsupported hash names and Tpm2Inner::get_pcr_ids() panicked on non-numeric PCR ID strings, both reachable via crafted JWE tokens stored on disk. These now return proper errors instead of crashing the process during early-boot decryption. The sealed object's nameAlg was hardcoded to SHA-256, causing TPM2_Create to fail when the policy digest size didn't match; it now respects the user's hash configuration. Non-SHA-256 hash with PCR binding is explicitly rejected since tpm2-policy 0.6.0 hardcodes SHA-256 for policy sessions on the decrypt path.

[sarroutbi]

Can you please evaluate next findings provided by Gemini?:

1. Robust Session Management (src/main.rs)

This ensures the TPM trial session is flushed even if the policy calculation fails, preventing session slot exhaustion.

// Replace the session handling logic in compute_policy_digest_with_pcr_digest
let trial_session = ctx
    .start_auth_session(
        None,
        None,
        None,
        SessionType::Trial,
        SymmetricDefinition::AES_128_CFB,
        name_hash_alg,
    )?
    .ok_or_else(|| anyhow::anyhow!("Failed to create trial session"))?;

let session_handle: tss_esapi::handles::SessionHandle = trial_session.into();

// Execute policy operations and guarantee session flush
let result = {
    let res = (|| -> Result<tss_esapi::structures::Digest> {
        ctx.policy_pcr(session_handle, hashed_data, pcr_sel)?;
        Ok(ctx.policy_get_digest(session_handle)?)
    })();

    // Ensure the session is flushed regardless of success or failure of the policy steps
    let _ = ctx.flush_context(session_handle.into());
    res
};

Ok((None, Some(result?)))

---

2. Strict Algorithm and Length Validation (src/cli.rs)

This prevents "silent" encryption success where the resulting object can never be decrypted due to algorithm or ordering mismatches.

// Add/update these checks inside TPM2Config::normalize()
if let Some(ref digest_b64) = self.pcr_digest {
    let decoded = base64::engine::general_purpose::URL_SAFE_NO_PAD
        .decode(digest_b64)
        .map_err(|e| anyhow::anyhow!("invalid pcr_digest base64: {}", e))?;

    let pcr_bank_alg = self.get_pcr_hash_alg()?;
    let name_alg = self.get_name_hash_alg()?;

    // 1. Ensure the Name Algorithm matches the PCR Bank to avoid complex double-hash mismatches
    if pcr_bank_alg != name_alg {
        bail!("When using pcr_digest, 'hash' (nameAlg) must match 'pcr_bank' to ensure policy consistency.");
    }

    // 2. Validate exact length based on number of PCRs and hash size
    if let Some(ref pcr_ids_val) = self.pcr_ids {
        let num_pcrs = match pcr_ids_val {
            serde_json::Value::Array(v) => v.len(),
            _ => bail!("pcr_ids must be an array"),
        };
        
        let hash_size = crate::utils::hash_digest_size(self.pcr_bank.as_ref())?;
        let expected_len = num_pcrs * hash_size;

        if decoded.len() != expected_len {
            bail!(
                "pcr_digest length mismatch: expected {} bytes ({} PCRs * {} bytes), but got {} bytes. \
                 Ensure values are concatenated in ascending numerical order of PCR index.",
                expected_len,
                num_pcrs,
                hash_size,
                decoded.len()
            );
        }
    }
}

---

3. Safer PCR ID Parsing (src/main.rs)

Prevents process panics when encountering malformed or trailing-comma strings in stored JWE headers.

// Replace Tpm2Inner::get_pcr_ids in src/main.rs
fn get_pcr_ids(&self) -> Result<Option<Vec<u64>>> {
    match &self.pcr_ids {
        None => Ok(None),
        Some(ids) => {
            let parsed: Result<Vec<u64>> = ids
                .split(',')
                .filter(|s| !s.trim().is_empty()) // Prevent errors on trailing or double commas
                .map(|x| {
                    x.trim()
                        .parse::<u64>()
                        .map_err(|e| anyhow::anyhow!("Invalid PCR ID '{}': {}", x, e))
                })
                .collect();
            Ok(Some(parsed?))
        }
    }
}

[sergio-correia]

Thanks for the thorough review! Here are my thoughts on each item:

1. Robust Session Management

The suggested pattern converts AuthSession to SessionHandle upfront and passes it to policy_pcr() and policy_get_digest(). Unfortunately this won't compile: both methods require PolicySession, not SessionHandle, and tss-esapi has no From<SessionHandle> for PolicySession conversion (the conversion is one-way only:
PolicySession -> SessionHandle). The SessionHandle type is a plain handle wrapper that strips the session metadata (hashing algorithm, session type) that the policy APIs need.

The current code keeps the value as AuthSession (which is Copy), uses try_into() to get PolicySession for the policy calls, and converts to SessionHandle only for flush_context. The session is always flushed regardless of whether the policy operations succeed or fail, which was the original concern.

2. Strict Algorithm and Length Validation

The base64url-no-pad decoding, empty-digest rejection, and length validation (num_pcrs * hash_size) suggested here are already implemented in normalize() at src/cli.rs.

Regarding the pcr_bank != hash restriction: this would be incorrect per the TPM 2.0 specification. The PCR bank and the policy session hash (nameAlg) serve different purposes and are intentionally independent. Per TPM 2.0 Library Specification Part 3: Commands (v1.84), Section 23.7, Table 135, pcrDigest is defined as the "expected digest value of the selected PCR using the hash algorithm of the session." The PCR bank selection (TPML_PCR_SELECTION) determines which bank to read
PCR values from, while the session hash determines how those values are digested for the policy. Reading SHA-1 PCR values and hashing them with SHA-256 for the policy is valid and is how the TPM works.

Adding this restriction would break the existing (and tested) configuration {"pcr_bank": "sha1", "pcr_ids": [23]}, which works correctly end-to-end.

The actual consistency constraint is already enforced: non-SHA-256 hash is rejected when pcr_ids is set, because tpm2-policy 0.6.0 hardcodes SHA-256 for policy sessions on the decrypt path.

3. Safer PCR ID Parsing

The current code already handles trailing/double commas without panicking. For input like "7,,23", the empty string from split(',') fails parse::<u64>() and returns a descriptive error via map_err. No panic occurs.

Silently filtering empty segments would be the wrong approach for this code path. Tpm2Inner is deserialized from JWE token headers on the decrypt path (untrusted input). The pcr_ids string is produced by the encrypt path's get_pcr_ids_str(), which joins validated u64 values with commas and never produces trailing or double commas. Any deviation from that format indicates tampering or corruption, and the correct response is an error, not silent normalization. The encrypt path
(normalize_pcr_ids()) also rejects such strings as parse errors, so adding a filter on the decrypt side would create an asymmetry.

[sarroutbi]

Thanks for the thorough review! Here are my thoughts on each item:

1. Robust Session Management

The suggested pattern converts AuthSession to SessionHandle upfront and passes it to policy_pcr() and policy_get_digest(). Unfortunately this won't compile: both methods require PolicySession, not SessionHandle, and tss-esapi has no From<SessionHandle> for PolicySession conversion (the conversion is one-way only: PolicySession -> SessionHandle). The SessionHandle type is a plain handle wrapper that strips the session metadata (hashing algorithm, session type) that the policy APIs need.

The current code keeps the value as AuthSession (which is Copy), uses try_into() to get PolicySession for the policy calls, and converts to SessionHandle only for flush_context. The session is always flushed regardless of whether the policy operations succeed or fail, which was the original concern.

2. Strict Algorithm and Length Validation

The base64url-no-pad decoding, empty-digest rejection, and length validation (num_pcrs * hash_size) suggested here are already implemented in normalize() at src/cli.rs.

Regarding the pcr_bank != hash restriction: this would be incorrect per the TPM 2.0 specification. The PCR bank and the policy session hash (nameAlg) serve different purposes and are intentionally independent. Per TPM 2.0 Library Specification Part 3: Commands (v1.84), Section 23.7, Table 135, pcrDigest is defined as the "expected digest value of the selected PCR using the hash algorithm of the session." The PCR bank selection (TPML_PCR_SELECTION) determines which bank to read PCR values from, while the session hash determines how those values are digested for the policy. Reading SHA-1 PCR values and hashing them with SHA-256 for the policy is valid and is how the TPM works.

Adding this restriction would break the existing (and tested) configuration {"pcr_bank": "sha1", "pcr_ids": [23]}, which works correctly end-to-end.

The actual consistency constraint is already enforced: non-SHA-256 hash is rejected when pcr_ids is set, because tpm2-policy 0.6.0 hardcodes SHA-256 for policy sessions on the decrypt path.

3. Safer PCR ID Parsing

The current code already handles trailing/double commas without panicking. For input like "7,,23", the empty string from split(',') fails parse::<u64>() and returns a descriptive error via map_err. No panic occurs.

Silently filtering empty segments would be the wrong approach for this code path. Tpm2Inner is deserialized from JWE token headers on the decrypt path (untrusted input). The pcr_ids string is produced by the encrypt path's get_pcr_ids_str(), which joins validated u64 values with commas and never produces trailing or double commas. Any deviation from that format indicates tampering or corruption, and the correct response is an error, not silent normalization. The encrypt path (normalize_pcr_ids()) also rejects such strings as parse errors, so adding a filter on the decrypt side would create an asymmetry.

Thanks for the thorough reply ! Changes LGTM

[sergio-correia]

Thank you for the review!