Code Review Request: TLS 1.3 Implementation

Valerie Peng valerie.peng at oracle.com
Tue Jun 19 19:19:09 UTC 2018


Hi Xuelei,

Just questions and some nits.

<src/java.base/share/classes/sun/security/ssl/SSLKeyDerivation.java>
looks good

<src/java.base/share/classes/sun/security/ssl/SSLBasicKeyDerivation.java>
looks good

<src/java.base/share/classes/sun/security/ssl/SSLSecretDerivation.java>
- line 98: for unsupported digest, maybe we should consider throw 
exceptions. It's easier to find out where the support needs to be added 
if we fail early.

<src/java.base/share/classes/sun/security/ssl/SSLMasterKeyDerivation.java>
- Just wondering why we need difference classes of 
SecretKeyDerivationGenerator (i.e. 
S30/T10/T12MaterSecretKeyDerivationGenerator instances), it seems that 
they all use LegacyMasterKeyDerivation class. The enum already has name 
which indicates the protocol version, is the SSLKeyDerivationGenerator 
passed around by itself and you are using its class to find out the 
protocol version? Or, is there other reason for this?

<src/java.base/share/classes/sun/security/ssl/SSLTrafficKeyDerivation.java>
- line 71: add a default to catch the unsupported ProtocolVersion case?
- line 319: check params or assert params to be null? I assume it'll be 
null as it seems to be not used.

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