Hello Andrew, I'll need a little more time to come up to speed on this fix. I'm concerned that there may be interoperability or backwards compatibility issues.
Andrew John Hughes wrote: > 2009/9/2 Andrew John Hughes <gnu_and...@member.fsf.org>: >> 2009/9/2 Michael StJohns <mstjo...@comcast.net>: >>> At 09:38 PM 9/1/2009, Andrew John Hughes wrote: >>>> 2009/9/2 Michael StJohns <mstjo...@comcast.net>: >>>>>  This appears to be related specifically to PKCS11. Specifically, >>>>> PKCS11 >>>>> v2.20 has some ambiguity of the representation of an EC point (which is >>>>> different in the text than an ASN1 ECPoint). >>>>> >>>>> This is being clarified in v2.30 with the unencoded point format (e.g.the >>>>> format described in X9.62, where the first octet indicates the encoding >>>>> and >>>>> there are either N or 2N octets following) being the expected value, but >>>>> with PKCS11 providers allowed - legacy - to accept either. >>>>> >>>>> One of the reasons for going that way was how the JDK PKCS11 provider had >>>>> interpreted the issue and implemented its code. >>>>> >>>>> I don't support this fix - among other things, this fix only deals with >>>>> 1/2 >>>>> of the problem. The other half is related to encoding the value. Also, >>>>> changing the code at decodePoint seems further into the stack than needed >>>>> and may affect other uses of that method. >>>>> >>>> That's really too vague to be of much help in improving the patch. >>>> You seem to be saying little more than 'I don't like it'. >>> Sorry about that. My point was that your patch didn't completely solve the >>> problem and that the point at where you were fixing it could have some bad >>> side effects for anyone calling decodePoint directly. >>> >>> >>>>> There's an existing JDK bug on this coming at it from a different >>>>> direction >>>>> - 6763530 ... and there may be considerations at >>>>> >>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=480280 >>>>> >>>> It seems likely that's the NSS change that causes the current failure. >>>> The fix I submitted here is based on the way this is handle in NSS. >>>> In fact, the code is similar enough to suggest that one was developed >>> >from the other. >>>>>  that should be looked at. >>>> The JDK bug is not really 'from a different direction', it's reporting >>>> exactly the same error but from a less trivial example (I get the same >>>> failure while trying to create an example key, while this seems to >>>> require specific hardware if I'm reading it correctly). >>> Not exactly. You're using the NSS as a PKCS11 module - this problem would >>> occur with any PKCS11 module that implements EC stuff. >>> >>> >>>> Also see 6779460 which is mostly a duplicate of >>>>> 6763530. >>>>> >>>> The patch on 6779460 seems wrong. It means that the method will >>>> return a DER-encoded value where it would either have returned an >>>> uncompressed value before or failed. >>> My point exactly as I mentioned in the comments. :-) >>> >>> >>>>> It's probable that the fix I suggested at 6763530 (in comments >>>>> submitted 29 >>>>> Nov 08) may be a better approach given the NSS fixes. I believe it will >>>>> fix >>>>> the keytool problem noted in the original message. >>>>> >>>> Ok, I can see the logic in the fix and it would appear to work, though >>>> I haven't tested it. >>>> Given the patch was written nine months ago, why has it not been >>>> applied? If it had, it would have saved me hours having to debug this >>>> same issue again. >>> Yup. I did do a search for PKCS11 related bugs when I encountered the same >>> problem and did find the original error. >>> >>>> Do you have an SCA with Sun? If so, I'll create a webrev based on your >>>> patch and we can finally get this fixed. Without it, NSS support is >>>> completely broken in OpenJDK6 which makes me wonder why this is a low >>>> priority bug! >>> I do have an SCA on file. Note that the recommendation from the NSS guys >>> was to raise the priority. >>> >>> The reason I haven't submitted this is because I submitted a different EC >>> fix https://bugs.openjdk.java.net/show_bug.cgi?id=100048 per the >>> documented process >>> and was waiting on progress there before continuing. I've got a number of >>> EC and PKCS11 related fixes I'd like to submit, but I was trying for a >>> worked example before proceeding. And then I got busy with some other >>> things... >>> >>> Mike >>> >>> >>> >>> >>> >>>>> Mike >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> At 04:39 PM 9/1/2009, Joe Darcy wrote: >>>>> >>>>> Andrew John Hughes wrote: >>>>> >>>>> 2009/8/28 Andrew John Hughes <gnu_and...@member.fsf.org>: >>>>> >>>>> In OpenJDK6, the elliptic curve cryptography algorithms are available >>>>> if the PKCS11 provider is configured to point to NSS. See: >>>>> >>>>> http://blogs.sun.com/andreas/entry/the_java_pkcs_11_provider >>>>> >>>>> If NSS is configured as specified in this blog, keytool can be used to >>>>> generate a key as follows: >>>>> >>>>> Hello. >>>>> >>>>> Allowing keytool and friends to work in more cases if the provider is >>>>> capable seems fine to me. >>>>> >>>>> Security team, do you have concerns about this patch? >>>>> >>>>> Thanks, >>>>> >>>>> -Joe >>>>> >>>> >>>> >>>> -- >>>> Andrew :-) >>>> >>>> Free Java Software Engineer >>>> Red Hat, Inc. (http://www.redhat.com) >>>> >>>> Support Free Java! >>>> Contribute to GNU Classpath and the OpenJDK >>>> http://www.gnu.org/software/classpath >>>> http://openjdk.java.net >>>> >>>> PGP Key: 94EFD9D8 (http://subkeys.pgp.net) >>>> Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8 >>> >>> >> Ok here is a new webrev: >> >> http://cr.openjdk.java.net/~andrew/6763530/webrev.02/ >> >> with a slightly revised version of your change (you can't throw a >> PKCS11Exception which only takes a long ID from the native code, so I >> changed this to an IllegalArgumentException). >> >> Security team, does this look ok to push? >> -- >> Andrew :-) >> >> Free Java Software Engineer >> Red Hat, Inc. (http://www.redhat.com) >> >> Support Free Java! >> Contribute to GNU Classpath and the OpenJDK >> http://www.gnu.org/software/classpath >> http://openjdk.java.net >> >> PGP Key: 94EFD9D8 (http://subkeys.pgp.net) >> Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8 >> > > Ping! Security developers, any thoughts on this patch: > > http://cr.openjdk.java.net/~andrew/6763530/webrev.02/ > > Does it look ok to push? > > Thanks,
