Great! I took a look at the patch, and I have some comments, the first
of which probably needs to be addressed before I can test the change:
1) Is this patch against the http://hg.openjdk.java.net/jdk/jdk
repository? I suspect it isn't because some of the paths are different
than what I expect. We have made a lot of changes to the repositories in
the last few months. If this patch is against an older repo, please send
a patch against http://hg.openjdk.java.net/jdk/jdk .
2) TestECDH.java: It's probably better to remove the provider name check
on line 116 and test on any providers that support the curve.
3) oid.c: I think you can remove the comments that say "XXX bounds
check" (e.g. line 362). If I am interpreting these comments correctly,
they are saying that memcmp may read out of bounds, but you fixed that
problem by using oideql.
4) Is there an existing test that exercises ECDSA with the new curves?
Maybe there is something in the PKCS11 tests that does this already, but
I didn't find it. I think we should have an ECDSA test to make sure that
we didn't forget anything. ECDSA test vectors probably aren't
necessary---a simple test that signs and verifies using the new curves
should be sufficient.
On 1/12/2018 9:12 AM, Tobias Wagner wrote:
Hi,
here is the next patch for brainpool curve support in SunEC.
Differences from the first patch:
* Brainpool curves with less than 256 bits are removed. Subsequently, the curve
oid check is made more robust to avoid null
pointer caused Segmentation Faults in memcmp calls.
* Bug JDK-8189594 is fixed.
* Known answer tests for each new curve are added to
sun.security.pkcs11.ec.TestECDH. The tests are only executed, if the
tested provider's name is "SunEC" and the tested provider claims to support the
respective curve. For SunEC, these tests are
executed during sun.security.ec.TestEC.
I decided to add these test vectors to TestECDH to avoid code duplications, as
TestECDH is describes exactly the test
for that kind of test vectors.
The superclass to TestECDH, TestPKCS11, is also adapted to provide a method to
check, whether one particular curve is
supported.
While the test vectors for the 256, 384 and 512 bit curve are taken from [1],
the test vector for brainpoolP320r1 comes from [2].
The latter one is a draft version of RFC 6954.
Regards,
Tobias
[1] https://tools.ietf.org/html/rfc7027#appendix-A
[2] https://tools.ietf.org/html/draft-merkle-ikev2-ke-brainpool-00#appendix-A.5
