On 11/8/2018 10:30 PM, Weijun Wang wrote:
On Nov 9, 2018, at 12:28 AM, Adam Petcher <adam.petc...@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.
