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.
