RFR: 8326643: JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message [v5]

[John Jiang]

On Wed, 20 Mar 2024 09:55:34 GMT, Prasadrao Koppula <pkoppula at openjdk.org> wrote:

>> JDK server does not send a dummy change_cipher_spec record after HelloRetryRequest message.
>> 
>> According to RFC 8446 (Middlebox Compatibility Mode), if the client sends a non-empty session ID in the ClientHello message, the server sends a dummy change_cipher_spec (CCS) record immediately after its first handshake message. This may either be after a ServerHello or a HelloRetryRequest.
>> 
>> https://datatracker.ietf.org/doc/html/rfc8446#appendix-D.4
>
> Prasadrao Koppula has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8326643

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 71:

> 69:     private void runDemo(boolean isHRRTest) throws Exception {
> 70: 
> 71:             if(isHRRTest){

need a space between `if` and `(`.

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 113:

> 111:                 System.out.println("=========== CCS found ===========");
> 112:             }
> 113:             else{

need a space between `else` and `{`

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 192:

> 190:                                     "Test for Handshake requires only HS type");
> 191:                             } else {
> 192:                                 // Go into the first handhshake message in the

typo: handhshake -> handshake

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 253:

> 251:     }
> 252: 
> 253:     private static void log(String str) {

I suggest method `log` accepts `String...` instead of `String`.
This method can concatenate the strings, then its callers don't need to use so many `+` to concatenate the log.

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 280:

> 278:             log("===== " + header + " (" + tlsRecType(type) + " / " +
> 279:                 ver_major + "." + ver_minor + " / " +
> 280:                 bufLen + " bytes) =====");

Do you want to print `recLen` or `bufLen`? `recLen` is not used yet.

test/jdk/javax/net/ssl/TLSv13/EngineOutOfSeqCCS.java line 284:

> 282:             for (int i = 0; i < bufLen; i++) {
> 283:                 if (i != 0 && i % 16 == 0) {
> 284:                     System.out.print("\n");

Just `System.out.println()`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532439209
PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532437657
PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532258153
PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532420679
PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532357089
PR Review Comment: https://git.openjdk.org/jdk/pull/18372#discussion_r1532361801