RFR: 8280494: (D)TLS signature schemes [v19]

[Xue-Lei Andrew Fan]

On Tue, 8 Mar 2022 16:13:08 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add test for DTLS
>
> src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 501:
> 
>> 499: 
>> 500:             // Note that if the System Property value is not defined (JDK
>> 501:             // default value) or empty, the provider-specific default is used.
> 
> I think you can remove this comment as it is repeated on lines 507-508 (and makes more sense there).

I agreed.

> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 564:
> 
>> 562:                 String[] signatureSchemes) {
>> 563: 
>> 564:         if (signatureSchemes == null ||  signatureSchemes.length == 0) {
> 
> Nit: remove extra space after `||`.

Thanks for the catch!

> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 576:
> 
>> 574:                     SSLLogger.finest(
>> 575:                             "Ignore the signature algorithm (" + ss
>> 576:                           + "), unsupported or unavailable");
> 
> Should this message be more consistent with the message in `getCustomizedSignatureScheme`?: "The current installed providers do not support signature scheme: " + schemeName

The usage of the method is a little bit different.  This method is mainly used for selecting a few from many signature schemes, and it is usually log with "ignore ..." for those does not match but with high preference.  See also the caller method, getSupportedAlgorithms(), between lines 389-405.

> test/jdk/javax/net/ssl/DTLS/DTLSSignatureSchemes.java line 125:
> 
>> 123:             testCase.runTest(testCase);
>> 124:             if (exceptionExpected) {
>> 125:                 throw new RuntimeException("Unexpected success!");
> 
> The catch block on line 127 will end up catching this exception and swallowing it, and the test will incorrectly pass.

Good catch!

> test/jdk/javax/net/ssl/SSLParameters/SignatureSchemes.java line 81:
> 
>> 79:             super.runClientApplication(sslSocket);
>> 80:             if (exceptionExpected) {
>> 81:                 throw new RuntimeException("Unexpected success!");
> 
> The catch block on line 83 will end up catching this exception and swallowing it, and the test will incorrectly pass.

Good catch!

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

PR: https://git.openjdk.java.net/jdk/pull/7252