fix(xmldsig): validate HMAC before reference work

[polaz]

Summary

• Reject weak HMAC output before reference resolution/transforms in signing and verification.
• Reject inadmissible HMAC output before signing-key callbacks and preserve typed unsupported-method errors in signing preflight.
• Keep early HMAC parameter validation free of key metadata callbacks; template signing queries metadata once during key preflight.
• Share SignatureMethod parsing and enforce XML Schema integer whitespace rather than Unicode trimming.
• Cover SHA1/224/256/384/512 at every permitted octet truncation length, full-width defaults, wrong keys, changed data, malformed parameters and stricter policy.
• Preserve constant-time comparison and existing policy-controlled named/raw key loading; document the XMLDSig 1.1 contract.

Validation

• 3893/3893 workspace tests passed without skips or leak warnings.
• Reciprocal signing/verification against libxmlsec1 1.3.13 passed for every HMAC method, including minimum, explicit full and omitted output lengths.
• Workspace formatting, clippy, build and doctests passed.
• Alloc-only host and thumbv7em-none-eabihf checks passed.
• Six regression tests failed before the fixes and passed afterwards, including typed preflight errors, callback ordering and metadata callback count.

Closes #176

Summary by CodeRabbit

• Bug Fixes
  • Signing and verification now check HMAC output lengths and policy limits before processing references. Invalid or unsupported signature methods are rejected earlier.
  • HMAC output-length parsing follows XML Schema whitespace rules and rejects unsupported Unicode whitespace.
• Documentation
  • Clarified XMLDSig 1.1 HMAC output-length requirements, the default policy minimum, and conformance limits for truncated outputs.
• Tests
  • Added coverage for HMAC output lengths, policy limits, incorrect keys, tampered documents, and interoperability with xmlsec1.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-02T20:59:56.522965Z	9f9f56f	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: structured-world/xml-sec/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e680dbbb-f69e-4492-bcd8-dc916c20a9ce

📥 Commits

Reviewing files that changed from the base of the PR and between ae448f9 and 9f9f56f.

📒 Files selected for processing (5)

• docs/xmldsig.md
• src/policy.rs
• src/xmldsig/sign.rs
• src/xmldsig/verify.rs
• tests/hmac_pipeline.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• src/xmldsig/verify.rs
• docs/xmldsig.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

HMAC output-length parsing now follows XML Schema integer whitespace rules. Signing and verification validate HMAC output-length policies before reference processing. Tests cover HMAC algorithms, length policies, malformed values, key and payload changes, and xmlsec1 interoperability.

Changes

HMAC XMLDSig pipeline

Layer / File(s)	Summary
Signature method and HMAC length parsing src/xmldsig/parse.rs	parse_signed_info uses a shared SignatureMethod parser. HMAC output-length parsing trims only XML Schema whitespace. A test checks accepted and rejected whitespace.
Signing and verification policy preflight src/xmldsig/sign.rs, src/xmldsig/verify.rs, tests/hmac_pipeline.rs	Signing validates the signature method and HMAC output policy during template validation. Verification checks output policy before reference processing. Tests check early policy rejection, typed unsupported-algorithm errors, and key-metadata query counts.
Pipeline tests and documented contract tests/hmac_pipeline.rs, docs/xmldsig.md, src/policy.rs	Tests cover HMAC algorithms, output lengths, malformed values, key and payload changes, stricter policy, and optional xmlsec1 interoperability. Documentation and policy comments state the output-length requirements and validation timing.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9f9f5

No actionable merge-blocking risk remains; merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9f9f5

The reviewed changes strengthen early rejection of inadmissible HMAC parameters while preserving signature-length checks and atomic document signing. No introduced security regression was identified. Residual uncertainty concerns downstream key-provider integrations and behavior outside the reviewed library paths.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly supported security scope is caller-supplied XML signatures and templates, their reference-processing work, the signing document, and the configured key-provider callbacks. The available evidence does not establish a deployment-wide, tenant-wide, or infrastructure-privilege expansion.

Security Findings and Attack Paths

• observed — An untrusted document can select an inadmissible HMACOutputLength. The new checks reject that declaration before reference resolution or transforms, and before signing-key callbacks. Regression tests combine weak output with missing reference targets and an unavailable signing key to assert this ordering; they do not establish a newly introduced attack path.

Trust Boundaries and Controls

• observed — Early declaration validation does not replace final authentication checks. Verification retains exact decoded-length matching, delegates through policy-aware key methods, and the built-in HMAC verifier retains policy validation and constant-time comparison. Custom key-provider behavior remains an existing integration responsibility.

Resilience and Maintainability Implications

• observed — The added tests assert zero metadata callbacks for rejected weak output and exactly one for valid HMAC templates. Additional coverage exercises all five HMAC methods, wrong keys, changed payloads, malformed lengths, and stricter output policy. Interoperability coverage is conditional on xmlsec1 availability; these tests were inspected, not executed in this assessment.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: validating HMAC output before reference processing during XMLDSig signing and verification.
Linked Issues check	✅ Passed	The PR addresses the coding requirements in [#176]. It centralizes SignatureMethod parsing and applies XML Schema whitespace rules. It validates XMLDSig 1.1 minimum and octet-aligned HMAC lengths be…
Out of Scope Changes check	✅ Passed	The changed files cover HMAC parsing, signing, verification, policy documentation, XMLDSig pipeline tests, and related contract documentation. These changes directly implement or verify [#176]. No unr…

Full details: Docstring Coverage

Explanation

Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 5 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b78cfe40a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1848411ca8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".