Code Review Request: TLS 1.3 Implementation

[Xuelei Fan]

Update: http://hg.openjdk.java.net/jdk/sandbox/rev/0811eaea3cd4

On 6/11/2018 3:44 PM, Valerie Peng wrote:
> Hi Xuelei,
> 
> <sun/security/ssl/SSLConfiguration.java>
> 
> - If no child class intended, class may be made final.
Updated.

> - line 99-101 and 120-133: delete as the comments said so?
It is used for interop testing right now.  I will remove them before we 
ship the product.  It it tracked with:
    https://bugs.openjdk.java.net/browse/JDK-8204636

> - I have some doubt on line 191 and 198, does "noSniExtension/Matcher" 
> means "no SNI Extension/Matcher"? If yes, it seems that the condition 
> should be
> 
>     if (serverNames.isEmpty() || noSniExtension) {
> 
The checking is a little bit confusing.  If serverNames is an empty 
list, it may indicate one of two status:
. the server names is configured to be empty, or
. the server name is not configured

The behaviors of the two status are different, so we introduced another 
flag, 'noSniExtension', which is used together with 
serverNames.isEmpty() to identify the status.

It's confusing to me as well.  Looks like we can make an improvement 
here.  I will think more about it.

> - Essentially getEnabledExtensions(SSLHandshake, ProtocolVersion) is 
> almost same as getEnabledExtensions(SSLHandshake,
> List<ProtocolVersion>. It looks possible to refactor the impl to 
> minimize code duplication. But this is no big deal.
> 
Good point.  Updated.

Thanks,
Xuelei

> Thanks,
> Valerie
> 
> 
> 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
>