Code Review Request: TLS 1.3 Implementation

[Bradford Wetmore]

Also reviewed SSLSocketFactoryImpl.java


BaseSSLSocketImpl.java
----------------------
I don't see where requireCloseNotify is actually being used in the 
remaining code.  If you are removing this functionality, you should 
probably be calling that out in your CSR.

SSLSessionContextImpl.java
--------------------------
The only change here was to reorder the import of Vector (obsolete) and 
update the copyright?  May I ask why?  If this was somehow a netbeans 
suggestion, there were several other things that could be updated that 
seem more important.  e.g.  37-39 could be made final.  207: 
unnecessary unboxing.

Brad




On 6/6/2018 5:45 PM, Bradford Wetmore wrote:
> Today, I looked over SunJSSE.java, Utilities.java, and module-info.java. 
>   Mostly nits below, some style things.
> 
> 
> Utilities.java
> --------------
> 
> 39:  hexDigits can be private.
> 
> 42:  Extra level of indent.  4 or 8.
> 
> 39-41:  Nits: When variables are static finals, they are usually written 
> uppercase.
> 
> 115:  Extra " " and can we get SNI cap'd?
> 
> HostName for  server name indication
> ->
> HostName for Server Name Indication
> 
> 150:  Minor Nit:  You're concating ("+") and then using 
> StringBuilder.append(), instead of simply append chaining which might be 
> slightly more efficient.
> 
> 167-217:  The code looks ok, but this might be reworked to take better 
> advantage of StringBuilder.append() chaining, rather than the mix of 
> concat and append here now.  e.g. what you did with toHexString(long). I 
> see much of the same code repeated over and over here.  Can be done later.
> 
> Brad
> 
> 
> 
> 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