On 6/4/2018 8:36 AM, Anthony Scarpino wrote:
Here are questions I have about the code before I or someone makes a
change:
ChangeCipherSpec.java
- 69 & 200: Given this is legacy data on older version of TLS, is the
exception better worded "Not supported"? "yet" implies future additions.
I agreed. Would you mind update it in one of your changeset as well?
- ChangeCipherSpec is removed in 1.3, why is there a T13 consumer ?
For compatibility reason, ChangeCipherSpec was back in TLS 1.3 in the
recent drafts. But it is used for compatibility purposes only. See
Section 5 of draft 28:
An implementation may receive an unencrypted record of type
change_cipher_spec consisting of the single byte value 0x01 at any
time after the first ClientHello message has been sent or received
and before the peer's Finished message has been received and MUST
simply drop it without further processing.
As JDK is using the compatibility mode, so ChangeCipherSpec has a T13
consumer.
Client/ServerHello are consuming these messages too, shouldn't their
references be removed as well?
See above.
Below are some changes I will make:
Alert.java
- optimized imports.
Authenticator.java
- 271-2: Clarifying comments about byte array ordering. Moved sequence
number to the end
- 406: static declaration for the interface is redundant
ClientHandshakeContext:
- 3 spelling errors
Thanks!
Xuelei
