feat(xmldsig): write x509 signing key info by polaz · Pull Request #81 · structured-world/xml-sec

polaz · 2026-07-11T00:55:51Z

Summary

Add a signing KeyInfoWriter API plus X509CertificateKeyInfoWriter for embedded X.509 certificate KeyInfo.
Populate direct / template placeholders during signing without changing no-writer output.
Target the appended/latest XMLDSig Signature during signing so already-signed documents keep existing Signature/KeyInfo blocks untouched.
Validate that embedded X.509 certificate public key matches the configured signing key before returning signed XML.
Update the p521 dependency requirement to stable 0.14.

Testing

cargo fmt -- --check
cargo check --all-features
cargo clippy --all-targets --all-features -- -D warnings
cargo nextest run --all-features -E 'binary(signing_digest)' — 16 passed
cargo nextest run --all-features --status-level fail --final-status-level slow — 536 passed
cargo test --doc --all-features — 3 passed

Closes #80

- Add signing KeyInfoWriter and X509 certificate writer - Populate direct Signature/KeyInfo placeholders during signing - Verify embedded certificate KeyInfo through DefaultKeyResolver Closes #80

coderabbitai · 2026-07-11T00:56:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dc1532d6-e097-40d7-a21a-374fb47fc0b1

📥 Commits

Reviewing files that changed from the base of the PR and between 6df8714 and b908e4a.

📒 Files selected for processing (3)

src/xmldsig/mutation.rs
src/xmldsig/sign.rs
tests/signing_digest.rs

📝 Walkthrough

Summary by CodeRabbit

New Features
- Added optional X.509 certificate embedding in XMLDSig signatures through KeyInfo.
- Added certificate validation to ensure it matches the signing key.
- Improved signing support for documents containing multiple signatures.
- Added public APIs for configuring certificate-based KeyInfo generation.
Documentation
- Updated XMLDSig feature and implementation status documentation to reflect expanded signing and verification support.
Bug Fixes
- Improved signature-scoped XML updates and validation for complex templates.

Walkthrough

XMLDSig signing now optionally embeds X.509 certificates in <KeyInfo>, targets the latest signature during mutation and digest processing, and exposes public writer APIs. Tests, documentation, and the optional p521 dependency were updated accordingly.

Changes

XMLDSig KeyInfo signing

Layer / File(s)	Summary
Signature-scoped XML mutation `src/xmldsig/mutation.rs`	Adds direct `<Signature>/<KeyInfo>` counting and raw replacement, while restricting digest and signature-value mutations to the last signature.
Certificate writer and signing integration `src/xmldsig/sign.rs`, `src/xmldsig/mod.rs`	Adds X.509 PEM/DER writers, SPKI matching, KeyInfo errors, optional `SignContext` integration, latest-signature selection, and public re-exports.
Signing verification coverage and release metadata `tests/signing_digest.rs`, `README.md`, `Cargo.toml`	Tests certificate serialization, verification, invalid templates, certificate mismatch, and multiple signatures; documentation and the optional `p521` dependency are updated.

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

Sequence Diagram(s)

sequenceDiagram
  participant SignContext
  participant X509CertificateKeyInfoWriter
  participant fill_key_info
  participant DefaultKeyResolver
  SignContext->>X509CertificateKeyInfoWriter: Generate embedded X.509 KeyInfo
  SignContext->>fill_key_info: Populate latest Signature KeyInfo
  fill_key_info-->>SignContext: Return signed XML
  DefaultKeyResolver->>SignContext: Resolve verification key from certificate

Possibly related PRs

structured-world/xml-sec#51: Adds related X.509 KeyInfo parsing used by the writer tests.
structured-world/xml-sec#77: Provides the XML mutation framework extended by this signing flow.
structured-world/xml-sec#79: Shares the signing and digest-processing paths updated for signature scoping.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main change: adding X.509 KeyInfo writing for XMLDSig signing.
Description check	✅ Passed	The description matches the changeset and summarizes the new writer API, signing behavior, tests, and dependency update.
Linked Issues check	✅ Passed	The PR appears to satisfy `#80` by adding KeyInfo writing, X.509 certificate embedding, placeholder handling, verification tests, and preserving no-writer behavior.
Out of Scope Changes check	✅ Passed	No obvious out-of-scope changes are present; the README, tests, and p521 bump all support the stated XMLDSig signing work.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/#80-keyinfo-writer

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

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

