Code Review Request: TLS 1.3 Implementation

[Xuelei Fan]

 > BaseSSLSocketImpl.java
 > ----------------------
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/bad9f4c7eeec

Update to handle EOFException and requireCloseNotify property.

Xuelei

On 6/7/2018 10:28 AM, Bradford Wetmore wrote:
> 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