Code Review Request: TLS 1.3 Implementation

[Xuelei Fan]

 > http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01

NewSessionTicket.java
---------------------
  184   if (shc.pskKeyExchangeModes.isEmpty()) {
Need additional check of the requested mode.


  215   if (sessionTimeoutSeconds > SEVEN_DAYS_IN_SECONDS) {
  216       if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
  217         SSLLogger.fine(
  218          "Session timeout is too long. No NewSessionTicket sent.");
  219       }
  220       return null;
  221   }
It does not sound right to me that if the session timeout is too long, 
log and silent return for the producing side.  It may involve more wider 
scope of session time.  I need time to think more about it.


RenegoInfoExtension.java
------------------------
Looks fine to me.


RSAClientKeyExchange.java
-------------------------
   76  // This happens in server side only.
   77  ServerHandshakeContext shc =
   78         (ServerHandshakeContext)handshakeContext;
The 'context' can be used directly where shc is used in the method.  The 
above lines are not strictly necessary.


SSLExtension.java
-----------------
  441     // TLS 1.3 PSK extension must be last
  442     CH_PRE_SHARED_KEY       (0x0029, "pre_shared_key",
          ...
  449                                 null, null),
  450     SH_PRE_SHARED_KEY       (0x0029, "pre_shared_key",
          ...
  456                                 null, null, null);
No SSLStringize defined for the above two extensions.  Nice to have them 
for debug logging.


SessionId.java
--------------
Looks fine to me.  A minor comment, checking session id length after the 
construction looks like friable unless coding carefully.


StatusResponseManager.java
--------------------------
Looks fine to me.

Thanks,
Xuelei


On 6/3/2018 9:43 PM, Xuelei Fan wrote:
> Hi,
> 
> Here it the 2nd full webrev:
>    http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01
> 
> and the delta update to the 1st webrev:
>    http://cr.openjdk.java.net/~xuelei/8196584/webrev-delta.00/
> 
> Xuelei
> 
> On 5/25/2018 4:45 PM, Xuelei Fan wrote:
>> Hi,
>>
>> I'd like to invite you to review the TLS 1.3 implementation.  I 
>> appreciate it if I could have compatibility and specification feedback 
>> before May 31, 2018, and implementation feedback before June 7, 2018.
>>
>> Here is the webrev:
>>      http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.00
>>
>> The formal TLS 1.3 specification is not finalized yet, although it had 
>> been approved to be a standard.  The implementation is based on the 
>> draft version 28:
>>      https://tools.ietf.org/html/draft-ietf-tls-tls13-28
>>
>> For the overall description of this enhancement, please refer to JEP 332:
>>      http://openjdk.java.net/jeps/332
>>
>> For the compatibility and specification update, please refer to CSR 
>> 8202625:
>>      https://bugs.openjdk.java.net/browse/JDK-8202625
>>
>> Note that we are using the sandbox for the development right now.  For 
>> more information, please refer to Bradford's previous email:
>>
>> http://mail.openjdk.java.net/pipermail/security-dev/2018-May/017139.html
>>
>> Thanks & Regards,
>> Xuelei