Skip to content

feat(pkcs11): add opaque external keys - #186

Merged
polaz merged 2 commits into
mainfrom
feat/#185-pkcs11
Oct 5, 2026
Merged

polaz merged 2 commits into
mainfrom
feat/#185-pkcs11

Conversation

@polaz

@polaz polaz commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Add an optional cryptoki PKCS#11 adapter selected explicitly through CryptoProvider, with a read-only token session, opaque RSA/AES/EC handles, exact provider binding, key usage enforcement and typed redacted failures.
  • Wire token signing, verification, RSA-OAEP recovery, AES content decryption/key unwrap and ECDH into the XML signature and encryption pipelines without software-provider fallback.
  • Add an isolated SoftHSM harness, XML-backend CI matrix and documentation covering module lifecycle, supported mechanisms and limitations.

Validation

  • All-feature workspace: 4039 tests passed, 0 skipped.
  • SoftHSM integration passed with xmloxide, roxmltree and differential configurations; covers wrong credentials, unavailable parameters, usage denial, stale/ambiguous objects, provider binding and concurrency.
  • All-feature workspace build, Clippy and formatting passed.
  • Rust 1.92 test compilation and alloc-only host/bare-metal checks passed.
  • Workspace doctests passed; one existing ML-DSA example remains ignored.

Boundaries

  • Tested with SoftHSM 2.6.1 on Linux and 2.7 on macOS, not a physical HSM; no production token was initialized. Invalid GCM releases no plaintext, and its typed failure preserves the native module's distinction between invalid ciphertext and generic operational failure.
  • Private RSA/EC material stays in the token. RSA-OAEP recovered symmetric bytes transiently enter zeroized host memory before import as a non-extractable session key; AES-unwrapped content keys remain token objects.
  • Module, slot, authentication and key selection are explicit library API context; no CLI credential discovery or implicit provider fallback.

Closes #185

Summary by CodeRabbit

  • New Features
    • Added optional PKCS#11 support for using external cryptographic tokens with XML signing, verification, encryption, decryption, key wrapping, and key agreement. Private keys can remain non-exportable.
    • Added opaque-key adapters for content decryption and key unwrapping.
  • Documentation
    • Expanded the crypto-provider guide with PKCS#11 setup, supported operations, and limitations.
  • Tests
    • Added PKCS#11 integration coverage and automated CI checks using SoftHSM.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: structured-world/xml-sec/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5cfb7503-456a-40eb-b2c7-9dae7899b234
📥 Commits

Reviewing files that changed from the base of the PR and between b1d4334 and 4242132.

📒 Files selected for processing (11)
  • .github/workflows/ci.yml
  • Cargo.toml
  • README.md
  • docs/crypto-providers.md
  • scripts/test-pkcs11.sh
  • src/provider.rs
  • src/provider/pkcs11.rs
  • src/xmldsig/verify.rs
  • src/xmlenc/decrypt.rs
  • src/xmlenc/mod.rs
  • tests/pkcs11.rs

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: f23f8b08-8e02-43bf-ada3-de046dd86cbd
📥 Commits

Reviewing files that changed from the base of the PR and between 6b8b5be and b1d4334.

📒 Files selected for processing (11)
  • .github/workflows/ci.yml
  • Cargo.toml
  • README.md
  • docs/crypto-providers.md
  • scripts/test-pkcs11.sh
  • src/provider.rs
  • src/provider/pkcs11.rs
  • src/xmldsig/verify.rs
  • src/xmlenc/decrypt.rs
  • src/xmlenc/mod.rs
  • tests/pkcs11.rs

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

Adds an optional PKCS#11 provider for token-backed cryptographic operations. Opaque keys can be used through XML encryption flows without exporting key material. The change also adds provider-binding checks, SoftHSM integration tests, documentation, and CI coverage.

Changes

PKCS#11 opaque-key support

Layer / File(s) Summary
Opaque-key contracts and provider binding
src/provider.rs, src/xmldsig/verify.rs
Adds opaque recovered-key handles, typed external-provider errors, provider-binding accessors, and hooks for key recovery, unwrap, and decryption. Verification keys forward provider bindings, and key-agreement dispatch checks bindings before use.
PKCS#11 session and key resolution
Cargo.toml, src/provider.rs, src/provider/pkcs11.rs
Adds the optional pkcs11 feature and provider module. The adapter supports explicit module and slot setup, login, persistent RSA, AES, and EC key lookup, mechanism checks, and typed error mapping.
Token-backed cryptographic operations
src/provider/pkcs11.rs
Implements token-backed RSA operations, AES decryption and unwrap, random generation, digest, and ECDH. Temporary AES keys are sensitive, non-extractable session objects, and their host key copies are zeroized.
Opaque keys in XML encryption
src/xmlenc/decrypt.rs, src/xmlenc/mod.rs
Adds opaque content-key and KEK resolvers. XML decryption passes recovered-key handles through OAEP recovery, key-length validation, candidate deduplication, and content decryption.
Integration, documentation, and CI
tests/pkcs11.rs, scripts/test-pkcs11.sh, .github/workflows/ci.yml, docs/crypto-providers.md, README.md
Adds isolated SoftHSM integration tests and a test runner. Documents setup, operations, and limitations. CI tests the feature across three XML backends and includes the PKCS#11 job in the aggregate test job.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant XMLDecryption
  participant OpaqueKekDecryptor
  participant Pkcs11Provider
  participant PKCS11Token
  XMLDecryption->>OpaqueKekDecryptor: resolve wrapped content key
  OpaqueKekDecryptor->>Pkcs11Provider: unwrap content key
  Pkcs11Provider->>PKCS11Token: token-side key unwrap
  PKCS11Token-->>Pkcs11Provider: opaque session key
  Pkcs11Provider-->>XMLDecryption: opaque recovered-key handle
  XMLDecryption->>Pkcs11Provider: decrypt content through key handle
  Pkcs11Provider->>PKCS11Token: token-backed content decryption
