Code Review Request: TLS 1.3 Implementation

[Valerie Peng]

I see. Maybe document what you explained for these flags would help.
Update looks good.
Thanks,
Valerie

On 6/11/2018 5:23 PM, Xuelei Fan wrote:
> 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
>>