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

Reply via email to