On Thu, 18 Aug 2022 14:05:38 GMT, Weibing Xiao <[email protected]> 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
