Repository navigation
Support JSSE jdk.tls.allowLegacyMasterSecret to require TLS Extended Master Secret - #448
Conversation
|
@claude review |
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive EMS enforcement relies on native wolfSSL code absent from this snapshot, leaving fail-closed handshake behavior and compatibility across supported, older, and EMS-disabled builds unverified.
No open issues 🎉
1 file not reviewed
native/com_wolfssl_WolfSSLSession.h: Generated file
What changed in this PR
Adds wolfJSSE support for jdk.tls.allowLegacyMasterSecret, allowing applications to require Extended Master Secret (EMS) for TLS 1.2 and earlier.
Changes:
- Adds a Java/JNI wrapper for requiring EMS with native wolfSSL 5.9.4+.
- Validates the property and fails closed when EMS cannot be required, while preserving TLS 1.3 and explicitly disabled EMS behavior.
- Documents the property and adds native-wrapper and JSSE handshake tests.
| File | Description |
|---|---|
src/test/com/wolfssl/test/WolfSSLSessionTest.java |
Tests native support detection and calls after session cleanup. |
src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java |
Captures handshake exceptions for test assertions. |
src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java |
Tests EMS enforcement, unsupported configurations, and property validation. |
src/java/com/wolfssl/WolfSSLSession.java |
Exposes requireExtendedMasterSecret(). |
src/java/com/wolfssl/provider/jsse/WolfSSLUtil.java |
Parses and validates the new system property. |
src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java |
Applies EMS requirements during session setup. |
README.md |
Documents property behavior and native requirements. |
native/com_wolfssl_WolfSSLSession.h |
Declares the new JNI entry point. |
native/com_wolfssl_WolfSSLSession.c |
Wraps native EMS enforcement with capability guards. |
Review effort: Balanced
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and found no bugs. It changes when TLS handshakes fail during security negotiation, so a human should still look at it before merge.
What was reviewed:
- I followed the JNI path from
requireExtendedMasterSecret()to the nativewolfSSL_RequireExtendedMasterSecret(). The version and feature checks return NOT_COMPILED_IN when support is missing. - In
setLocalExtendedMasterSecret(), a "false" value is ignored when EMS is disabled. If requiring EMS fails, setup throwsSSLExceptionunless only (D)TLS 1.3 can be negotiated. setLocalParams()callsapplyConfiguredCipherProtocolSettings()beforesetLocalExtendedMasterSecret(). That meanseffectiveProtocolsis already set whenisPreTLS13Enabled()reads it, and the method assumes older protocols are possible if the list is null.- I did not run the build or the JUnit tests.
Extended reasoning...
The change adds a JNI wrapper and native function for requiring TLS Extended Master Secret (RFC 7627). It also adds wolfJSSE handling of the jdk.tls.allowLegacyMasterSecret property, with new tests and README text. This touches TLS handshake security: when a TLS 1.2 or earlier peer does not negotiate EMS, the handshake now fails instead of falling back to a legacy master secret. I deferred because this is crypto and protocol security behavior, and the fail-closed rules depend on the native wolfSSL version and build flags. The repository has no CODEOWNERS file, and the timeline shows no outstanding objections.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #448
Scan targets checked: wolfssljni-src, wolfssljni-bugs
Not reviewed (no scan target covers them): README.md
Coverage: 6 of 8 in-scope changed file(s) opened by the reviewer; not opened: src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java, src/test/com/wolfssl/test/WolfSSLSessionTest.java
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
Adds support for the SunJSSE
jdk.tls.allowLegacyMasterSecretSystem property. When set to"false", TLS 1.2 and earlier handshakes that do not negotiate Extended Master Secret (RFC 7627) fail instead of falling back to a legacy master secret.WolfSSLSession.requireExtendedMasterSecret(), wrapping nativewolfSSL_RequireExtendedMasterSecret()(wolfSSL 5.9.4+)"false". TLS 1.3 is not affected, and the property is ignored whenjdk.tls.useExtendedMasterSecret=false.SSLExceptionwhen a protocol below TLS 1.3 can be negotiated