RFR: 8133816: Display extra SSLServerSocket info in debug mode [v2]

[Weibing Xiao]

On Wed, 17 Aug 2022 14:27:56 GMT, Xue-Lei Andrew Fan <xuelei at 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 420:
> 
>> 418:                 if (!HandshakeContext.isNegotiable(
>> 419:                         proposed, shc.negotiatedProtocol, cs)) {
>> 420:                     continue;
> 
> I may add a debug log that the cipher suite is not negotiable her for the protocol.

If there is no common cipher suite between the client and server. All of the server cipher suites will be printed out. The error message of "no common cipher suites" will be logged. If any cipher suites are shared between the client and server, the rest of the cipher suites would not be used. For this case, the error message of "key exchange failed" will be logged.

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 438:
> 
>> 436: 
>> 437:                 if (ke == null) {
>> 438:                     continue;
> 
> I may add a debug log here that the key exchange is not good for the cipher suite and protocol.

As mentioned in my previous reply, if handshaking failed, all of the cipher suites will be logged. Technically key name is  included in cipher suite name.

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 447:
> 
>> 445:                 }
>> 446: 
>> 447:                 SSLPossession[] hcds = ke.createPossessions(shc);
> 
> I may not remove this blank line.

Updated accordingly

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 449:
> 
>> 447:                 SSLPossession[] hcds = ke.createPossessions(shc);
>> 448:                 if ((hcds == null) || (hcds.length == 0)) {
>> 449:                     continue;
> 
> I may add a debug log here that the cipher suite is legacy.

It is logged in the new method.

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 452:
> 
>> 450:                 }
>> 451:                 // The cipher suite has been negotiated.
>> 452:                 if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
> 
> I may not remove this blank line.

Updated accordingly

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 461:
> 
>> 459:                 SSLKeyExchange ke = SSLKeyExchange.valueOf(
>> 460:                         cs.keyExchange,  shc.negotiatedProtocol);
>> 461: 
> 
> I may not add this extra line.

Updated accordingly

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 472:
> 
>> 470: 
>> 471:             throw shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
>> 472:                     "no cipher suites in common");
> 
> As there are detailed negotiation debug log, I may just update this line from "no cipher suites in common" to "no cipher suites or key exchange algorithms in common"

See my previous reply.

> src/java.base/share/classes/sun/security/ssl/ServerHello.java line 757:
> 
>> 755:             if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
>> 756:                 printServerSocketConfig(shc, null);
>> 757:             }
> 
> Similarly, I may break down the debug log closer to the actions.

See my previous reply.

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

PR: https://git.openjdk.org/jdk/pull/9731