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

[Jamil Nimeh]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180606/892d1d0c/attachment.htm>