code review request: 8012971 PKCS11Test hiding exception failures

Anthony Scarpino anthony.scarpino at oracle.com
Wed Jul 24 20:53:20 UTC 2013


On 07/23/2013 06:00 PM, Valerie (Yu-Ching) Peng wrote:
>
> <PKCS11Test.java>
>
> 263                 System.arraycopy(data, 900, data, 0, 100);
> 264                 is.read(data, 0,  900);
>
> Do you really mean to overwrite the data[0..99] that you just copied on
> line 263 with line 264?


Good catch.. I would have thought all my testing would have tripped me 
up on this, but it appears it didn't.

> In addition, don't you want to know how much is read in order to exclude
> the data from earlier read(...) calls in case that the current read only
> returns a few bytes?

Yeah, the end of the file read is a bit sloppy, even though it should 
never see the end of the file as all the nss libraries have the header 
in it.

>
> Do you have the list of supported curves does NSS ECC Basic and Extensive?
> Some tests you check the curves and some tests you return immediately.

There are three.  In TestCurves.java is one example where the test now 
to distinguish between Basic and Extended, and to know the proper PKCS11 
error code (CKR_DOMAIN_PARAMS_INVALID) if it is not supported.

>
> Also, do you know if there is a mozilla bug# for the DER issue that you
> referred in several files?
> Seems hard to tell which call would trigger this NSS bug, i.e. when to
> add this check, until the test actually failed using the older NSS library.

I was never comfortable in the NSS DER bug.. My guess was it is 480280, 
but I was not confident and thought it was better to not list if I 
wasn't sure.

It is a trial-and-error process in finding the NSS bugs.  The easiest is 
finding an older version of NSS failing and a new one working,

>
> After seeing all these checks and list of known bugs for testing against
> NSS, I think we probably need a README or some wiki page to keep track
> all this...

I can throw a readme in the directory with the bug IDs and even my 
theory that the DER is 480280.

webrev updated at:
http://cr.openjdk.java.net/~ascarpino/8020424/webrev.01/

> Thanks,
> Valerie
>
> On 07/17/13 13:51, Anthony Scarpino wrote:
>> JDK-8020424 The NSS version should be detected before running crypto
>> tests
>> http://cr.openjdk.java.net/~ascarpino/8020424/webrev.00/
>>
>> Tony
>>
>




More information about the security-dev mailing list