Code Review Request: TLS 1.3 Implementation

Xuelei Fan xuelei.fan at oracle.com
Sun Jun 10 04:55:20 UTC 2018


Hi Valerie,

All good catches!  I made a code clean up in the update:
    http://hg.openjdk.java.net/jdk/sandbox/rev/38c2a4078033

Thanks,
Xuelei

On 6/8/2018 5:16 PM, Valerie Peng wrote:
> Hi Xuelei,
> 
> <sun/security/ssl/SSLSessionImpl.java>
> 
> Will this.requestedServerNames ever be null? This field is initialized 
> in constructors with Collections.xxxList(...) method.
> 
> Are the two private fields "peerPrincipal" and "localPrincipal" really 
> used? There are two pkg private methods, setPeer/LocalPrincipal(...) 
> which sets their values, however, I don't see these two fields used 
> inside this class.
> 
> line 232, 240, 248, 264, - 273: Are we sure that we want to throw 
> RuntimeException? It seems too generic?
> 
> Quite some more fields are initialized in one constructor (line 191) 
> than the other one (line 175), e.g. peerCerts, compressionMethod, 
> negotiatedMaxFragLen (and more). Is such difference expected? The 
> ordering of initialization is also different which may be error prone 
> for future maintenance.
> 
> That's all I have for today. Will continue review next Monday.
> 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
> 


More information about the security-dev mailing list