RFR: 8133816: Display extra SSLServerSocket info in debug mode [v2]

Sean Mullan mullan at openjdk.org
Mon Aug 22 13:02:55 UTC 2022


On Thu, 18 Aug 2022 14:05:38 GMT, Weibing Xiao <duke at openjdk.org> wrote:

>> Log the debugging info for server cipher suites when setting javax.net.debug == ssl, handshake.
>
> Weibing Xiao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add or remove the blank line according to the comments

src/java.base/share/classes/sun/security/ssl/ServerHello.java line 475:

> 473:             }
> 474: 
> 475:             //negotiation failed between client and server, print server enabled cipher suites

add space after `//`

src/java.base/share/classes/sun/security/ssl/ServerHello.java line 775:

> 773:         LinkedList<String> fieldsList = new LinkedList<>();
> 774: 
> 775:         fieldsList.add("SSLSeverSocket info");

Typo: SSLServerSocket

src/java.base/share/classes/sun/security/ssl/ServerHello.java line 778:

> 776:         fieldsList.add(shc.sslConfig.preferLocalCipherSuites ? "Server preference" : "Client preference");
> 777:         fieldsList.add(shc.sslConfig.clientAuthType.toString());
> 778:         fieldsList.add(shc.activeCipherSuites != null ? shc.activeCipherSuites.toString() : "Not Set");

Why is this capitalized? "not set" seems better.

src/java.base/share/classes/sun/security/ssl/ServerHello.java line 781:

> 779:         fieldsList.add(Security.getProperty(LegacyAlgorithmConstraints.PROPERTY_TLS_LEGACY_ALGS));
> 780: 
> 781:         if (!shc.negotiatedProtocol.name.equalsIgnoreCase(ProtocolVersion.TLS13.name)) {

This check is somewhat incorrect in my opinion, just because none of the legacy algs don't apply to TLS 1.3 doesn't mean that we won't add one in the future or someone could configure it themselves. It feels like this is the wrong place to be logging about legacy/disabled suites and this should be logged only if the constraints have been applied. I would remove the logging of the legacy algs as it doesn't seem critical. Plus it doesn't make sense to only log legacy algs and not disabled algs.

-------------

PR: https://git.openjdk.org/jdk/pull/9731



More information about the security-dev mailing list