[security-dev 01227]: Re: 6840752: Provide out-of-the-box support for ECC algorithms

Andrew John Hughes gnu_andrew at member.fsf.org
Fri Sep 18 11:28:44 UTC 2009


2009/9/18 Vincent Ryan <Vincent.Ryan at sun.com>:
> Hello again Andrew,
>
> Sorry for the delay getting to your request.
>

No problem.

> Your mechanism to control the inclusion of the SunEC provider looks like a
> fine solution. I've created the following CR:
>
>  http://bugs.sun.com/view_bug.do?bug_id=6882745
>

Thanks.  Pushed: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/39c15c0a71f7

>
>
>
> Andrew John Hughes wrote:
>> 2009/9/10 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>>> 2009/9/9 Vincent Ryan <Vincent.Ryan at sun.com>:
>>>> Hello Andrew,
>>>>
>>>> I realize that you, along with others in the Linux community, are less
>>>> than satisfied with the changeset to provide out-of-the-box support for
>>>> ECC algorithms.
>>>>
>>>> As I mentioned earlier, we were quite constrained in what we could
>>>> openly discuss before we pushed. However, now that we have pushed I
>>>> am eager to fix any problems that I've introduced.
>>>>
>>> Yes, I can understand that to an extent, but I find it hard to believe
>>> that you had to push it before it could even be discussed.  Why could
>>> the same patch that was pushed not have been posted for public review
>>> instead?
>>>
>>> This seems to be a more general issue.  This is endemic behaviour that
>>> I've seen from the majority of Sun engineers working on OpenJDK (there
>>> are thankfully some exceptions) and I've blogged about this in more
>>> detail: http://blog.fuseyism.com/index.php/2009/09/08/im-so-tired/
>>>
>>>> We wish to reconcile the conflicting demands of including an ECC
>>>> implementation for platforms without underlying ECC support with the
>>>> exclusion of the ECC implementation on platforms with underlying ECC
>>>> support. I'd like to solicit input from security-dev on how best to
>>>> achieve this.
>>>>
>>> It's good to hear you're open to changing this.  There is a third
>>> option you've missed; the demand of not wanting ECC support at all.
>>> You'll be aware that there are legal issues from your own discussions
>>> on this within Sun, and the change in direction that occurred.  Not
>>> having ECC support needs to be an option as well.
>>>
>>> The existing ECC implementation already fulfilled two of these
>>> demands; it could be enabled on platforms with ECC support but this
>>> wasn't the default case.  We can make this easier with IcedTea by
>>> detecting NSS at build time and auto-generating the configuration if
>>> the user wishes.  This also can be used to ship it 'out of the box' on
>>> distributions if required; all the distro packager has to do is build
>>> IcedTea with NSS support enabled and then make their binary depend on
>>> it.
>>>
>>> So the real problem here is that Sun's proprietary builds can't ship
>>> it 'out of the box' because they don't know if the system it ends up
>>> on will have NSS and, even if it does, where it will be located.  I
>>> can understand how that's a problem that needs to be fixed, but we
>>> need a way of disabling that.  If the PKCS11 provider is still
>>> suitable, then making building the ec directory would actually be
>>> enough:
>>>
>>> ifndef DISABLE_NSS
>>>  SUBDIRS += ec
>>> endif
>>>
>>> Job done.  A more complex solution is to link against the system NSS
>>> instead of the provided C sources.  I've managed to do this with the
>>> following change:
>>>
>>> diff -r 7a23bfc44c92 make/sun/security/ec/Makefile
>>> --- a/make/sun/security/ec/Makefile     Tue Sep 08 18:03:43 2009 +0100
>>> +++ b/make/sun/security/ec/Makefile     Wed Sep 09 23:50:24 2009 +0100
>>> @@ -153,7 +153,9 @@
>>>   #
>>>   # C and C++ files
>>>   #
>>> +ifndef USE_SYSTEM_NSS
>>>   include FILES_c.gmk
>>> +endif
>>>
>>>   FILES_cpp = ECC_JNI.cpp
>>>
>>> @@ -185,6 +187,11 @@
>>>     OTHER_LDLIBS += $(JVMLIB)
>>>   else
>>>     OTHER_LDLIBS = -ldl $(JVMLIB) $(LIBCXX)
>>> +    ifdef USE_SYSTEM_NSS
>>> +      OTHER_LDLIBS += -Wl,-rpath $(SYSTEM_NSS_DIR) -Wl,-rpath
>>> $(SYSTEM_NSPR_DIR) \
>>> +        -L$(SYSTEM_NSS_DIR) -L$(SYSTEM_NSPR_DIR) -lnssutil3 -lnss3 \
>>> +        -lplds4 -lplc4 -lnspr4 -lsoftokn -lfreebl
>>> +    endif
>>>   endif
>>>
>>>   include $(BUILDDIR)/common/Mapfile-vers.gmk
>>>
>>> but unfortunately, while the resulting sunecc library is dynamically
>>> linked against NSS, it causes HotSpot to segfault in
>>> sun.security.ec.ECKeyPairGenerator.generateECKeyPair(I[B[B)[J.  I'm
>>> still looking into this, I assume there is either some mismatch in the
>>> versions of NSS or local changes in the Sun copy.  As you say, only
>>> part of the library was imported into OpenJDK; does this mean that the
>>> JNI code is not using published interfaces for NSS?
>>>
>>>> Your proposal to supply an NSS config file for the SunPKCS11 provider
>>>> is one approach but what about platforms where an ECC-enabled NSS is
>>>> not present?
>>>>
>>>>
>>> It's only really an idea that works where we have an autoconf wrapper
>>> to detect NSS at build time, and which also allows it to be disabled.
>>> The patch to IcedTea automatically finds out where NSS is installed,
>>> via pkg-config, and writes the config file based on that.  I don't
>>> know of a portable way of doing that in OpenJDK's makefiles as
>>> pkg-config won't be available on all platforms.
>>>
>>> snip...
>>>
>>>>>  * Which version of NSS were these sources pulled from?  Running diff
>>>>> -bu on them, and ignoring the additional copyright headers,
>>>>>  there are still a large number of changes.  I suspect this is
>>>>> because the version is older than my system copy (3.12.3); notably my
>>>>>  testing shows it does not exhibit the bug discussed in
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/security-dev/2009-September/001167.html
>>>>> (which
>>>>>  incidentally is still awaiting review).
>>>> The sources were pulled from OpenSolaris 2009.06.
>>>>
>>> Ok, so which version of NSS does that have?
>>>
>>>>>  * Why was a new provider used instead of the existing
>>>>> sun.security.pkcs11.SunPKCS11 provider?  I noticed this has not be
>>>>> removed, yet
>>>>>  it appears to provide duplicate functionality unless I'm mistaken.
>>>>> This does perhaps mean we could fix the issues with this changeset
>>>>> simply
>>>>>  by allowing the ec subdirectory to be turned off, but there may be
>>>>> something about the new provider I'm missing.
>>>> We introduced the new SunEC provider because we wanted a fast compact
>>>> ECC implementation that we could ship on all platforms. We have not
>>>> bundled all of NSS - only its ECC implementation.
>>>>
>>> Yeah I noticed that.  I suppose the big question is how interchangable
>>> are SunEC and PKCS11?  Could we just turn off SunEC, given we already
>>> have NSS support via PKCS11?  If so, just making SunEC optional would
>>> solve this IMO.
>>>
>>>>
>>>>>  * I notice that a number of algorithms are replaced with NULL.  I
>>>>> assume there is some (perhaps legal) reason for this?
>>>> Which ones?
>>>>
>>> This is the change I'm referring to:
>>>
>>> /* mapping between ECCurveName enum and pointers to ECCurveParams */
>>>  static const ECCurveParams *ecCurve_map[] = {
>>>     NULL,                               /* ECCurve_noName */
>>> -    &ecCurve_NIST_P192,                 /* ECCurve_NIST_P192 */
>>> -    &ecCurve_NIST_P224,                 /* ECCurve_NIST_P224 */
>>> +       NULL,                   /* ECCurve_NIST_P192 */
>>> +       NULL,                   /* ECCurve_NIST_P224 */
>>>     &ecCurve_NIST_P256,                 /* ECCurve_NIST_P256 */
>>>     &ecCurve_NIST_P384,                 /* ECCurve_NIST_P384 */
>>>     &ecCurve_NIST_P521,                 /* ECCurve_NIST_P521 */
>>> -    &ecCurve_NIST_K163,                 /* ECCurve_NIST_K163 */
>>> -    &ecCurve_NIST_B163,                 /* ECCurve_NIST_B163 */
>>> -    &ecCurve_NIST_K233,                 /* ECCurve_NIST_K233 */
>>> -    &ecCurve_NIST_B233,                 /* ECCurve_NIST_B233 */
>>> -    &ecCurve_NIST_K283,                 /* ECCurve_NIST_K283 */
>>> -    &ecCurve_NIST_B283,                 /* ECCurve_NIST_B283 */
>>> -    &ecCurve_NIST_K409,                 /* ECCurve_NIST_K409 */
>>> -    &ecCurve_NIST_B409,                 /* ECCurve_NIST_B409 */
>>> -    &ecCurve_NIST_K571,                 /* ECCurve_NIST_K571 */
>>> -    &ecCurve_NIST_B571,                 /* ECCurve_NIST_B571 */
>>> -    &ecCurve_X9_62_PRIME_192V2,         /* ECCurve_X9_62_PRIME_192V2 */
>>> -    &ecCurve_X9_62_PRIME_192V3,         /* ECCurve_X9_62_PRIME_192V3 */
>>> -    &ecCurve_X9_62_PRIME_239V1,         /* ECCurve_X9_62_PRIME_239V1 */
>>> -    &ecCurve_X9_62_PRIME_239V2,         /* ECCurve_X9_62_PRIME_239V2 */
>>> -    &ecCurve_X9_62_PRIME_239V3,         /* ECCurve_X9_62_PRIME_239V3 */
>>> -    &ecCurve_X9_62_CHAR2_PNB163V1,      /* ECCurve_X9_62_CHAR2_PNB163V1 */
>>> -    &ecCurve_X9_62_CHAR2_PNB163V2,      /* ECCurve_X9_62_CHAR2_PNB163V2 */
>>> -    &ecCurve_X9_62_CHAR2_PNB163V3,      /* ECCurve_X9_62_CHAR2_PNB163V3 */
>>> -    &ecCurve_X9_62_CHAR2_PNB176V1,      /* ECCurve_X9_62_CHAR2_PNB176V1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB191V1,      /* ECCurve_X9_62_CHAR2_TNB191V1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB191V2,      /* ECCurve_X9_62_CHAR2_TNB191V2 */
>>> -    &ecCurve_X9_62_CHAR2_TNB191V3,      /* ECCurve_X9_62_CHAR2_TNB191V3 */
>>> -    &ecCurve_X9_62_CHAR2_PNB208W1,      /* ECCurve_X9_62_CHAR2_PNB208W1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB239V1,      /* ECCurve_X9_62_CHAR2_TNB239V1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB239V2,      /* ECCurve_X9_62_CHAR2_TNB239V2 */
>>> -    &ecCurve_X9_62_CHAR2_TNB239V3,      /* ECCurve_X9_62_CHAR2_TNB239V3 */
>>> -    &ecCurve_X9_62_CHAR2_PNB272W1,      /* ECCurve_X9_62_CHAR2_PNB272W1 */
>>> -    &ecCurve_X9_62_CHAR2_PNB304W1,      /* ECCurve_X9_62_CHAR2_PNB304W1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB359V1,      /* ECCurve_X9_62_CHAR2_TNB359V1 */
>>> -    &ecCurve_X9_62_CHAR2_PNB368W1,      /* ECCurve_X9_62_CHAR2_PNB368W1 */
>>> -    &ecCurve_X9_62_CHAR2_TNB431R1,      /* ECCurve_X9_62_CHAR2_TNB431R1 */
>>> -    &ecCurve_SECG_PRIME_112R1,          /* ECCurve_SECG_PRIME_112R1 */
>>> -    &ecCurve_SECG_PRIME_112R2,          /* ECCurve_SECG_PRIME_112R2 */
>>> -    &ecCurve_SECG_PRIME_128R1,          /* ECCurve_SECG_PRIME_128R1 */
>>> -    &ecCurve_SECG_PRIME_128R2,          /* ECCurve_SECG_PRIME_128R2 */
>>> -    &ecCurve_SECG_PRIME_160K1,          /* ECCurve_SECG_PRIME_160K1 */
>>> -    &ecCurve_SECG_PRIME_160R1,          /* ECCurve_SECG_PRIME_160R1 */
>>> -    &ecCurve_SECG_PRIME_160R2,          /* ECCurve_SECG_PRIME_160R2 */
>>> -    &ecCurve_SECG_PRIME_192K1,          /* ECCurve_SECG_PRIME_192K1 */
>>> -    &ecCurve_SECG_PRIME_224K1,          /* ECCurve_SECG_PRIME_224K1 */
>>> -    &ecCurve_SECG_PRIME_256K1,          /* ECCurve_SECG_PRIME_256K1 */
>>> -    &ecCurve_SECG_CHAR2_113R1,          /* ECCurve_SECG_CHAR2_113R1 */
>>> -    &ecCurve_SECG_CHAR2_113R2,          /* ECCurve_SECG_CHAR2_113R2 */
>>> -    &ecCurve_SECG_CHAR2_131R1,          /* ECCurve_SECG_CHAR2_131R1 */
>>> -    &ecCurve_SECG_CHAR2_131R2,          /* ECCurve_SECG_CHAR2_131R2 */
>>> -    &ecCurve_SECG_CHAR2_163R1,          /* ECCurve_SECG_CHAR2_163R1 */
>>> -    &ecCurve_SECG_CHAR2_193R1,          /* ECCurve_SECG_CHAR2_193R1 */
>>> -    &ecCurve_SECG_CHAR2_193R2,          /* ECCurve_SECG_CHAR2_193R2 */
>>> -    &ecCurve_SECG_CHAR2_239K1,          /* ECCurve_SECG_CHAR2_239K1 */
>>> -    &ecCurve_WTLS_1,                    /* ECCurve_WTLS_1 */
>>> -    &ecCurve_WTLS_8,                    /* ECCurve_WTLS_8 */
>>> -    &ecCurve_WTLS_9,                    /* ECCurve_WTLS_9 */
>>> +       NULL,                   /* ECCurve_NIST_K163 */
>>> +       NULL,                   /* ECCurve_NIST_B163 */
>>> +       NULL,                   /* ECCurve_NIST_K233 */
>>> +       NULL,                   /* ECCurve_NIST_B233 */
>>> +       NULL,                   /* ECCurve_NIST_K283 */
>>> +       NULL,                   /* ECCurve_NIST_B283 */
>>> +       NULL,                   /* ECCurve_NIST_K409 */
>>> +       NULL,                   /* ECCurve_NIST_B409 */
>>> +       NULL,                   /* ECCurve_NIST_K571 */
>>> +       NULL,                   /* ECCurve_NIST_B571 */
>>> +       NULL,                   /* ECCurve_X9_62_PRIME_192V2 */
>>> +       NULL,                   /* ECCurve_X9_62_PRIME_192V3 */
>>> +       NULL,                   /* ECCurve_X9_62_PRIME_239V1 */
>>> +       NULL,                   /* ECCurve_X9_62_PRIME_239V2 */
>>> +       NULL,                   /* ECCurve_X9_62_PRIME_239V3 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB163V1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB163V2 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB163V3 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB176V1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB191V1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB191V2 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB191V3 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB208W1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB239V1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB239V2 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB239V3 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB272W1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB304W1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB359V1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_PNB368W1 */
>>> +       NULL,                   /* ECCurve_X9_62_CHAR2_TNB431R1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_112R1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_112R2 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_128R1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_128R2 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_160K1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_160R1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_160R2 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_192K1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_224K1 */
>>> +       NULL,                   /* ECCurve_SECG_PRIME_256K1 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_113R1 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_113R2 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_131R1 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_131R2 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_163R1 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_193R1 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_193R2 */
>>> +       NULL,                   /* ECCurve_SECG_CHAR2_239K1 */
>>> +       NULL,                   /* ECCurve_WTLS_1 */
>>> +       NULL,                   /* ECCurve_WTLS_8 */
>>> +       NULL,                   /* ECCurve_WTLS_9 */
>>>     NULL                                /* ECCurve_pastLastCurve */
>>>  };
>>>
>>>
>>> It could be a NSS version issue, but seems more deliberate to me.
>>> It leaves three curves:
>>>     &ecCurve_NIST_P256,                 /* ECCurve_NIST_P256 */
>>>     &ecCurve_NIST_P384,                 /* ECCurve_NIST_P384 */
>>>     &ecCurve_NIST_P521,                 /* ECCurve_NIST_P521 */
>>>
>>>>> I'm afraid my current impression of this changeset is that it doesn't
>>>>> help us with packaging OpenJDK for GNU/Linux distributions at all, but
>>>>> instead makes things ten times worse as there is now a stale NSS to
>>>>> contend with.  Not only are there the issues with bit rot I alluded to
>>>>> last time, but as you mention in your reply there are legal issues
>>>>> with EC support.  Notably, I've found that Fedora doesn't ship any EC
>>>>> support (https://bugzilla.redhat.com/show_bug.cgi?id=492124) so we'd
>>>>> have to rip this out in packages for that distribution (and it's
>>>>> dubious whether others should be shipping it).  IANAL, so I won't
>>>>> comment further on such issues, but suffice to say this changeset
>>>>> significantly reduces the options for handling NSS support downstream.
>>>>>  In contrast, the existing support in 1.6 is almost ideal, once you've
>>>>> discovered how it works; the distro packager can choose whether to
>>>>> enable support or not and they don't have to worry about rotting
>>>>> security code in OpenJDK.  Maybe I'm missing something but this makes
>>>>> me think this would have been better as a local addition to Sun's
>>>>> proprietary builds rather than adding it to OpenJDK.
>>>>>
>>>>> I try to be as positive as I can about the OpenJDK project, but I'm
>>>>> sorry to say that changesets like this don't help.  I actually find
>>>>> them quite depressing.  As I said in my previous email, there appears
>>>>> to have been no discussion of this change; it was merely committed
>>>>> with no public review and appeared in b70.  Meanwhile, myself and
>>>>> other external contributors have to spend days trying to get replies
>>>>> to emails to even get a simple bug fix in (I've lost count of how many
>>>>> I still have waiting; there must be at least four or five).  That's
>>>>> just not fair and doesn't bode well for a successful community
>>>>> project.
>>>>>
>>>>> Thanks,
>>>>
>>> Cheers,
>>> --
>>> 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
>>>
>>
>> I've added this changeset:
>>
>> http://hg.openjdk.java.net/icedtea/jdk7/jdk/rev/2a1a7fb44226
>>
>> to the IcedTea project's JDK7 forest to solve this issue.  If it looks
>> ok, then give me a bug ID and I'll push it to tl-gate.
>



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



More information about the security-dev mailing list