RFR 8213400: Support choosing curve name in keytool keypair generation

[Adam Petcher]

On 11/8/2018 10:30 PM, Weijun Wang wrote:

>> On Nov 9, 2018, at 12:28 AM, Adam Petcher <adam.petcher at oracle.com> wrote:
>>
>> On 11/8/2018 8:10 AM, Weijun Wang wrote:
>>
>>> - CurveDB.java:
>>>
>>> -        add("sect163r2 [NIST B-163]", "1.3.132.0.15", BD,
>>> +        add("sect163r2 [NIST B-163]", "1.3.132.0.15", B,
>>>
>>> All other NIST B-*** curves do not have BD. This should have been a typo.
>> I think this will change the default 163-bit curve from sect163r2 to sect163k1. We shouldn't change these defaults without a compelling reason.
> I think this is a bug, as there should not be 2 curves with the same field size both having BD.
>
> I can file another bug on this. Maybe it needs it own CSR.

Yes, it's probably a bug/typo. You can also fix it by changing sect163k1 
from BD to B, since this won't change the behavior (though this would 
need to be tested). But I wouldn't worry about it. You can create a 
separate ticket if you like, but I would expect that fixing this is very 
low priority. The current behavior may even be correct (or at least "as 
intended"), and this is just a code cleanup issue.

>>> - NamedCurve.java:
>>>
>>> A new field commonNames added, which is used by the new GroupName.java test.
>> I don't see why this is necessary. The test is using this list of common names to make sure the correct named curve is used. Why not just check the value returned by NamedCurve.getName() against the expected (canonical) name of the curve? Or check the OID?
> NamedCurve.getName() returns "secp256r1 [NIST P-256, X9.62 prime256v1]".
>
> I don't want to canonicalize the name (how? returning NamedCurve.getName()?) or use an OID. The test is about making sure the curve used matches the one user specified. What if I am making the same error inside keytool and this canonicalization or string-to-oid method?
>
> <snip>

I think what I am suggesting is map from common name to canonical name 
or OID in the test. Then you can test that the correct common name is 
used, and that the mapping from common name to curve is correct. If you 
don't want to write out this map, then you can implement it by looking 
up the common name in the EC AlgorithmParameters, and then converting to 
an ECParameterSpec. This just moves the complexity from CurveDB to the 
test, but I think that is better.

Though I don't have very strong feelings about this. Perhaps the list of 
common names in the NamedCurve will be useful for other things. If you 
prefer it the way that it is, then I am okay with it.