feat(pkcs11): add opaque external keys - #186
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou'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. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (11)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdds 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. ChangesPKCS#11 opaque-key support
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
Merge Risk: ⚪ Minimal · up to 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 ReviewSecurity architecture risk: 🔵 Low · up to 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
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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 💡
🧪 Generate unit tests (beta)
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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".
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.
b1d4334 to
4242132
Compare
Summary
cryptokiPKCS#11 adapter selected explicitly throughCryptoProvider, with a read-only token session, opaque RSA/AES/EC handles, exact provider binding, key usage enforcement and typed redacted failures.Validation
Boundaries
Closes #185
Summary by CodeRabbit