RFR: 8371333: Optimize static initialization of SSLContextImpl classes and improve logging [v3]
Jamil Nimeh
jnimeh at openjdk.org
Mon Dec 22 05:30:01 UTC 2025
On Thu, 18 Dec 2025 12:44:30 GMT, Sean Coffey <coffeys at openjdk.org> wrote:
>> Introduce lazy static initialization logic to SSLContextImpl via use of the new LazyConstant API and improve logging output
>>
>> As per JBS comments:
>>
>> * Each subclass of AbstractTLSContext (TLSv10. TLSv11 etc) is being initialization at framework initialization time due to the getApplicableSupportedCipherSuites(..) calls made in static block. Such calls are unnecessary if the subclass isn't required. This is especially true for the default JDK configuration where TLSv10, TLSv11 protocols are disabled. I've moved logic to lazy initialization of these fields via LazyConstant
>>
>> * The debug prints output never made clear what protocol version each cipher suite was being disabled for. Improved logging there
>> * The debug prints never printed out the resulting set of enabled/allowed cipher suites
>>
>> There's efficiency gain also in having one less call to the getApplicableEnabledCipherSuites method in the scenario where customized cipher suites are not in use.
>>
>> example of new debug output:
>>
>>
>> javax.net.ssl|TRACE|30|main|2025-11-26 14:31:31.997 GMT|SSLContextImpl.java:425|Ignore disabled cipher suites for protocols:[TLSv1.3, TLSv1.2]
>> [TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDH_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256
>> TLS_ECDH_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384,TLS_ECDH_RSA_WITH_AES_256_CBC_SHA384
>> TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256,TLS_ECDH_RSA_WITH_AES_128_CBC_SHA256,TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA
>> TLS_ECDH_RSA_WITH_AES_256_CBC_SHA,TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDH_RSA_WITH_AES_128_CBC_SHA,TLS_RSA_WITH_AES_256_GCM_SHA384
>> TLS_RSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_256_CBC_SHA256,TLS_RSA_WITH_AES_128_CBC_SHA256,TLS_RSA_WITH_AES_256_CBC_SHA
>> TLS_RSA_WITH_AES_128_CBC_SHA]
>> javax.net.ssl|TRACE|30|main|2025-11-26 14:31:31.997 GMT|SSLContextImpl.java:425|Available cipher suites for protocols:[TLSv1.3, TLSv1.2]
>> [TLS_AES_256_GCM_SHA384,TLS_AES_128_GCM_SHA256,TLS_CHACHA20_POLY1305_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
>> TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
>> TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
>> TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256,TLS_DHE_DSS_WITH_AES_256_GCM_SHA384,TLS_DHE_RSA_WITH_AES_128_GCM_SHA256
>> TLS_DHE_DSS_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SH...
>
> Sean Coffey has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>
> - Merge branch 'master' into 8371333-ssl-debug
> - Move wrapText method to Utilities
> - Merge branch 'master' into 8371333-ssl-debug
> - use LINE_SEP
> - 8371333
src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java line 369:
> 367: List<ProtocolVersion> protocols) {
> 368: LinkedHashSet<CipherSuite> suites = new LinkedHashSet<>();
> 369: List<String> disabledSuites = new ArrayList<>();
Do you need to perform the instantiation here? I ask only because it looks like you're only using these two lists when debugging is turned on, so maybe instantiation can happen only in that circumstance?
src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java line 400:
> 398: }
> 399:
> 400: if(SSLLogger.isOn() && SSLLogger.isOn("ssl,sslctx")) {
Nit: space between if and parenthesis.
src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java line 401:
> 399:
> 400: if(SSLLogger.isOn() && SSLLogger.isOn("ssl,sslctx")) {
> 401: logSuites("Ignore disabled cipher suites for protocols:",
Nit: just a personal preference - I wouldn't mind seeing a space after the colons. But no strong feelings on this one.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28511#discussion_r2638717667
PR Review Comment: https://git.openjdk.org/jdk/pull/28511#discussion_r2638718651
PR Review Comment: https://git.openjdk.org/jdk/pull/28511#discussion_r2638720100
More information about the security-dev
mailing list