[PATCH]: Support for brainpool curves from CurveDB in SunEC

Adam Petcher adam.petcher at oracle.com
Thu Jan 18 19:57:55 UTC 2018


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.




More information about the security-dev mailing list