Update: http://hg.openjdk.java.net/jdk/sandbox/rev/1cc2f6afa943

On 6/19/2018 12:19 PM, Valerie Peng wrote:
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.

Good point!

<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?

It was designed to use the new key derivation APIs. It looks unusual now as there is no key derivation APIs yet.

Updated by removing the S30/T10/T12MaterSecretKeyDerivationGenerator classes.

<src/java.base/share/classes/sun/security/ssl/SSLTrafficKeyDerivation.java>
- line 71: add a default to catch the unsupported ProtocolVersion case?
The default case is considered in the final "return null" clause.

- line 319: check params or assert params to be null? I assume it'll be null as it seems to be not used.
The caller might use non-null params, potentially. I would like to ignore the params and checking, as if it is not used in this implementation.

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

Reply via email to