Loading

Merge Risk: ⚪ Minimal · up to b1d43

This change adds an optional PKCS#11 provider that keeps keys on the token, with typed errors and SoftHSM-backed tests required in CI. No concrete merge-blocking defect was identified in the reviewed changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b1d43

Explicit provider selection, identity checks, and key-use restrictions limit exposure. However, failed cleanup can leave derived shared secrets alive longer than intended. Validation covers a software token rather than production devices.

Retained concerns

  • Low · security · inferred: If destruction fails after ECDH derivation, an extractable, non-sensitive shared-secret session object can remain alive. The function returns an error and discards the object handle, but the shared provider session is not retired and no cleanup owner is retained. This extends secret lifetime across later operations. Normal cleanup and host-value zeroization reduce exposure; disclosure additionally requires access to the application’s token-session environment, and no remote extraction path was demonstrated.
Security review details

Security Blast Radius

  • inferred — Effective authority is bounded by the application-selected token domain, retained key objects, operation policy, and token permissions. Untrusted XML reaches cryptographic operations through that context rather than choosing a module or PIN. Independent deployment or tenant isolation cannot be established from the supplied production context.

Security Findings and Attack Paths

  • observed — The supplied opaque-wrap mismatch candidate remains rejected for the built-in PKCS#11 path: unwrap capability accepts the AES family, the key implementation checks KEK width, and unsupported content families are rejected. This conclusion does not establish complete coverage of arbitrary caller-implemented providers.

Trust Boundaries and Controls

  • observed — PKCS#11 signing and both RSA recovery hooks enforce exact binding inside the key implementation before token execution. Opaque decryption, unwrap, verification, and agreement also check identity at provider dispatch. Delegation without a trait-level binding accessor therefore does not demonstrate a bypass for these built-in keys.

Resilience and Maintainability Implications

  • observed — ECDH attempts destruction even when attribute retrieval fails and zeroizes a retrieved value when destruction fails. Nevertheless, failed destruction has no retained cleanup state or session-retirement transition. Session-scoped lifetime and the absence of a raw session in the provider’s public interface constrain, but do not eliminate, residual-secret exposure.

Hardening Proposals

  • proposed — Give derived-secret objects explicit cleanup ownership. On destruction failure, retain cleanup responsibility or retire the execution domain rather than continuing with an abandoned exportable object; validate this transition with injected read and destruction failures.
  • proposed — Define the supported token-mutation lifecycle and validate identity behavior during key rotation and handle reuse on target modules. Where reuse is possible, revalidate policy-relevant identity or invalidate retained handles. Current evidence does not demonstrate a wrong-key execution attack.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 112 functions across 7 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed [#185] The PR adds an optional, explicitly selected PKCS#11 provider with opaque RSA, AES, and EC handles. The change summary reports token-backed signing, verification, key unwrap, content decryption…
Out of Scope Changes check ✅ Passed The summarized changes all support [#185]. The CI job, SoftHSM harness, integration tests, provider-contract changes, XML pipeline changes, and documentation implement or validate the requested adapte…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding PKCS#11 support for opaque external keys.
Full details: Docstring Coverage

Explanation

Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 112 functions across 7 files. (4 skipped: 4 unsupported.)

✨ 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

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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-05T11:34:44.036294Z 4242132 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1d43344d6

ℹ️ 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".

Comment thread src/provider/pkcs11.rs Outdated
polaz added 2 commits October 5, 2026 14:07
Route token signatures, verification, key recovery, content decryption, unwrap and ECDH through CryptoProvider with exact provider binding and key usage enforcement.

Add isolated SoftHSM integration coverage, XML backend CI matrix and documented module lifecycle and supported-mechanism boundaries.

Closes #185
Keep token operations usable without persistent write access. Verify corrupted GCM against exact native errors on SoftHSM 2.6 and 2.7 without treating generic failures as authentication evidence.
@polaz
polaz force-pushed the feat/#185-pkcs11 branch from b1d4334 to 4242132 Compare October 5, 2026 11:24
@polaz
polaz merged commit d48bc38 into main Oct 5, 2026
35 checks passed
@polaz
polaz deleted the feat/#185-pkcs11 branch October 5, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support opaque PKCS#11 keys

1 participant