Use JDK cryptography for HTTP TLS identities - #91
Conversation
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🔇 Additional comments (5)
📝 WalkthroughWalkthroughThe change removes Bouncy Castle dependencies and replaces certificate creation with a JDK-based generator. TLS identity checks now use numeric X.509 identifiers and the generator’s IP-literal detection. Tests cover certificate-name validation and confirm that packaged artifacts omit Bouncy Castle. ChangesJDK TLS certificate generation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant HttpTlsIdentity
participant JdkX509CertificateGenerator
participant JCA_Signature
participant X509_CertificateFactory
HttpTlsIdentity->>JdkX509CertificateGenerator: Pass certificate role, SAN, and issuance time
JdkX509CertificateGenerator->>JCA_Signature: Sign encoded certificate data
JCA_Signature-->>JdkX509CertificateGenerator: Return signature
JdkX509CertificateGenerator->>X509_CertificateFactory: Parse encoded certificate
X509_CertificateFactory-->>JdkX509CertificateGenerator: Return X.509 certificate
JdkX509CertificateGenerator-->>HttpTlsIdentity: Return certificate
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the change is ready for normal merge checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1fcd0e8cd
ℹ️ 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".
| if (value == null || value.isEmpty()) return false; | ||
| if (value.indexOf(':') >= 0) { | ||
| if (!value.matches("[0-9A-Fa-f:.]+")) return false; | ||
| try { return InetAddress.getByName(value).getAddress().length == 16; } |
There was a problem hiding this comment.
When advertisedHost is a valid IPv4-mapped IPv6 literal such as ::ffff:192.0.2.1 or ::ffff:c000:201, InetAddress.getByName returns a four-byte Inet4Address, so this check returns false and create subsequently throws because the value contains a colon. The previous Bouncy Castle predicate accepted and encoded these literals, so existing deployments using them can no longer load or create their TLS identity; parse these valid mapped forms as IPv6 rather than relying on the returned address length.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
1 participant
Summary
Motivation
VotingPlugin embeds SimpleAPI. Bouncy Castle contributes roughly 7.7 MB compressed to the downstream plugin even though the HTTP transport needs only EC certificate issuance. JDK 21 already supplies EC key generation, ECDSA signing, certificate parsing, PKCS#12, and TLS.
Compatibility
sun.*APIs or module export flags are usedValidation
mvn -B -f SimpleAPI/pom.xml clean package: passedgit diff --check: passedDownstream
VotingPlugin PR #1620 uses this change to remove the external crypto provider from its downloadable JAR.
Summary by CodeRabbit