Just FYI. SoftHSM2 from the OpenDNSSec project is a good P11 to test with, and I believe it supports brainpool in recent versions. https://github.com/opendnssec/SoftHSMv2
It works really good) Regards, Tomas On 2018-02-09 02:03, Valerie Peng wrote: > Hi Tobias, > > Just curious, which PKCS11 library did you use to test your patch? After > I applied your patch and ran the regression tests, I noticed that both > the Solaris PKCS11 library and NSS skipped testing Brainpool curves with > different error codes which may be due to lack of support... > > Regards, > Valerie > > On 1/17/2018 3:02 PM, Tobias Wagner wrote: >> -----Ursprüngliche Nachricht----- >>> Von:Adam Petcher <adam.petc...@oracle.com> >>> Gesendet: Die 16 Januar 2018 18:38 >>> An: security-dev@openjdk.java.net >>> Betreff: Re: [PATCH]: Support for brainpool curves from CurveDB in SunEC >>> >>> 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 . >> 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. >> >>> 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. >> OK, it's removed. I was thinking the same. >> >>> 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. >> >>> 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 also changed the InvalidCurve test to use brainpoolP160r1 now, as >> brainpoolP256r1 is supported >> by using this patch. >> >>> 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 >>>> >>>> >>>> >
