Code Review Request: TLS 1.3 Implementation

Bradford Wetmore bradford.wetmore at oracle.com
Wed Jun 13 00:43:48 UTC 2018


I really don't have much to say about SSLEngineImpl.  I like the 
refactoring you've done.  Just some nits.

Ignore my previous suggestion about conContext, after seeing it a bit 
more often, it makes sense now.

SSLSocketImpl.java
------------------
72:  I noticed you removed the public here from SSLEngineImpl, and there 
wasn't one from SSLServerSocketImpl, but public is still here in 
SSLSocketImpl.  Maybe we can remove it?  Is there a reason to keep it 
public?


SSLEngineImpl.java
------------------
116:  You could put this on one line if you want.

122-123:  Indent 8 spaces here.

757:  At line 735, you use

CipherSuite.validValuesOf(suites);

but here you use:

ProtocolVersion.namesOf(protocols);

Seems like it's basically the same kind of method, do you want to make 
the names consistent.

Brad




On 6/8/2018 10:21 AM, Xuelei Fan wrote:
> Here is the 3rd full webrev:
>     http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.02
> 
> and the delta update to the 1st webrev:
>     http://cr.openjdk.java.net/~xuelei/8196584/webrev-delta.01
> 
> 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


More information about the security-dev mailing list