RFR 8171279: Support X25519 and X448 in TLS 1.3

[Adam Petcher]

On 9/6/2018 12:10 PM, Xuelei Fan wrote:

>> The algorithm name is not quite sufficient. See the new methods that 
>> were added to ECUtil that encode/decode public keys. We also need to 
>> know the key length (which is in XECParameters) in order to 
>> encode/decode public keys.
>>
> I did not get your point.  The public key sizes for x25519 and x448 
> are fixed, right?

Yes, the key sizes are fixed. All we need in ECUtil is a mapping from 
curve name to this (fixed) size. Are you suggesting some other solution, 
other than using the XECParameters to map curve names to key sizes?

>
>>
>> Yes, it supports "TlsPreMasterSecret" in the webrev now. Perhaps I'm 
>> not sure what you are suggesting here.
>>
> OKay, let me say it in another way.
>
> Thank you for making it works with the SunJSSE provider as you use a 
> SunJSSE provider private algorithm name "TlsPreMasterSecret", and 
> implement the algorithm in the cyrpto provider.  That's good.
>
> The "TlsPreMasterSecret" is not a public one, and it is not expected 
> to be used in other JSSE provider.  If I want to implement a JSSE 
> provider by myself, and I use the name "x25519", the crypto provider 
> will deny it.  So I have to use the "TlsPreMasterSecret" for my 
> provider. However, the name is not supported, and the name can be 
> changed at anytime in the future. How should I do to use the crypto 
> provider for my JSSE provider? Looks like no way to use the JDK cyrpto 
> provider unless I use the SunJSSE private name.  Should I implement my 
> own crypto provider? I don't want to do that unless it is really 
> necessary.
>
> For the KeyAgreement.generateSecret​(String algorithm) method, it 
> seems that the supported algorithms of each provider are missed in the 
> documentation.  As may make the method hard to use.
>
> This issue is not specific to XDH.  I'm fine if you don't want to 
> address it in this update.

I agree that this is an issue, but I think it is out of scope for this 
ticket. There are several tickets[1,2] related to the issues associated 
with generateSecret, we just need to find the time to work on them. 
Continuing work on the KeyDerivation JEP[3] may also help with all of this.

[1] https://bugs.openjdk.java.net/browse/JDK-8189441
[2] https://bugs.openjdk.java.net/browse/JDK-6791936
[3] https://bugs.openjdk.java.net/browse/JDK-8189808

>
>>
>> In the latest webrev, I changed it so there is a single static 
>> NamedGroupFunctions of each type, and the NamedGroup is passed in as 
>> the first argument to each method that requires it (rather than being 
>> a member of NamedGroupFunctions).
>>
> After the re-org, I guess you can put the inner classes in NamedGroup 
> enum and declare them as private?  The getFunctions() method may be 
> unnecessary then.

I don't know if that works, exactly, due to the fact that I can't 
reference static enum members in the body of an enum constructor. How 
about this alternative? I can move the NamedGroup enum and all the 
NamedGroupFunction classes into a separate class (in a separate file) 
called NamedGroups. Then all the NamedGroupFunction classes can be 
private in this class, but the NamedGroup enum can still have package 
access. I would prefer to leave the getFunctions() method of NamedGroup 
(and keep it private) because the functions object may be missing and 
the Optional return type of getFunctions() forces me to deal with this 
when I call it from within NamedGroup.