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

Thu Jan 21 07:44:29 UTC 2010

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