Return full peer certificate chain from SSLSession.getPeerCertificates() - #390
Return full peer certificate chain from SSLSession.getPeerCertificates()#390cconlon wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates wolfSSL’s JSSE/JNI integration so SSLSession.getPeerCertificates() can return the full peer-sent certificate chain (leaf first), aligning behavior with the JSSE spec and enabling certificate pinning against intermediates/roots when native SESSION_CERTS support is available. It introduces new JNI/native plumbing to fetch the peer chain via wolfSSL_get_peer_chain() and adjusts session/certificate handling (including resumed sessions and oversized peer cert edge cases), with accompanying tests.
Changes:
- Add native/JNI + Java APIs to retrieve the peer certificate chain in DER form and expose a native
SESSION_CERTSfeature-detect (WolfSSL.sessionCertsEnabled()). - Rework JSSE
SSLSessionpeer-certificate retrieval to use DER-chain conversion, cache results for session resumption, and support deprecatedgetPeerCertificateChain()while enabling explicit native cleanup (WolfSSLX509X.free()). - Add tests covering full-chain behavior, resumption behavior, and the “large peer cert skipped by native chain storage” scenario.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/com/wolfssl/test/WolfSSLTest.java | Adds a feature-detect reachability test for WolfSSL.sessionCertsEnabled(). |
| src/test/com/wolfssl/test/WolfSSLSessionTest.java | Adds a JNI-level test for WolfSSLSession.getPeerCertificateChainDER() / getPeerCertificateDER(). |
| src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java | Adds helper to generate a large-leaf cert chain for JSSE tests. |
| src/test/com/wolfssl/provider/jsse/test/WolfSSLSessionTest.java | Adds JSSE tests validating full-chain return, resumption behavior, and large-peer-cert ordering. |
| src/java/com/wolfssl/WolfSSLSession.java | Adds DER-based peer cert + chain retrieval and “ensure peer cert first” logic. |
| src/java/com/wolfssl/WolfSSL.java | Adds sessionCertsEnabled() native feature-detect API. |
| src/java/com/wolfssl/provider/jsse/WolfSSLX509X.java | Adds free() method to explicitly release native resources for deprecated cert objects. |
| src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java | Switches peer-certificate retrieval to DER-chain conversion/caching and updates deprecated chain API to return full chain. |
| native/com_wolfssl_WolfSSLSession.h | Declares JNI entrypoint for getPeerCertificateChainDER. |
| native/com_wolfssl_WolfSSLSession.c | Implements JNI getPeerCertificateChainDER using wolfSSL_get_peer_chain(). |
| native/com_wolfssl_WolfSSL.h | Declares JNI entrypoint for sessionCertsEnabled. |
| native/com_wolfssl_WolfSSL.c | Implements JNI sessionCertsEnabled via #ifdef SESSION_CERTS. |
Files not reviewed (2)
- native/com_wolfssl_WolfSSL.h: Generated file
- native/com_wolfssl_WolfSSLSession.h: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #390
Scan targets checked: wolfssljni-bugs, wolfssljni-src
Findings: 2
1 finding(s) posted as inline comments (see file-level comments below)
Low (1)
WolfSSLX509X(long, boolean) constructor now unused
File: src/java/com/wolfssl/provider/jsse/WolfSSLX509X.java:88
Function: WolfSSLX509X
Category: Dead code
This constructor's only caller was the old pointer-based getPeerCertificateChain() implementation, which this PR replaced with a DER-byte-based conversion using the byte[] constructor. No code in src or test now calls WolfSSLX509X(long, boolean).
Recommendation: Remove the now-unused constructor, or confirm it is kept intentionally for external API consumers.
Referenced code: src/java/com/wolfssl/provider/jsse/WolfSSLX509X.java:88-91 (4 lines)
This review was generated automatically by Fenrir. Findings are non-blocking.
Prior to this PR,
SSLSession.getPeerCertificates()returned only the leaf peer certificate. The JSSE spec notes this should return the full chain the peer sent, leaf first. This breaks certificate pinning, where pinning to an intermediate or root is the recommended practice, and was reported in PR #256.This PR adds
WolfSSLSession.getPeerCertificateChainDER()andgetPeerCertificateDER()over nativewolfSSL_get_peer_chain(), and reworksgetPeerCertificates()and the deprecatedgetPeerCertificateChain()to use them. Also addsWolfSSL.sessionCertsEnabled()andWolfSSLX509X.free().SESSION_CERTS, enabled by--enable-jni. Without it, behavior degrades to the previous leaf-only result.MAX_X509_SIZEor larger when storing the chain, including the peer cert. The separateKEEP_PEER_CERTcopy is restored to index zero so the peer certificate is always first.