Code Review Request: TLS 1.3 Implementation

Xuelei Fan xuelei.fan at oracle.com
Fri Jun 22 04:30:09 UTC 2018


Update: http://hg.openjdk.java.net/jdk/sandbox/rev/76025c6c6e29

On 6/21/2018 1:10 PM, Valerie Peng wrote:
 > - line 175, the exception message seems not right, clientAlias is a
 > private key entry with no cert chain stored.
Good catch!

 > - line 56, instead of returning null, I wonder if we should throw
 > exception to catch unsupported type early.
The 'null' returned value is useful when the key type does not support 
X.509 authentication.  It is a common case.  Using return value may be 
more lightweight than using exception.

On 6/20/2018 6:15 PM, Valerie Peng wrote:
> Hi Xuelei,
> 
> <src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java>
> - line 2, why is the copyright year changed from 2015 only to 2003 and 
> 2018? Don't we normally preserve the first year and only update/add the 
> second year?
Update.

> - line 110, instead of erroring out, I wonder if it's better to call 
> createPossessions(handshakeContext) and only error out if the result is 
> more than one.
Good catch!  The createPossessions(handshakeContext) is not used so it 
is not necessary to implement this interface any more.  Removed this 
interface and the method.

> - line 421 and 464, RSA is left out for these two cases, is this 
> asymmetry (comparing to line 381 and 494) intentional?
> 
The block started from line 381 is asymmetry to line 494; line 412 to 
line 464.  RSA is in the right block accordingly.

> <src/java.base/share/classes/sun/security/ssl/RSAServerKeyExchange.java>
> - line 2, for a new file, shouldn't there be only one year, i.e. 2018?
> - line 219, is this INSTANCE really used? I did a quick search and 
> didn't find reference to it.
> - line 309, why is EC mentioned here? Typo perhaps?
> 
All good catches!

> <src/java.base/share/classes/sun/security/ssl/ServerKeyExchange.java>
> - line 2, for a new file, shouldn't there be only one year, i.e. 2018?
> - line 89, typo: "producing" should be "consuming"
> - line 107, typo: "not" should be "no"
> 
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
> 



More information about the security-dev mailing list