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.petc...@oracle.com> wrote:
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.
