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

[Jamil Nimeh]

Yes, I'll delete that commented line and rework the absolute put later 
tonight.

--Jamil


On 06/06/2018 05:44 PM, Xuelei Fan wrote:
> 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
>>