Closed Bug 1128792 Opened 9 years ago Closed 8 years ago

Stop advertising/support P-521 curve in Suite-B-only mode

Categories

(NSS :: Libraries, defect)

Product:
NSS
Component:
Libraries
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: briansmith, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

remove P-521 support from libssl when not built with NSS_ECC_MORE_THAN_SUITE_B
2.10 KB, patch
briansmith
: review?
wtc
Details | Diff | Splinter Review 
NSS has a build time option to either disable all ECC, enable Suite B curves only, or enable all the curves it supports. The "enable Suite B curves only" option enables P-256, P-384, and P-521. But, P-521 is NOT a Suite B curve.

See bug 536389, bug 650338, and bug 325495, and bug 319252 for the easy-to-find historical issues with the P-521 code in NSS.

In order to make auditing NSS-based clients (Firefox in particular) easier, it's better to just disable P-521 so we can concentrate on the actually-frequently-used curves.

The current Google Chrome Canary, based on BoringSSL, has disabled P-521 support as well. See https://boringssl.googlesource.com/boringssl/+/e9fc3e547e557492316932b62881c3386973ceb2.

Internet Explorer also only offers P-256 and P-384.

When we discussed this a year or so ago as part of the cipher suite support changes [1], there seemed to be no disagreement about only supporting P-256 and P-384.

The data at [2] and [3] shows that, in Firefox, the P-521 curve is almost never used.

Finally, I'd like to remove support for the P-521 curve from mozilla::pkix, and it would be better to make this change so that Firefox doesn't advertise a curve it can't verify signatures for.

[1] https://briansmith.org/browser-ciphersuites-01.html
[2] http://telemetry.mozilla.org/#filter=nightly%2F38%2FSSL_AUTH_ECDSA_CURVE_FULL
[3] http://telemetry.mozilla.org/#filter=nightly%2F38%2FSSL_KEA_ECDHE_CURVE_FULL
Hi Brian,

While you are technically correct about P521 not being in suite-B, this NIST curve is widely implemented in most cryptographic products using ECC.
My reading shows AES-256 was implemented as Suite-B because of the lack of hardware implementations of AES-192. (to match P384). 
There is an incorrect assumption by some that P521 is included in Suite-B to match AES-256 and so is part of the set.

If this curve is removed from the mainline (and only working) ECC implementation within NSS, this curve will lost from the web.
It is difficult to make the other 'supported' 25 curves work without other undocumented fixes. Condemning this curve to 'non-Suite-B' NSS configuration is its death.

IMHO NSS ECC has been crippled since 2006 with the redaction of the curve parameters beyond P256, P384 and P521.
As of 2015 it also contains other implementation bugs such as the SuiteB_only key-size check bug which I mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1117297 
I am yet to propose an alternative, but would like to hear wtc's opinion on this. I would say NSS can only work with "SuiteB" mode at build time today.

I am also concerned that the Google overlords have decided to nuke this curve. I just don't see the purpose of doing this without any indication of either a security break or deep implementation errors. It seems everyone is all about the monty's and eddy's these days, and still being battling this through CFRG.
There seems to be no discussion at that link on why this was done. agl?

This is not correct about IE (and Microsoft SChannel) not supporting P521.
IE can use the P521 curve for ECDHE and ECDSA. Works for me W7/IE11 + OpenSSL 1.0.2
Microsoft supports this curve in AD CS (Enterprise PKI), and so some enterprises may have rolled this on certificates.
The use within CNG is documented here: https://msdn.microsoft.com/en-us/library/windows/desktop/bb931355%28v=vs.85%29.aspx
Browser interoperability with this curve for both ECDHE and ECDSA is desirable.
For this reason I do not agree with removing it from mozilla:pkix either.

My opinion is we should not be redacting more curves without good strong reasons. 
I request that this curve not be removed for the sake of satisfying the 'true' definition of Suite-B.
IMHO making life easier for auditing is not a good reason to remove 9 year old functionality that works.
However, I do acknowledge this is somewhat problematic with the codebase.

