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

Fri Jan 22 18:53:37 UTC 2010

On 22/01/2010 01:38, Andrew John Hughes wrote:
> 2010/1/21 Vincent Ryan <Vincent.Ryan at sun.com>:
>> I hear ya. Sorry for the delay on this. I'll push the fix for OpenJDK today.
>>
> 
> Thanks!  Would this be suitable for OpenJDK6 as well?  CCing the
> jdk6-dev list on that.

Yes. The patch should be applied to OpenJDK6 too.

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