Code Review Request: TLS 1.3 Implementation
Xuelei Fan
xuelei.fan at oracle.com
Mon Jun 4 15:50:12 UTC 2018
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
More information about the security-dev
mailing list