On Mon, 22 Aug 2022 12:52:54 GMT, Sean Mullan <[email protected]> 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