RFR 8213400: Support choosing curve name in keytool keypair generation

Sean Mullan sean.mullan at oracle.com
Wed Nov 14 21:55:13 UTC 2018


For 12, I think we should also document the group names in the std. alg 
names document so we have somewhere to point to for what names can be 
specified, otherwise users will be guessing.

So I targeted this to 12: https://bugs.openjdk.java.net/browse/JDK-8210755

--Sean

On 11/11/18 8:08 PM, Weijun Wang wrote:
> Webrev updated at
> 
>    https://cr.openjdk.java.net/~weijun/8213400/webrev.01/
> 
> Please review again.
> 
> I've removed the change to CurveDB and NamedCurve. The test now simply looks at key.getParams().toString(). This is implementation dependent but it works within OpenJDK.
> 
> No change on other files.
> 
> I filed https://bugs.openjdk.java.net/browse/JDK-8213719 for the double BD issue.
> 
> Thanks
> Max
> 
> 
>> On Nov 9, 2018, at 11:33 PM, Adam Petcher <adam.petcher at oracle.com> wrote:
>>
>> 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.
> 


More information about the security-dev mailing list