With an implementation hat on, I agree, P256 and P384 are an order more advanced an implementation than their larger cousin P521.
P521 uses more generic field arithmetic such as direct calls to mpi_sub rather than QWORD optimized ones.
There could easily be side channels lurking here and so should be reviewed.

But this problem is not limited to P521; There are other issues with the 384 bit field arithmetic function that group P384 calls too, 
and that isn't a reason to throw away all 384 bit curves also. More eyes needed, not razor blades.

I mean no disrespect to yourself, wan-teh, adam or ryan. Just stating an opinion. :-)

Thank you,
MiW
(In reply to MiW CryptoCurrency from comment #1)
> If this curve is removed from the mainline (and only working) ECC
> implementation within NSS, this curve will lost from the web.

That is OK, and in fact the desired result. It's hardly ever used now, and the cost of supporting it properly isn't justified by the benefits it brings.

> This is not correct about IE (and Microsoft SChannel) not supporting P521.

I agree MS supports P-521, but in its default configuration, SChannel will not advertise or choose P-521. At https://www.ssllabs.com/ssltest/viewClient.html?name=IE&version=11&platform=Win%2010%20Preview you can see that this is the case even in IE11 on Windows 10.

> Microsoft supports this curve in AD CS (Enterprise PKI), and so some
> enterprises may have rolled this on certificates.
>
> Browser interoperability with this curve for both ECDHE and ECDSA is
> desirable.
> For this reason I do not agree with removing it from mozilla:pkix either.

If you look at the telemetry for last actual Firefox release, only 8 (yes, eight!) out of 1.67 BILLION handshakes used a P-521 curve. This is a good indication that P-521 isn't needed, at least for certificates verified by web browsers.

> IMHO making life easier for auditing is not a good reason to remove 9 year
> old functionality that works.
> However, I do acknowledge this is somewhat problematic with the codebase.
>
> With an implementation hat on, I agree, P256 and P384 are an order more
> advanced an implementation than their larger cousin P521.
> P521 uses more generic field arithmetic such as direct calls to mpi_sub
> rather than QWORD optimized ones.
> There could easily be side channels lurking here and so should be reviewed.

:). Exactly. And, nobody really has time to do this.
 
> I mean no disrespect to yourself, wan-teh, adam or ryan. Just stating an
> opinion. :-)

None taken.

I think the most important thing TLS implementations can do is reduce their attack surfaces. My proposal here is about reducing attack surface. Also, as you noted, the P-521 curve has issues that make it something that nobody would really recommend over the other alternatives available. It's primary merit is that it is listed in a NIST standard, which isn't really a sufficient justification.
Some notes to help you understand this patch:

SSL3_SUITE_B_SUPPORTED_CURVES_MASK is used by ssl3_GetSupportedECCurveMask, which is used to set the negotiatedECCurves variable. ssl3_HandleSupportedPointFormatsXtn contains this logic:

    /* What curves do we support in common? */
    mutualCurves = ss->ssl3.hs.negotiatedECCurves &= peerCurves;
    if (!mutualCurves) { /* no mutually supported EC Curves */
        goto loser;
    }

This patch doesn't remove support for certificates with P-521 keys. It only remove support for P-521 ECDH(E) operations. A certificate verification library (like mozilla::pkix in Firefox) to reject P-521 certificates if it chooses to do so. This patch makes it so that we're no lying in the TLS handshake about which curves we'd accept in signatures. (It is unfortunate that there aren't separate extensions for negotiating supported curves for ECDHE and ECDSA, but I'd recommend dropping P-521 for both regardless.)
Attachment #8558657 - Flags: review?(wtc)
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #3)
> This patch doesn't remove support for certificates with P-521 keys. It only
> remove support for P-521 ECDH(E) operations.

Actually, it doesn't exactly do that either. It makes it so that the client doesn't advertise P-521 support, and it makes it so that we ignore P-521 in the peer's supported curves extension. But, AFAICT, we'd still negotiate P-521 ECDHE as long as the peer doesn't send a supported curve extension.
Hi,

