Code Review Request: TLS 1.3 full handshake (JDK-8196584)

[Xuelei Fan]

Hi Jamil,

All good catches to me.  Would you mind take care of the update as well?

Thanks,
Xuelei

On 6/6/2018 3:19 PM, Jamil Nimeh wrote:
> Hello, comments below, minor stuff for the most part:
> 
>   * KeyUpdate.java
>       o 128-129: Spelling nit: REQUSTED --> REQUESTED
>       o That change will have to get sprinkled throughout the code
>   * OutputRecord.java
>       o 291-295: It seems like using an absolute put would simplify this
>         code a bit:  Just advance the destination limit by one like
>         you're already doing, then destination.put(destination.limit() -
>         1, contentType);  Or you can set an int variable to
>         destination.limit() before you make the limit one larger and
>         then use that as the first argument to the absolute put.  Either
>         way you don't need to move your position marker forward and
>         backward.
>           + I know absolute puts are not guaranteed to be implemented,
>             but they are used further down on line 312, 317-318 so I
>             think we're safe here.
>   * SSLEngineOutputRecord.java
>       o Looks good to me
>   * SSLSocketOutputRecord.java
>       o 315: REQUSTED -> REQUESTED
>       o Looks okay otherwise
>   * DTLSOutputRecord
>       o 68: Do we need to hang onto this for now, or can we remove this
>         commented out line?
>       o Otherwise looks OK.
> 
> More on the way...
> 
> --Jamil
> 
> 
> On 02/22/2018 12:29 PM, Xuelei Fan wrote:
>> Updated to use package private HKDF implementation.
>>
>> webrev (based on JDK-8185576):
>> http://cr.openjdk.java.net/~xuelei/8196584/webrev-step.01
>>
>> webrev (including JDK-8185576):
>> http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01
>>
>> Thanks,
>> Xuelei
>>
>> On 2/20/2018 11:57 AM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> I'd like to invite you to review the TLS 1.3 full handshake 
>>> implementation.  I appreciate it if I could have feedback before 
>>> March 9, 2018.
>>>
>>> In the "JDK-8185576: New handshake implementation" [1] code review 
>>> around, I was trying to re-org the TLS handshaking implementation in the
>>> SunJSSE provider.  If you had reviewed that part, you can start from 
>>> the following webrev that based on the update of JDK-8185576:
>>> http://cr.openjdk.java.net/~xuelei/8196584/webrev-step.00
>>>
>>> If you would like start from earlier, here is the webrev that 
>>> contains the handshaking implementation re-org in JDK-8185576:
>>> http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.00
>>>
>>>
>>> This changeset only implements the full handshake of TLS 1.3, rather 
>>> then a fully implementation of the latest TLS 1.3 draft [2].
>>>
>>> In this implementation, I removed:
>>> 1. the KRB5 cipher suite implementation.
>>> Please let me know if you are still using KRB5 cipher suite.  I may 
>>> not add them back if no objections.
>>>
>>> 2. OCSP stapling.
>>> This feature will be added back later.
>>>
>>> Resumption and key update, and more features may be added later.
>>>
>>> Thanks & Regards,
>>> Xuelei
>>>
>>> [1]: 
>>> http://mail.openjdk.java.net/pipermail/security-dev/2017-December/016642.html 
>>>
>>> [2]: https://tools.ietf.org/html/draft-ietf-tls-tls13-24
>