[security-dev 01547]: Re: PING: [PATCH FOR REVIEW]: 6763530: Fix breakage of NSS-based Elliptic Curve Cryptography in OpenJDK6

Thu Jan 21 02:24:15 PST 2010

Wonderful! Thanks!

Cheers,
Tomas

Vincent Ryan wrote:
> I hear ya. Sorry for the delay on this. I'll push the fix for OpenJDK today.
> 
> 
> On 21/01/2010 07:44, Tomas Gustavsson wrote:
>> Now it has one more vote.
>>
>> /Tomas
>>
>> Andrew John Hughes wrote:
>>> 2010/1/20 Tomas Gustavsson <tomas at primekey.se>:
>>>> I'll second this request. This is a critical patch and many production
>>>> installations have to live with this manually patched now.
>>>>
>>>> I know of no pkcs11 implementation that works with the current code.
>>>>
>>> It has four votes:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6763530
>>> I don't know how many they need to wake up and review the patch.
>>>
>>> The new release of IcedTea6 1.7 is imminent and will include the fix
>>> so it should at least be resolved on the next version shipping with
>>> most GNU/Linux distributions.
>>>
>>>> Regards,
>>>> Tomas Gustavsson
>>>> PrimeKey Solutions AB
>>>>
>>>>
>>>> On Wed, 20 Jan 2010, Michael StJohns wrote:
>>>>
>>>>> Hi - this seems to have stalled out again.  Any chance of revival?
>>>>>
>>>>> Mike
>>>>>
>>>>>
>>>>> At 12:33 PM 9/24/2009, Vincent Ryan wrote:
>>>>>> 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_andrew at member.fsf.org>:
>>>>>>>> 2009/9/2 Michael StJohns <mstjohns at comcast.net>:
>>>>>>>>> At 09:38 PM 9/1/2009, Andrew John Hughes wrote:
>>>>>>>>>> 2009/9/2 Michael StJohns <mstjohns at 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_andrew at 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,
>>>
>>>