On Mon, 22 Aug 2022 12:52:54 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> 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 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. Looks like the TLS Legacy algs and the disabled cipher suites values are final static. I think we should remove them from this patch and open a new JBS enhancement. The new issue can log these two final values when set by the JDK -- useful values to know for debug logs. ------------- PR: https://git.openjdk.org/jdk/pull/9731