It seems that MS added P521 back into its active Ciphersuite (CS) list (they tag each CS with a curve internally!?) with KB2992611 (fix for CVE-2014-6321 MS14-066 'winshock') on at least Windows 7.
This was discussed at Qualsys forum here: https://community.qualys.com/thread/13996

So this is a recent addition (Nov 2014). 
I find this unusual to make changes to ClientHello for a RCE patch; passively observable vulnerability shibboleth. :-(

The big difference here is that Windows allows a runtime registry key to en/disable this curve. This slash and burns it in NSS.

So we either go with G and nuke it; or go with MS and keep it.
Frankly you make a good case and I have to say I'm coming around to this reductionist philosophy. :-)
The additional analytics that https://bugzilla.mozilla.org/show_bug.cgi?id=1130653 will will show its actual use for ECDSA.
It would be good to have a look through the CT logs for more evidence of this curve in the wild too.

I think removing it from the CH will cause the remote peer to drop connection post CH if this curve is used for an ECDSA certificate.
I noticed this behavior with OpenSSL when trying to use secp256k1 ECDSA cert, i would always get a  
SSL routines:SSL3_GET_CLIENT_HELLO:no shared cipher:s3_srvr.c:1360: from OpenSSL s_server if the browser didnt send this specific curve in EC ext.

I don't believe its valid to negotiate ECDHE at all if the EC ext is missing from the CH, even if ECDHE-* is included the CS list.
One could force a peer to negotiate over a weak curve that is supported in peer's code but not acknowledged as supported by that peer in the EC ext.

Anyway, fwiw i retract my original reservations, the continued existence of this curve just adds complexity.
Lets clear out the cobwebs for donna and hope she brings some bigger siblings with her to replace it.
Target Milestone: 3.18 → 3.18.1
Target Milestone: 3.18.1 → 3.19
This missed 3.19, please set a new target milestone if you have a good estimate.

(3.19.1 target milestone will soon be available once bug 1158958 is fixed.)
Target Milestone: 3.19 → ---
I just saw a Chromium bug related to the removal of curve P-521:
https://code.google.com/p/chromium/issues/detail?id=478225

Let me look into this.
IMO secp521r1 should remain supported for everyone interested in using non-backdoored curves.

Look at http://safecurves.cr.yp.to/rigid.html and ask yourselves if you really want to be limited to curves that are manipulatable and susceptible to secret attacks.
(In reply to lcf from comment #8)
> Look at http://safecurves.cr.yp.to/rigid.html and ask yourselves if you
> really want to be limited to curves that are manipulatable and susceptible
> to secret attacks.

I think you are confusing E-521 with P-521. P-521 is similar to P-256 and P-384 in that respect, right?
While I appreciate the implication (both here and on the Chrome bug tracker - http://crbug.com/477623 ) that the Firefox and Chrome maintainers are collaborating with the NSA to backdoor people's crypto, I'm afraid you've somewhat misunderstood Dr. Bernsteins' arguments about crypto design. While he certainly praises P-521 in some ways, it also doesn't meet the criteria for rigidity that he's set out here - http://safecurves.cr.yp.to/rigid.html - so which piece of advice of his would you like to cite?

Namely, it lacks rigidity because the coefficients for P-521 are generated using an unexplained seed for the hash, thus making it manipulable; the seed is d09e8800 291cb853 96cc6717 393284aa a0da64ba from FIPS 186-4, the same place the P-256 seeds came from.

Curious, isn't it, that Dr. Bernstein didn't list his criticisms of P-521's rigidity on that page. Perhaps he's trying to promote curves mathematically backdoored by the NSA.

I hope you see how specious the argument is about NSA collusion, as well as personally insulting. Also, the comparison of P-521 vs P-384 depends on a lot of external factors - which has received optimizations and which hasn't. You can compare with NSS to get different speed results, you can compare on different architectures (e.g. ARM) to get different speed results. You're just not making an informed comparison for the arguments at hand, but cherrypicking examples.

Hopefully this explains more of the reasoning in a way that won't lead you to imply collusion or compromise.
@Ryan Sleevi: forgive me, I didn't try to insult anyone. 

I am not yet able to verify EC properties by myself (still learning ECC), so I have to rely on sources like DJB publications for now. I haven't found P-521 on DJB list, so it seemed to be more secure (status: unlisted) than P-384 (status: manipulatable) to me. And then P-521 was faster than P-384 (1764 op/s vs 1340 op/s) on one of most popular architectures (amd64), so conclusion was pretty clear: P-521 should stay.

Some kind of collusion was just simplest logical explanation for attempts of removal faster and seemed-to-be-more-secure P-521 while leaving support for P-384. NSA backdooring different areas (from standards to firmware) is a fact, so I cannot exclude they're trying to influence open-source projects as well - especially when we take into account how many people on this planet would be able to detect subtle ECC backdoors and that developers living in the US are under NSA jurisdiction (gag orders are real).
All of the NIST curves are explicitly called out in the definition of "Manipulable" in http://safecurves.cr.yp.to/rigid.html - because the function given *is* the one used for the NIST Curve Generation (for all of the 'NIST curves' - which P-521 is one of them)

And before you get too hyped up on the criteria Dr. Bernstein listed, you should also read his related work - http://safecurves.cr.yp.to/bada55.html - which was showing about how even with specific criteria, you can still find room for playing games.

As for "one of the most popular architectures (amd64)", I think you'd find that's not true for either Firefox or for Chrome. Windows' users (where both browsers dominate orders of magnitude more than any other OS) are notoriously behind the hardware curve, and mobile browsers (where both are investing in the future) are entirely different architectures (generally, arm). Even in the server space, you'll find that amd64 systems are barely the majority if examined at large.

So yes, these decisions are made on careful calculations about the cost of performance, complexity, and security. P-521 does not offer compelling benefits over P-384 (they're both more expensive than P-256 - see https://www.imperialviolet.org/2010/12/21/eccspeed.html for the discussion about this), while not offering compelling benefits (see https://www.imperialviolet.org/2014/05/25/strengthmatching.html for the discussion about this).

I appreciate that you're just learning about ECC, and you care about these topics, but it may help to present things as genuine questions, rather than implications and assumptions.

Hopefully this has addressed why comment #8 doesn't apply (P-521 meets the same criteria set out for 'backdoored') and why, independent of that claim (which is quite contentious in the industry of well-respected security engineers, on solid technical grounds rather than any sort of NSA affiliation), there's still compelling reason to remove P-521 support.
According to Adam, P-521 is optimized in OpenSSL more than P-384 because it was easier to optimize than P-384 is: http://www.ietf.org/mail-archive/web/cfrg/current/msg06198.html. That doesn't mean that P-521 is inherently faster.

AFAIK, P-521 was specified with the same techniques, by the same people, with the same problems, as P-256 and P-384.

As far as my own motivation for wanting to get rid of P-521 goes, you can read a little bit about it in the first section of this: https://briansmith.org/GFp-0. Note in particular the my desire to implement curves that seem less prone to manipulation is a primary factor. Another factor is that small "Internet of Things" devices benefit from having fewer algorithms to implement, due to hardware constraints. I hope you can see that these explanations are simpler than conspiracy theories.
@Ryan & Brian: for now we can choose only between P-256, P-384 and P-521, because they're only 3 curves that are supported in every TLS implementation:
https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations#Supported_elliptic_curves

Now, taking math a bit aside, and assuming all three NIST curves are backdoored and may be exploited by NSA folks knowing some of its properties we can see P-256 is enough for protection information labeled as SECRET, and P-384 for TOP SECRET: https://www.nsa.gov/ia/programs/suiteb_cryptography/
This would also mean that more people could know about weakness in P-256 than in P-384. And the same reasoning may work in the other direction - if P-521 is backdoored, then even smaller circle should be allowed to know any secret attack against it. That logic alone is enough for me to stick with P-521 until I am able to verify your statement about P-384 being as secure as P-521 on the math ground.

Mind that all 3 of you (Ryan, Brian and Adam) are US citizens, and as core participants of open-source crypto libraries you're all pretty natural targets to NSA if they wanted to weaken crypto around the world. And I bet they want it pretty much, so please forgive me my lack of faith in your good will every time I read about disabling strong crypto - either when you speak about removal of P-521, or blocking AES-256 (both CBC and GCM):
https://code.google.com/p/chromium/issues/detail?id=442572 - reported by rsleevi, isn't it?
Strong symmetric cipher shouldn't be a problem (for NSA) when key exchange is under control, though.

Forgive me my conspiracy theories (again ;P), but number of active NSA programs (starting with BULLRUN) doesn't leave me much choice. When I see browser from Canada (BB10) supporting both P-521 and AES-256-GCM without any problem, and no single PC browser from US able to do the same in 2015 - what should I think, guys? That you're promoting browsers giving ssl_error_no_cypher_overlap on securely configured servers for my own good?

Please also mind some of us here might aim at military grade encryption, level 8 in ECRYPT II terms:
http://www.keylength.com/en/3/

Maybe it's something that should be left for end-user to choose? Folks that are OK with lower security standards could use limited set of cipher suites, and those who want secure environment could choose full set. But it should be decision on the application level, then, not limitation of the NSS library (IMO).
This is a bug worth fixing but I don't have the time to finish this, so unassigning myself.
Assignee: brian → nobody
IMO, these discussions show that there are disagreements on which curves are best to use . Different applications may have different needs. Firefox is not the only consumer of NSS.

Today, Oracle still builds NSS with 25 curves - with NSS_ECC_MORE_THAN_SUITE_B, which still builds if you checkout this version of ecl-curve.h :

https://hg.mozilla.org/projects/nss/file/63133a0fc438/security/nss/lib/freebl/ecl/ecl-curve.h

Mozilla/RH build with the "light" version that only supports 3 curves - P256, P384, P521.
I think given that NSS is a library, and it has more than one consumer, it is appropriate for it to include support for many curves.

Up to now, the curve support has been entirely dictated by ecl-curve.h - and all curves are supported for all parts of NSS - PKIX cert verification, and ECDH/ECDHE key exchange.

It may be long overdue to add runtime configuration support so that applications can decide which curves they want to use. Just like we have APIs for enabling/disabling TLS cipher suites, we should have APIs for enabling/disabling support for specific curves. NSS can still have whatever defaults it wants - be it 2 curves, 3 curves, or more. But it should allow other consumers that want more curves to make that choice.
If anything you should be going the other way and adding more curves, I really hate telemtry because it has led to some bad decisions been made since its introduction.

What happens if e.g. suddenly a good attack is discovered on the 3 curves used in firefox (and the now only 2 in chrome), a server administrator is strangled as they cannot change due to lack of support in the web browser.  More choice is supreme, really the full set of curves should be compiled into the browser.

I am waiting for brainpool and ed25519.  Especially the latter but I expect chrome wont add it because its not in firefox, and firefox wont add it because its not in chrome, both sets of developers seem obsessed with "simplifiying" the codebase.

Then we have the lack of DNS DANE support which is a much better way of authenticating certificates over the poor HPKP system.  I guess Brian did some research and said look only 1% of people are using DNSSEC that means dont add support. :)
I'm inclined to mark this WONTFIX.  We have recently audited our curves and we intend to remove all other than the four we use from NSS proper: 25519, P-256, P-384 and P-521.  Based on this, we have no reason to believe that P-521 is worse than the alternatives.  It's also in moderately wide use, and we don't need a compatibility drop (see bug 1306003 for an example).

We are interested in EdDSA and the goldilocks(448) stuff, as well as 4Q, but it's a matter of engineering priorities.

Tim, is this a fair assessment?  Can we just close this?
Flags: needinfo?(ttaubert)
Yeah, I agree. Marking as WONTFIX, please see comment above.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(ttaubert)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: