RFR 8171279: Support X25519 and X448 in TLS 1.3

[Adam Petcher]

Xuelei,

Here is the latest webrev: 
http://cr.openjdk.java.net/~apetcher/8171279/webrev.04/

I modified the TLS implementation so that it uses X509EncodedKeySpec 
when interacting with the provider. I did a small amount of refactoring 
in X509Key to expose the functionality I needed to do this. This change 
removed the dependency on XECParameters, and I moved it back into 
jdk.crypto.ec. I also moved some more functions into NamedGroup (e.g. 
isAvailableGroup).

This webrev also has the change to NamedGroup that makes 
NamedGroupFunctions private. I put the NamedGroup enum into its own 
file, which I think is reasonable because it is used in several places 
other than SupportedGroupsExtension. It also makes sense to separate all 
of the known named groups and their properties (NamedGroups.java) from 
the supported_groups extension and the logic related to which groups are 
supported for key exchange (SupportedGroupsExtension.java). This change 
required changes to the import statements of several files.

I'm not totally sure I understood your concern related to the 
AlgorithmParameters map. The map is always created, but we only create 
and cache parameters when they are first needed. Of course, the map is 
not a proper cache, and the objects stay in memory forever once they are 
created. Is this your concern, or is it something else?

I've made several structural changes since the first webrev, and I 
haven't been paying too close attention to style, so you probably 
shouldn't either. Once the approach and structure look good to you, I 
can clean up the code and submit another webrev so you can check it for 
style, formatting, etc. I also haven't merged in changes from the 
default branch in a while, so I'll need to do that, too.


On 9/7/2018 12:12 PM, Xuelei Fan wrote:
> On 9/7/2018 9:03 AM, Adam Petcher wrote:
>> This is more clear, thanks. See below.
>>
>>
>> On 9/7/2018 11:34 AM, Xuelei Fan wrote:
>>> EncodedKeySpec keySpec = ... // find a way to construct the keySpec
>>>                              // at least, we can use:
>>>                              //    new EncodedKeySpec(byte[]);
>>>                              // But please check if there's a better 
>>> one
>>
>> Do you mean X509EncodedKeySpec, or some class that is specific to XDH? 
> I did not search the spec definitions.  At least we can use the 
> EncodedKeySpec class.   It's nice if you can find a better one, or 
> define a new one for XDH.
>
> Your following comments make sense to me.
>
> Thanks,
> Xuelei
>
>> An X509EncodedKeySpec would probably work---we would just need to put 
>> the key into a SubjectPublicKeyInfo, which means I need the OID. Is 
>> it okay for me to put the OID in JSSE? I could put it in the 
>> NamedGroup enum, like this:
>>
>>      X25519      (0x001D, "x25519", true, "x25519", "1.3.101.110",
>>          ProtocolVersion.PROTOCOLS_TO_13),
>>      X448        (0x001E, "x448", true, "x448", "1.3.101.111",
>>          ProtocolVersion.PROTOCOLS_TO_13),
>>
>> I'm not sure if the second x25519/x448 strings (for algorithm and 
>> NamedParameterSpec names) would still be needed, since I can use the 
>> OID in some of these places. If it's not needed, then perhaps I can 
>> remove it and only use the OID to interact with the JCA provider.
>>
>> We don't have a type for XDH encoded public keys. It would be nice to 
>> be able to do something simple like:
>>
>> byte[] keyBytes = ...
>> NamedParameterSpec paramSpec = new NamedParameterSpec("X25519");
>> XECEncodedPublicKeySpec keySpec = new XECEncodedKeySpec(paramSpec, 
>> keyBytes);
>>
>> and then give keySpec to the "XDH" key factory. But this 
>> XECEncodedKeySpec type does not exist, so this would require an 
>> enhancement to the API.