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

Thu Jan 21 10:12:25 UTC 2010

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,
>>
>>
>>