RFR 8171279: Support X25519 and X448 in TLS 1.3

[Adam Petcher]

Updated webrev: http://cr.openjdk.java.net/~apetcher/8171279/webrev.01/

On 9/4/2018 3:25 PM, Xuelei Fan wrote:

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

Sorry, I don't think I understand what you are suggesting here. As far 
as I know, nothing in sun.security.ssl uses XECParameters directly. It 
is used indirectly through ECUtil to encode and decode keys, and in this 
case the name is used to look up the parameters. Is there some place in 
the code where JSSE is doing something too complicated related to these 
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?

I simply continued the pattern that was used in DH and ECDH. We can use 
a different string here, but I worry that people will be surprised if DH 
and ECDH support "TlsPreMasterSecret" but XDH doesn't. I would rather 
continue to use "TlsPreMasterSecret" for now, and then add support for a 
standard, TLS-independent name (that means the same thing) in a separate 
ticket. But if you feel strongly that XDH should not support 
"TlsPreMasterSecret", then perhaps we can use a different string now. In 
this case, let me know if you have any suggestions for what string to use.

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

A simple way to accomplish this is to leave the structure alone and add 
methods in NamedGroup that pass through to the corresponding methods in 
NamedGroupFunctions. I made this change in the updated webrev. Let me 
know what you think.

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

This interface is only used to validate public keys against algorithm 
constraints. In the latest webrev, I moved all of this functionality 
into NamedGroupFunctions and removed the SSLKeyAgreementCredentials 
interface.