greptile-apps · 2026-07-11T00:59:58Z

Greptile Summary

This PR adds X.509 KeyInfo support to XMLDSig signing. The main changes are:

Adds a signing KeyInfoWriter API and X.509 certificate writer.
Fills appended signature DigestValue, SignatureValue, and KeyInfo placeholders during signing.
Validates that the embedded certificate public key matches the signing key.
Updates XMLDSig exports, README coverage, tests, and the p521 dependency.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex Logs

What T-Rex did

T-Rex created a log of the test plan execution that shows timestamped command sections, the exact commands run, and their exit statuses.
T-Rex noted that tooling was missing for formatting and linting and that cargo-nextest was not installed, as shown by the exit results.
T-Rex executed fallback tests and confirmed that the x509 subset 3 passed with 0 failures and the key_info subset 6 passed with 0 failures.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
src/xmldsig/mutation.rs	Scopes signing placeholder mutation to the latest XMLDSig signature and adds direct `KeyInfo` filling.
src/xmldsig/sign.rs	Adds the KeyInfo writer API, X.509 certificate embedding, certificate/key matching, and latest-signature signing selection.
src/xmldsig/mod.rs	Re-exports the new KeyInfo writer types and errors.
Cargo.toml	Updates the p521 dependency requirement to stable 0.14.
tests/signing_digest.rs	Adds signing tests for embedded X.509 KeyInfo, certificate/key mismatch rejection, and existing-signature handling.

_{Reviews (2): Last reviewed commit: "fix(xmldsig): harden signing key info wr..." | Re-trigger Greptile}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/signing_digest.rs`:
- Around line 266-302: The signing tests cover a missing direct KeyInfo
placeholder but not duplicate placeholders. Add a test alongside
key_info_writer_requires_direct_template_placeholder that signs a template
containing two direct ds:KeyInfo elements and asserts SigningError::XmlMutation
with ValueCountMismatch { element: "KeyInfo", expected: 2, actual: 1 }.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1b514777-04c7-48b3-9540-04e6fffc3fc1

📥 Commits

Reviewing files that changed from the base of the PR and between 5be736c and 6df8714.

📒 Files selected for processing (6)

Cargo.toml
README.md
src/xmldsig/mod.rs
src/xmldsig/mutation.rs
src/xmldsig/sign.rs
tests/signing_digest.rs

polaz · 2026-07-11T10:29:11Z

@coderabbitai review

coderabbitai · 2026-07-11T10:29:17Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

polaz added 2 commits July 11, 2026 03:53

build(deps): update p521 requirement

82c6154

feat(xmldsig): write x509 signing key info

6df8714

- Add signing KeyInfoWriter and X509 certificate writer - Populate direct Signature/KeyInfo placeholders during signing - Verify embedded certificate KeyInfo through DefaultKeyResolver Closes #80

greptile-apps Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread src/xmldsig/mutation.rs

Comment thread src/xmldsig/sign.rs

coderabbitai Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread tests/signing_digest.rs

fix(xmldsig): harden signing key info writer

b908e4a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(xmldsig): write x509 signing key info#81

feat(xmldsig): write x509 signing key info#81
polaz wants to merge 3 commits into
mainfrom
feat/#80-keyinfo-writer

polaz commented Jul 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

greptile-apps Bot commented Jul 11, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

T-Rex Logs

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

polaz commented Jul 11, 2026

Uh oh!

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

polaz commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

coderabbitai Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

greptile-apps Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

T-Rex Logs

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

polaz commented Jul 11, 2026

Uh oh!

coderabbitai Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

polaz commented Jul 11, 2026 •

edited

Loading

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

greptile-apps Bot commented Jul 11, 2026 •

edited

Loading

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading