RFR 8171279: Support X25519 and X448 in TLS 1.3

[Xuelei Fan]

I have no finished the full code review.  So far, I have a few question 
about the struct of the code.
1. XECParameters
I can see the reason to dynamic parameters for something other than 
X25519/X448.  But for JSSE, currently, only named curves are used.  It 
would be nice to use the name for the static parameters.

2. "TlsPremasterSecret" only XDH key agreement
It would be nice the JCE implementation can support key agreement other 
than TLS protocols and providers other than SunJSSE provider.  It would 
be nice if the JCE implementation does not bind to the SunJSSE provider 
private algorithm name.

We used to use SunJSSE private name in the JCE crypto implementation. 
But there are some known problems with this dependence.

Is there a problem to support generic key agreement?

3. NamedGroupFunctions
It might be more straightforward to define these functions in 
NamedGroup.  If comparing nameGroup.getFunctions().getParameterSpec() 
and namedGroup.getParameterSpec(), I'd prefer the latter.

4. SSLKeyAgreementCredentials
I did not see too much benefit of this new interface.  It is not always 
true that a key agreement could have a public key.  Although we can 
simplify the key agreement for public key based, but it also add 
additional layers.

I know where this improvement comes from.  Maybe, you can consolidate 
the named group functions, and encode/decode the credentials there.

Xuelei


On 8/30/2018 8:58 AM, Adam Petcher wrote:
> Webrev: http://cr.openjdk.java.net/~apetcher/8171279/webrev.00/
> JBS: https://bugs.openjdk.java.net/browse/JDK-8171279
> 
> Please review the following change to add support for X25519 and X448 
> (XDH) to TLS 1.3. This change includes some refactoring to remove code 
> that was duplicated for DH and ECDH, and to avoid adding more for XDH. 
> In addition to running the included regression test, I tested by 
> connecting to an openssl server and confirmed that the connection was 
> established using TLS 1.3 and X25519/X448.
> 
> Here are some detailed notes:
> 
> *) The NamedGroupFunctions class was added to hold the functions that 
> are needed for key agreement with some named group. Most of the 
> group-specific code was moved into subclasses of NamedGroupFunctions. 
> This allowed me to remove a bunch of code like "if (type == ECDHE) {...} 
> else if (type == FFDHE) {...}".
> *) There are a couple of files in the webrev with no changes due to a 
> webrev issue. Please ignore them.
> *) I moved some code related to XDH parameters and encoding into 
> java.base. ECUtil now has code to encode/decode XDH public keys, and the 
> XECParameters file was moved into java.base/sun.security.util. This 
> organization is similar to how CurveDB and NamedCurve are in java.base, 
> and it allows the TLS implementation to encode/decode keys without using 
> the jdk.crypto.ec module.
>