I applied your patch and ran some initial tests, and everything looks
good so far. I'm running the the regression tests on all platforms, and
I'll let you know how it goes.
We need a JDK Reviewer to review and approve the code, so I'm hoping
someone will volunteer to take a look. For the benefit of the reviewer:
I checked all the math parts and everything seems like it should work.
Both the point and field arithmetic are done using general-purpose
functions that should work for any curve over a prime field. In the case
of the field arithmetic, this means the performance is going to be much
worse than in other curves. This is unavoidable, though, because
Brainpool primes don't have any structure that we can use to optimize
the field arithmetic.
There are a couple of responses inline below.
On 1/17/2018 6:02 PM, Tobias Wagner wrote:
I switched to that repository now. As described on
http://openjdk.java.net/contribute/, I was
working with the http://hg.openjdk.java.net/jdk9/jdk9 repository.
That page seems to be out of date. I'm working to get it updated. Thanks
for letting me know.
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.
I left them in place - my interpretation is, that they are meant for a check
before accessing the *_oids arrays, as C arrays have no implicit check for that.
As long as only oids from CurveDB are used, there would be no problems.
The least significant byte of the requested oid is used as index. If that index
exeeds the defined array length, something odd from the memory there is
returned.
At least that's my understynding of C and the comment there.
You're interpretation is better than mine, so I agree it's best to leave
the comment in. If you wanted to, you could address the issue by
comparing against sizeof(array)/sizeof(array[0]), but this is definitely
not necessary.
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.
Yes, there are tests in TestCurves, which runs through TestEC. TestCurves gets
a List
of all supported ECParameterSpec by the Provider and runs ECDSA signatures and
verifications
with all of them. The results of all curves are logged in the jtreg report of
TestEC.
I see. This satisfies my request. Thanks.
