Stop honoring digital signatures based on MD5 hashes

[Miron Cuperman]

In http://www.win.tue.nl/hashclash/rogue-ca/ , a CA cert was created that is accepted by Firefox. This allows any web site to be impersonated.

See upstream https://bugzilla.mozilla.org/show_bug.cgi?id=471539 .

This may also apply to other PKI enabled packages in Ubuntu that accept MD5, including Thunderbird, Evolution, and anything that depends on openssl.

[Nelson-bolyard (nelson-bolyard)]

Created attachment 354834
patch v1 for NSS trunk

I believe we only want to disable MD5 in certificate signatures, and
not (yet) more broadly than that. I believe this patch does that.
More testing is needed. Bob, please review.

Of course, we will also want to stop creating certificates with MD5 based
signatures in certutil, but that can come later.

[Kai Engert (kaie)]

I found a sample site that currently uses MD5
https://ssl247.com/legal.php

Nelson asked me to apply the patch and test how Firefox behaves.
The result:

Secure Connection Failed
An error occurred during a connection to ssl247.com.
Peer's certificate has an invalid signature.
(Error code: sec_error_bad_signature)

[Eddy-nigg (eddy-nigg)]

Please create a meaningful error code.

[Ben-bucksch (ben-bucksch)]

What about MD2? Yes, that's still used, surprisingly.

[Miron Cuperman (devrandom)]

In http://www.win.tue.nl/hashclash/rogue-ca/ , a CA cert was created that is accepted by Firefox. This allows any web site to be impersonated.

See upstream https://bugzilla.mozilla.org/show_bug.cgi?id=471539 .

This may also apply to other PKI enabled packages in Ubuntu that accept MD5, including Thunderbird, Evolution, and anything that depends on openssl.

[Nelson-bolyard (nelson-bolyard)]

A number of additional suggestions have been made for how to address this
problem, including:

1) Add a new error code explaining that the cert uses an insecure algorithm
(requires changes to PSM also)
2) also disallow MD2 (easy enough)
3) Since the vulnerability only applies to certs whose serial numbers are
predictable, only apply the limitation to certs whose serial numbers are
"short" (e.g. less than 65 bits), on the grounds that larger serial
numbers are very likely to be random
4) make this rejection of old hash algorithms configurable (e.g. allow it
to be turned on or off)
5) invent a callback API by which applications can supply a function that
decides whether to accept or reject the signature based on the hash algorithm
and any other info it can glean from the certificate.

[Dveditz (dveditz)]

I was going to suggest 4) in particular.
 * we can't realistically turn off md5 support today in a shipping
   browser, but will want to as the current certs expire or are replaced.
 * Some concerned users will appreciate the ability to be more cautious
   in the meanwhile. Assuming they know how to set this option they
   know how to unset it should they encounter a site they really have
   to visit. This will be a hard failure; users can't add an exception
   for certs invalid for this reason, right?
 * SHA1 may someday fall to a similar attack. The NIST has started
   the process to define a SHA-3 so they must expect SHA-2 to fail
   eventually as well.

If we disallow MD2 (which IMO we should) what happens to the roots that are self-signed using MD2? A couple of them expire soon anyway but this might clean out the rest.

[C1-mozilla (c1-mozilla)]

It seems that eliminating MD5 is not so onerous, if the result is a jump to the "add security exception" flow. Maybe have a streamlined version of this flow?

[Phr-mozilla (phr-mozilla)]

*** Bug 471586 has been marked as a duplicate of this bug. ***

[Nelson-bolyard (nelson-bolyard)]

The patch attached to this bug will affect signatures on certs and CRLs.

[Rrelyea (rrelyea)]

Comment on attachment 354834
patch v1 for NSS trunk

r+

[Rrelyea (rrelyea)]

After reading the paper, I realized I misremembered. It was Lentra's 2005 collision paper I was remembering, which evidently used MD5. What the chaos paper did was show a practical example of this attack using real CAs.

SHA-1 is fine for now, though we should continue to push the use of SHA-2 when possible. EV already requires SHA-1 and requires SHA-2 by 2010.

Anyway, it seems this should be a reasonable patch.

bob

[Rob-comodo (rob-comodo)]

Bob,

> EV already requires SHA-1

That's correct, for Intermediate CA Certificates and end-entity EV Certificates.

For self-signed Root CA Certificates, MD5 is allowed until 2010. (IINM, NSS always ignores the signatures on the self-signed Root CA Certificates in its trust list).

> and requires SHA-2 by 2010.

Incorrect. SHA-1 is currently allowed post-2010, but the EV Guidelines also say...
"* SHA-1 SHOULD be used only until SHA-256 is supported widely by browsers used by a substantial portion of relying parties worldwide."

[Dveditz (dveditz)]

(In reply to comment #7)
> It seems that eliminating MD5 is not so onerous, if the result is a jump to
> the "add security exception" flow.

This patch creates a fatal error that does not allow an exception.

Allowing exceptions would not be an improvement, it would only further train users to bypass SSL errors and let sites get away with continuing to use weak certs.

[Jwalden+bmo (jwalden+bmo)]

Somewhat tangential, but will NSS ever decide to stop recognizing a particular hash algorithm, preventing even the possibility of configurable behavior? I for one would want MD5 configurable in the short term (maybe a couple years) and off permanently after that. (It's probably a good thing my somewhat hardline position isn't the consensus opinion here. :-) )

[Timeless-bemail (timeless-bemail)]

afaict, most NSS things are guarded by SSL_CipherPolicySet/SSL_SetPolicy.
You can disable/enable SSL2, SSL3, I believe even TLS, although I haven't
actually tried to do that.

I was looking around to see how hard it would be to use SSL_CipherPolicySet to
implement this such that people could turn off MD2, MD5, and SHA1 for testing
purposes.

I need to ask someone to explain why we have code which still calls
SSL_SetPolicy even though the api says it's deprecated.

[Alexander Sack (asac)]

waiting for upstream solution on this.

[Alexander Sack (asac)]

NSS is shipped by firefox package in dapper.

[Steve Langasek (vorlon)]

This is an important bug for us to see fixed in Ubuntu 8.04, but it should not be tied to the Ubuntu 8.04.2 milestone which is due out in just 2½ weeks. More regression-testing than that is warranted for such a regression-prone fix.

[Ben-bucksch (ben-bucksch)]

US CERT writes: <http://www.kb.cert.org/vuls/id/836068>
> Do not use the MD5 algorithm
> Software developers, Certification Authorities, website owners,
> and users should avoid using the MD5 algorithm in any capacity.
> As previous research has demonstrated, it should be considered
> cryptographically broken and unsuitable for further use.

I propose to
* Announce now that we will drop MD5 in 1 or 3 months
* Apply the patch in a future security release

I think that waiting one more 1-2 years more is too long. If somebody finds a way that does not require two random parts (signed cert determined by attacker and attackers matching faked cert), but only one (take one of the already signed ones and create a matching fake cert), it's entirely over.

[Dveditz (dveditz)]

There is no sign anyone is close to coming up with a preimage attack, as opposed to this refinement of the collision attacks known for several years. In addition to the hash weakness this attack also took advantage of weaknesses in a particular CA's process which have now been fixed. We do not need to rush into invalidating up to 30% of known SSL sites (by the researcher's numbers). A year gives current certs time to expire. I'd say drop them in "3.2" nightlies and betas (now? six months?) but don't flip the switch on shipping Firefox before a year.

Meanwhile if we had this user-configurable (prefs?) we'd be prepared should someone come up with a better attack, or an attack on SHA1 which seems eventually likely (especially if there's some kind of a breakthrough which leads to a preimage attack).

Is this code in the part of NSS that is FIPS certified? IIRC there's a current push to get 3.12 certified and it'd be nice if this functionality either got certified or was in code that wouldn't invalidate the certification.

[Marcausl (marcausl)]

Plea from a user. Just give me a warning if the cert chain contains a weak (e.g. md5) cert and let me decide what to do, just as you let me decide what to do when the url doesn't match the certificate.

I'm smart enough to tell the difference between some site I'm just browsing and my bank, and treat the exceptions differently.

The warning could be either a pop-up or a new color on the security bar.

[Schaedpq2 (schaedpq2)]

My 2 cents: Please don't use more colours to code states than strictly necessary. They are anyway quite unobstrusive and difficult to note and a significant percentage of people are in some way or another colour-blind. Asking the user _before_ transmitting any data to the site is definitely preferable.

[Nelson-bolyard (nelson-bolyard)]

In reply to comment 17,
This patch affects code that is not inside of the FIPS boundary.

I think that if One browser unilaterally disables MD5, and other browsers
do not, in the short term, it will merely drive users to other browsers.
So, it seems best (IMO) for the browsers to act together in concert on this.
I think that's feasible. The different browsers have been remarkably
cooperative in this area, especially within the CA-Browser forum.
But this begs some questions about older browsers.
Will we update FF2?
Will we expect MS to update IE6?

[Rrelyea (rrelyea)]

Kai would have to comment on the idea of doing warnings. I don't think it's a good idea. Either we consider MD5 safe enough or we don't. As a rule we should not be asking users questions that the user can't reasonably answer.

RE disabling: I agree with Nelson. I think Browsers need to act in concert. The critical browsers are all part of the CAB, making a suggestion to Sunset MD5 (after a reasonable period... like several months and a very public announcement) in that forum is probably a good idea. The only major browser not in the CAB is Safari. They should be contacted separately.

I also think we should decide when we want to reject all MD5 only signatures. MD5 is now at the point (security wise) MD4 was when NSS decided not to implement it at all (MD4 is now completely broken).

bob

[C1-mozilla (c1-mozilla)]

Currently, having a certificate that is not signed by an authority known to the browser is a warning. The user can decide to "add a security exception" and still establish communication.

If MD5 is used in the cert chain, and MD5 is considered insecure, then I believe the same situation obtains - i.e. the target certificate should be considered unsigned. So I believe the user should still be able to "add a security exception".

In sum, having an insecure signature and having an unrecognized signer should be handled in the same way.

[Johnath (johnath)]

(In reply to comment #22)
> Currently, having a certificate that is not signed by an authority known to the
> browser is a warning. The user can decide to "add a security exception" and
> still establish communication.
>
> If MD5 is used in the cert chain, and MD5 is considered insecure, then I
> believe the same situation obtains - i.e. the target certificate should be
> considered unsigned. So I believe the user should still be able to "add a
> security exception".

Even with a self-signed or otherwise unverified certificate, the promise is that if you decide add the exception, you continue to gain the benefits of TLS in terms of integrity and confidentiality. That isn't true if the cert is using technology which can be compromised by an attacker (though this is not the scenario that was presented in the 25c3 talk, I think it's clear as Bob R says, that md5 is in decline.)

I do agree strongly with Dan though - if we had per-algorithm prefs to toggle support, it would allow us to respond more fluidly to similar incidents in the future, while at the same time giving people in strange legacy environments the ability to re-enable support while migrating. Nelson, is that much pain, or otherwise ill-considered?

[Kai Engert (kaie)]

NSS should implement a function that allows an application to toggle the behavior with or without Nelson's patch.

void NSS_HashPrefsMD5InSigsSetDefault(PRBool enabled);

{{ A more general API can be done later (which could allow to control the use of various hashes in various contexts). I believe such an API would require more thinking, and this additional thinking shouldn't delay us at this point. Something similar to existing SSL_CipherPrefSetDefault(). Maybe a function
  enum HashAndContext {
    hac_MD5_in_CertAndCRLSignatures,
    hac_MD5_everywhere,
    hac_SHA1_in_CertAndCRLSignatures,
    ...
  };
  void NSS_HashPrefSetDefault(enum HashAndContext, PRBool enabled);
}}

NSS should add this function in NSS 3.12.3
PSM should add a preference, in both Firefox 3.0.x and Firefox 3.1.x,
(about:config only) which can be used to disable MD5 / activate the patch.

I propose to continue to have MD5 allowed for now, but this gives us the opportunity to change the default to disallowed at any time (with security updates), and allows security conscious users to disable it right away.

I propose a new error code gets added to NSS, like SEC_ERROR_UNSAFE_ALGO (where the meaning UNSAFE is based on runtime defaults).

Now let's say the runtime configuration is "forbid MD5" and the user gets an error page. Should the user be able to override the error, just like other errors? I think it should, but it would require much more logic to be added to both NSS and PSM. Similar to today's HandshakeCallback/AuthCertificateCallback functions, the NSS would need to call back into PSM and asks whether an override is active... More APIs to define...

[Wan-Teh Chang (wtc-google)]

I support adding a new NSS error code for weak/broken crypto
algorithms. SEC_ERROR_WEAK_CRYPTO?

[Nelson-bolyard (nelson-bolyard)]

Responding to a lot of things in this bug...

1) Is Mozilla willing to disable MD5 cert signatures and "break the web"
unilaterally, even if it drives users to IE?

Johnathan, as Mozilla's representative to CABForum, have you broached this
subject with the other browser representatives there?

I have so many things on my plate now that I must prioritize my work
carefully. Until I see that either
a) the browsers are willing to act in concert on this, and collectively
announce a date on which they will desupport it, or
b) someone speaking for Mozilla says here that Mozilla is willing to
unilaterally "break the web",
I don't think this bug is really urgent.

2) Error code

Whatever error code is set by CERT_VerifySignedDataWithPublicKey is likely
to be overridden by the caller, (see examples below) so I wouldn't fret too
much about which error code it is.
http://mxr.mozilla.org/security/source/security/nss/lib/certdb/crl.c#1565
http://mxr.mozilla.org/security/source/security/nss/lib/certhigh/certvfy.c#180
http://mxr.mozilla.org/security/source/security/nss/lib/certhigh/certvfy.c#635
http://mxr.mozilla.org/security/source/security/nss/lib/pk11wrap/pk11nobj.c#793

3) hash configuration API design

If we're going to make this configurable, then clearly we don't want to
have MD5 be a special case. We want it to be possible to configure all
the hashes. The configuration API needs to allow the caller to specify
which hash it wants to enable/disable.

Then there's the matter of the level of granularity of functional uses
that will be controlled by this API. Presently, we're proposing to be
able to control the acceptability of hashes in the context of verifying
signatures on certificates and CRLs (and probably OCSP responses).

Are those the only operations for which we will ever want to exert such control?

What about:
- SMIME signatures?
- signatures on JAR and XPI files?
- disabling MD5 alltogether?

Here's a first draft proposal for an API.

Two functions, a set and a get. The hash algorithm is specified using a
SECOidTag (effectively an enum, whose value maps to/from OIDs). The data
associated with each SecOidTag is a 32-bit value treated as a bit array.
Initially, only 1 bit is defined, bit 0 (value 0x00000001) which specifies
the use in signatures of certs and CRLs. The API is extensible in that
we have 31 more bits to define as we think of uses for them.

#define NSS_ALG_USABLE_IN_CERT_SIGNATURE 0x00000001

extern SECStatus
NSS_SetAlgorithmPolicy(SECOidTag tag, PRUint32 mask, PRUint32 value);

extern SECStatus
NSS_GetAlgorithmPolicy(SECOidTag tag, PRUint32 *pValue);

Either function may return SECSuccess or SECFailure with the error code
set to SEC_ERROR_UNKNOWN_OBJECT_TYPE if the SECOidTag is out of range.

The Get function outputs the 32-bit value associated with the SECOidTag.
The Set function modifies the stored value according to the following
algorithm:
   policy[tag] = (policy[tag] & mask) | value;

Default value for any policy would be 0xffffffff (enabled for all purposes).

How's that sound?

[Nelson-bolyard (nelson-bolyard)]

Here are a few more details about that API.

The able to policy bits would be initialized when NSS is initialized.

It would have no persistence. Each time NSS is initialized, the
application must then make any changes to the policy table that it wishes
to make.

To disable MD5 for use in certs, the application might use this code:

SECStatus rv;
rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, ~NSS_ALG_USABLE_IN_CERT_SIGNATURE, 0);

[Nelson-bolyard (nelson-bolyard)]

Make that "The table of policy bits would be initialized ...".

[Johnath (johnath)]

(In reply to comment #26)
> 1) Is Mozilla willing to disable MD5 cert signatures and "break the web"
> unilaterally, even if it drives users to IE?
>
> Johnathan, as Mozilla's representative to CABForum, have you broached this
> subject with the other browser representatives there?
>
> I have so many things on my plate now that I must prioritize my work
> carefully. Until I see that either
> a) the browsers are willing to act in concert on this, and collectively
> announce a date on which they will desupport it, or
> b) someone speaking for Mozilla says here that Mozilla is willing to
> unilaterally "break the web",
> I don't think this bug is really urgent.

I have spoken with the CABForum about this issue. The message I gave the CAs there (not all CAs by a long shot, but representing the vast majority of issued certs on the public web) is that while we still need to have our own community discussions about the specifics, we consider MD5 to be on the way out. I told them we were unlikely to kill it tomorrow, but that no CA should be counting on support for MD5 to last. I suggested a general ballpark for retirement at 6-18 months, very much dependent on the state of deployed certificates on the web. I asked the CAs if that was a surprising answer, and a few confirmed that this was as they expected. The rest may need to consult counsel before they can answer a question like that. :)

I have spoken to MS and Opera (Apple is not a member of the CABForum and George Staikos from Konqueror/Torch Mobile was not on the call) and while I am most decidedly not speaking for them, I got the impression that they were considering similar approaches and timelines.

If they weren't though, and we were otherwise satisfied that the state of the deployed web (and the state of MD5) was such that we felt comfortable making the change, I don't feel that we would wait for other browsers to do so first.

Making my policy thoughts here into concrete policy action is not really for this bug, of course. I hope it clarifies the state of my thinking, anyhow. How does that impact your estimation of priority?

> How's that sound?

From a distance, the API description sounds good.

[Timeless-bemail (timeless-bemail)]

A small plea (because I think we made a mistake in filing this bug)

to everyone else: this bug is filed against NSS, and I think the NSS team has made it clear that they're talking about enabling library functionality (which will be the majority of my comment).

Anyone interested in Firefox policy should please ignore this bug and campaign in a newsgroup or a Core/Firefox component. This bug is about NSS which is focussed on the underlying API.

It's unfortunate that the bug wasn't originally written to limit its scope to something like "enable applications to ask NSS to stop honoring certain classes of digital certificates", but that's hindsight.

---

the api looks technically supports the requirements.

but my first read of it totally missed the way you were using the MASK. It's clever and does what's necessary but I really don't like the method name because i was totally mislead.

Below please find what I ended up writing because I couldn't properly read your API:

I'm slightly worried.

if the NSS policy is later: ~FOO_BIT
and we hard code that does SetPolicy( ... ~BAR_BIT ...)
then we've stepped on ~FOO_BIT. This is likely to happen.
So either all example code needs to be of this form:

...
#define NSS_ALG_DEFAULT_POLICY 0xffffffff

SECStatus rv;
PRUint32 policy = NSS_ALG_DEFAULT_POLICY;
rv = NSS_GetAlgorithmPolicy(SEC_OID_MD5, &policy);
if (...FAILED(rv)) ...

policy ^= NSS_ALG_USABLE_IN_CERT_SIGNATURE;
rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, policy, 0);
if (...FAILED(rv)) ...

or you should seriously consider a different API.

There are two possibilities.

One:

#define NSS_ENABLE_ALGORITHM PRBIT(31)
#define NSS_DISABLE_ALGORITHM 0 * NSS_ENABLE_ALGORITHM

#define NSS_ALG_USABLE_IN_CERT_SIGNATURE PRBIT(0)

extern SECStatus
NSS_SetAlgorithmPolicy(SECOidTag tag, PRUint32 policyChanges);

extern SECStatus
NSS_GetAlgorithmPolicy(SECOidTag tag, PRUint32 *pValue);

to disable an algorithm:

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_DISABLE_ALGORITHM | NSS_ALG_USABLE_IN_CERT_SIGNATURE);

to enable an algorithm:

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ENABLE_ALGORITHM | NSS_ALG_USABLE_IN_CERT_SIGNATURE);

Yes, I'm aware my api wastes one bit (it's also obviously possible to have distinct enable/disable methods).

---
yes I eventually understood what the mask was doing.

But it's complicated, with your API, to enable something one has to do:
rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ALG_USABLE_IN_CERT_SIGNATURE, NSS_ALG_USABLE_IN_CERT_SIGNATURE);

or

rv = NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_ALG_USABLE_IN_CERT_SIGNATURE, 0xffffffff);

neither of these make me happy.

--

While I'm commenting, this may sound stupid, but are we really going to be miserly and try to use 1 bit to represent both CRLs and Certs?

iiuc CRLs merely contain lists of serial numbers. While I'd hope that the CAs would be using SHA1 (or better [and iiuc better isn't supported]), and while I can understand that a similar risk of collision for an MD5 signature over a random sequence of serial numbers exists, as I haven't heard much information about the current state of CRLs, I think at least tentatively it'd be nice from an API perspective for it to be it...

[Sean McNamara (smcnam)]

The essence of this bug is not one that we can fix in software. The problem is not an implementation detail; it is inherent in the algorithms used to comply with the X.509 Cert spec. The implementations thereof are not buggy with respect to this "bug" report; any implementation that correctly behaves according to the spec will exhibit the same behavior. Meanwhile, breaking compliance with the spec will cause any services which depend upon MD5-signed X.509 certificates to fail in some way.

The real fix rests entirely on the shoulders of CAs who continue to use MD5 hashing as an encryption technique. If Ubuntu software implements automatic refusal of MD5-signed PKI certificates, many users' workflow will be disrupted. Additionally, regression testing is not based solely on a particular domain of computers or deployments; a _comprehensive_ regression test would require attempting to establish HTTPS with _every_ website out there. And you can't do that, because many websites are within LANs, VPNs, etc.

In the example of Firefox, it might be acceptable to consider any MD5-signed X.509 certificates to be invalid, which then displays the "Get me out of here!" vs. "Add an exception..." buttons, just as if someone used themselves as the root CA, i.e. self-signing, which is never accepted as a valid certificate already.

I would caution against implementing anything, or accepting upstream "bug fixes" to any services, which does not provide the user an alternative (be it a configuration file, command line switch, environment variable, or GUI) to accept the certificate even if it is invalid. Otherwise, this will disrupt existing, legitimate, production services based on a fairly far-fetched attack which must be hand-crafted for each individual victim. But as long as users have some documented way of bypassing a warning about this problem, that is fine.

[Matthew Lye (matthew.lye)]

 I agree with Sean McNamara on this one. Implementing a fix for this other that the suggested Firefox equivalent would be a rash move that would disable far to many valid services based on a hand crafted attack that required considerable expertise and hardware.

Until a move is made to the SHA family of hashes by the certifying authorities there is no practical way to resolve this issue.

[Nelson-bolyard (nelson-bolyard)]

Comment on attachment 354834
patch v1 for NSS trunk

This patch unconditionally excludes MD5 from certificate and CLR signatures. We're going to make it conditional.

[Miron Cuperman (devrandom)]

A successful attack would mean that the attackers would have a rogue CA. They would then be able to generate a bogus certificate for any site without any additional resources. This issue should therefore be considered critical in my opinion. The benefit to an attacker would justify using considerable resources in generating the rogue CA cert.

I do think that the end-user should be able to override the security weakness warning.

[Matthew Lye (matthew.lye)]

I do think that the end-user should be able to override the security weakness warning. - Miron Cuperman

How do we mitigate that a large group of CA's still use MD5 instead of using the SHA certs. We cannot force a change on them and all we would do is remove potentially harmful services from users.

MD5 is still a valid hashing function, just not a valid cryptographic function. We should be pushing as a community for CA's to move to SHA based hashes which are still cryptographically sound.

[Miron Cuperman (devrandom)]

As long as the end-user can override the warning, they are still able to continue with existing workflows based on their judgment.

Also, the warning would encourage CA's and web sites to move to SHA more quickly.

[Steve Langasek (vorlon)]

As the consensus appears to be that we will track upstream on this, it's unrealistic to set any milestone deadline for the fix. If and when upstream moves, it will still be appropriate to address this, including in SRU.

[Nelson-bolyard (nelson-bolyard)]

Timeless's comment 30 tells me that something about the API I proposed in
comment 26 is confusing. It it confused Timeless, then I expect it will
confuse other less-capable developers too. Trouble is: I don't know what's
confusing about it, so I don't know how to improve it. It seemed dirt
simple to me.

Here's an alternative definition of the interface. It's very slightly
different from the previous one, but maybe enough to eliminate the confusion?

#define NSS_USE_ALG_IN_CERT_SIGNATURE 0x00000001 /* CRLs and OCSP, too */
#define NSS_USE_ALG_IN_CMS_SIGNATURE 0x00000002 /* used in S/MIME */

extern SECStatus
NSS_SetAlgorithmPolicy(SECOidTag tag, PRUint32 setBits, PRUint32 clearBits);

extern SECStatus
NSS_GetAlgorithmPolicy(SECOidTag tag, PRUint32 *pValue);

Either function may return SECSuccess or SECFailure with the error code
set to SEC_ERROR_UNKNOWN_OBJECT_TYPE if the SECOidTag is out of range.

The Get function outputs the 32-bit value associated with the SECOidTag.
Default value for any algorithm would be 0xffffffff (enabled for all purposes).

The Set function modifies the stored value according to the following
algorithm:
   policy[tag] = (policy[tag] & ~clearBits) | setBits;

Examples:
1) enable MD5 in cert/CRL/OCSP signatures
    NSS_SetAlgorithmPolicy(SEC_OID_MD5, NSS_USE_ALG_IN_CERT_SIGNATURE, 0);

2) disable MD2 in cert/CRL/OCSP signatures
    NSS_SetAlgorithmPolicy(SEC_OID_MD2, 0, NSS_USE_ALG_IN_CERT_SIGNATURE);

Timeless, Do you find this API definition to be clearer?
If not, please write (here or in email) explaining what was/is unclear.
Thanks.

[Ben-bucksch (ben-bucksch)]

You could separate set and clear into two functions.
Also, the terms were confusing to me. Maybe call the param "algo" instead of "tag", and "usage" instead of "setBits", and I'd call the functions differently:

NSS_GetAlgorithmUsage(SECOidTag algo, PRUint32 *usageFlags);
NSS_EnableAlgorithmUsage(SECOidTag algo, PRUint32 usageFlags);
NSS_DisableAlgorithmUsage(SECOidTag algo, PRUint32 usageFlags);

(The more abstract something is, the harder it is to comprehend what you mean, here "tag" and "policy", because they could mean anything.)

[Alexander Sack (asac)]

the consent among firefox/nss developers seems to be that disabling MD5 would break too much of the internet;

however, nss will provide a mechanism to disable MD5 through preferences in future, but we won't use this bug to track the progress on that issue. Marking won't fix.

[Nelson-bolyard (nelson-bolyard)]

Created attachment 365856
proposed patch v2 for NSS trunk - for discussion

Some questions for reviewers:
1) is PRUint32 enough? Should we make it PRUint64?
2) Should the GET/SET API allow only one bit to bet set/cleared/tested
in a single call?
3) Should we forcibly disallow setting reserved (undefined) policy flags?

[Wan-Teh Chang (wtc-google)]

Why don't we implement the "secure by default" principle
and just ban MD2 and MD4? CERT_VerifyCert would fail with
the SEC_ERROR_WEAK_CRYPTO error if any cert in the cert
chain (except the root CA cert) is signed with RSA-MD2
or RSA-MD4? Then apps, when upgrading to the new NSS,
will automatically get verification failures on those
certs. The apps that still want to support RSA-MD2 and
RSA-MD4 can choose to stick with NSS 3.12.2 or add
code to handle the SEC_ERROR_WEAK_CRYPTO error.

If all apps need to both upgrade to the new NSS and
add calls to NSS_SetAlgorithmPolicy to get the secure
behavior, that'll be a big hassle and that's even
more boilerplate code one needs to write to use NSS.

[Nelson-bolyard (nelson-bolyard)]

Wan-Teh, I gather you would prefer the patch to do this instead?

    /* initialize any policy flags that are disabled by default */
    xOids[SEC_OID_MD2 ].notPolicyFlags = ~0;
    xOids[SEC_OID_MD4 ].notPolicyFlags = ~0;
    xOids[SEC_OID_PKCS1_MD2_WITH_RSA_ENCRYPTION ].notPolicyFlags = ~0;
    xOids[SEC_OID_PKCS1_MD4_WITH_RSA_ENCRYPTION ].notPolicyFlags = ~0;
    xOids[SEC_OID_PKCS5_PBE_WITH_MD2_AND_DES_CBC].notPolicyFlags = ~0;

Any thoughts (objections) to using yet-another environment variable to
override that?

[Wan-Teh Chang (wtc-google)]

I haven't read the patch. I only read the prototypes of
the new public functions. What I proposed in comment 36
would not require adding new functions. I proposed a new
error code, SEC_ERROR_WEAK_CRYPTO, for our signature and
certificate verification functions. If any signature
involved (except the signature in root CA certs) uses a
weak hash algorithm such as MD2 and MD4, the signature or
certificate verification function would fail with
SEC_ERROR_WEAK_CRYPTO.

I think my proposal is difficult to implement right, so
you should feel free to continue work on your proposal.
But it's important that your proposal also be "secure
by default". I suspect that's what your code in comment 37
does. Only the apps that still want to support RSA-MD2
and RSA-MD4 signatures have to do extra work.

[Nelson-bolyard (nelson-bolyard)]

In reply to Wan-Teh's comment 38,
I have no objection to adding a new error code. But a new API function is
definitely going to be needed. If all the calls to the relevant function
came from the application, and not from elsewhere within NSS libraries,
then I would agree that new functions are not required. But there are
numerous places inside NSS libraries where NSS makes decisions based on
the outcome of a signature verification, and if we make all MD2 signatures
fail unilaterally, without any way for the application to revert that, then
those places will all fail with no recourse for the application.

Finally, we come to the question of what to do by default. There are
clearly two schools of thought here, and two groups with diametrically
opposing values. One school says "be secure by default, even if that breaks
compatibility and breaks existing deployments of NSS-based applications".
The other school says "Don't ever break existing deployments of NSS-based applications in a way that requires modification (or even recompilation) of
application code to return the application to a working state." If we
change the default behavior, we break the users in the latter camp.

Presently, the sponsors of NSS development fall into both camps. If we
change the default behavior, the only solution to the problem for the users
who cannot (or refuse to) rebuild their programs (the only one that
immediately occurs to me) is to continue to use environment variables to
select the old or new modes of operation.

I'm beginning to think that NSS needs one environment variable that says
"In all cases where a new behavior might break compatibility, choose the
old behavior", rather than (or in addition to) continuing to have separate
environment variables for every such case, as documented in
https://developer.mozilla.org/En/NSS_reference:NSS_environment_variables

[Ben-bucksch (ben-bucksch)]

> NSS needs one environment variable that says
> "In all cases where a new behavior might break compatibility,
> choose the old behavior"

I think that's a bit too broad. Who wouldn't want compat, after all?
I'd rather make one "NSS_I_WANT_TO_BE_INSECURE". If set to "YES", you'd allow MD2 for verification. I can't see why anyone would want that, though.

[Nelson-bolyard (nelson-bolyard)]

Created attachment 366681
Patch v3 - for review

This patch incorporates most of the comments previously discussed in this
bug. Rather than invent another new NSS error code this late, I found one
that is already used to mean "Hash algorithm not allowed for this purpose"
and reused it. It is SEC_ERROR_INVALID_ALGORITHM.

In this patch, I use NSS_ALLOW_WEAK_SIGNATURE_ALG as the overriding
environment variable. There was strong opposition to a single envar
that means "override all compatibility changes since 3.11", so I won't
do that.

Review invited from any NSS peer.

[Bugzilla+nospam (bugzilla+nospam)]

Comment on attachment 366681
Patch v3 - for review

I am not opposed to the idea of this patch, but I don't think we should keep this environment variable forever. This should be only a stopgap measure to give time to people to migrate their infrastructure away from using MD2.

I haven't had time to get up to speed on just how broken MD2 is. But I would prefer a stronger wording for the environment variable chosen. Something like :

NSS_MAKE_VULNERABLE_TO_MD2_ATTACK_FIX_YOUR_CERTS_BEFORE_OCT_01_2009_OR_YOU_WILL_BE_SORRY .
I know it's quite a mouthful, but it drives the point home. Other strong wordings are also welcome.

The idea would be that we remove support for this environment variable in any NSS release made after that date - there would be no way to turn MD2 back in any later release of NSS .

Another suggestion also - depending on how bad the attacks on MD2 are, we may also want to consider removing the MD2 implementation from softoken at a set date in the future.

[Rrelyea (rrelyea)]

Comment on attachment 366681
Patch v3 - for review

r+

I do have some caveats:

1) we define a policy to turn off SMIME signatures, but there is not code for that.

2) we probably should have a policy for *all* signatures (including those the application validates using the VFY_ directly).

3) ssl3 uses PK11_Verify directly. Currently it either uses or includes a SHA1 hash, but if SHA1 is turned off by policy, SSL would continue to accept client auth certs even though they use sha1 or sha1+md5 signatures. I supposed we can live with that until TLS 1.2 where we have hash agility, but we should make the conscious decision to do so rather than do so by default.

Anyway this patch provides important step forward, and is self contained, so an r+

bob

bob

[Bugzilla+nospam (bugzilla+nospam)]

Comment on attachment 366681
Patch v3 - for review

I spoke to Nelson for a while about this since I was lacking background.

I have several concerns.

A) This patch only allows MD5 to be turned off programmatically. MD5 is on by default, and cannot be turned off by environment variable. I think this may be an issue for some end customers at Sun who want to be secure, as many Sun applications will not make use of the programmatic method to turn off MD5.

One way to improve this would be to have finer-grain environment variables, ie. a way to turn off each algorithm separately, rather than having one environment variable controlling a set of 4 algorithms.

B) This may not be the best place to deal with MD2 and MD4.

My understanding is that MD2 and MD4 are far more broken than MD5, which is why the patch turns them off by default for signature verification.

My concern is that this may not go far enough. Depending on how much computational power is needed to break MD2 and MD4, even short-term hashes may be at risk.

As Bob pointed out in his review, there are other uses of these algorithms that this patch doesn't address, and didn't intend to address.

Plugging these other holes would require changes to several more places.

Because of the additional security concerns on MD2 and MD4, I think it would be preferable to throw the switch for them in a different place.

Since we have a layered architecture in NSS, we should take advantage of it to turn off all uses of MD2 and MD4.

The 2 places I suggest we add a switch (with MD2 and MD4 off by default) are :

1) in pk11wrap

This should take care of all uses by higher levels, including for certificate signatures, S/MIME, VFY APIs, and of course pk11wrap APIs.

The algorithms should be off by default.

It should be possible to turn them back on either by environment variable or programmatically, as in this patch. For the environment variable method, we may want fine-grain control for each algorithm, not one variable controlling a bunch of algorithms together.

2) in softoken or freebl

This is needed to prevent use of MD2 and MD4 by programs such as Java apps that only use the PKCS#11 but not the higher-level NSS libraries.

softoken already has a method of passing initialization parameters in to C_Initialize. This may not be acceptable to many applications, however.

So we may want to also use the same environment variables as I propose to use in pk11wrap to turn off these algorithms in softoken/freebl.

If we add those switches for MD2 and MD4 at those 2 other layers I propose, then there is no longer a need for controlling MD2 and MD4 in the certificate verification layer as in this patch, as it would be redundant.

At the time being, we probably would only require an environment variable at for MD5 in that layer.

[Nelson-bolyard (nelson-bolyard)]

Julien, there is no agreement that MD5 should be universally disabled,
or even disabled for all cert signatures. By some estimates, turning off
support for MD5 now would break 10-20% of https web sites.

Mozilla should work with the other browser vendors to have them all agree
to publish a drop-dead date for MD5, and we can drop it at that point.

The scope of this bug is specifically about the use of certain hashes in
signatures processed by NSS's higher layers. The scope excludes other
non-signature uses of these hashes, and excludes use of Softoken without
using libNSS3.

It's true that the patch here does nothing to prevent programs that use
Softoken by itself from using Softoken's MD2 and MD4/5, but that is outside
the scope of this bug.

If you want to disable all uses of MD2, 4 & 5 in Softoken, please file a
bug for that.
If you want to disable all uses of MD2, 4 & 5 for things other than
signatures, please file a bug for that.

[Bugzilla+nospam (bugzilla+nospam)]

Nelson,

Re: comment 46,

1) I am well aware that there is no current agreement on disabling MD5 universally. I don't have a problem with MD5 remaining turned on by default in the library for the time being. But the MD5 attacks have already been published, and I think there needs to be a way for end-users to turn it off earlier if they wish to. Individual users don't necessarily have to wait for all application vendors to decide to change their default. Your patch allows MD5 to be turned off only programmatically, and so Sun customers who cannot use the programmatic method (or users of any other products that haven't had a code change yet) will not be able to if they wish. There needs to be an environment variable support for MD5 in signatures in NSS somehow, and it's missing from your patch.

2) Regarding MD2 and MD4, I'm OK with filing a separate bug to turn MD2 and MD4 off in 2 other places. The point I was trying to make, and why I made it as part of my review comments to your patch, is that we should turn MD2 and MD4 off in these 2 places *instead* of turning them off in the signature verification code.
If we choose to make these MD2/MD4 changes in a separate bug, then the scope of this bug would be reduced to be about MD5 in signatures only, as it used to be.

[Bugzilla+nospam (bugzilla+nospam)]

I filed bug 482882 about disabling all uses of MD2 and MD4.

[Wan-Teh Chang (wtc-google)]

Comment on attachment 366681
Patch v3 - for review

You should explain why you store notPolicyFlags instead
of policyFlags in privXOid. The reason (for static
initialization to 0 to be the desired default value of
"enabled for all purposes") is not obvious.

[Wan-Teh Chang (wtc-google)]

Comment on attachment 366681
Patch v3 - for review

Another future optimization: the xOids array, which has 303 (the
current value of SEC_OID_TOTAL) elements, is very sparse. By
default we only use 5 elements. Some of the elements will never
be used because those OIDs are not algorithms (e.g, the OIDs
for certificate and CRL extensions).

We should come up with a more space-saving way to store the
policy flags for algorithms.

[Bugzilla+nospam (bugzilla+nospam)]

Bob discovered that not only is there no MD4 implementation in NSS, there is also no PKCS#11 mechanism defined for it. I think that means we shouldn't waste our time trying to "disable" MD4 - it's already effectively disabled today, and cannot be enabled.

I think that invites further comments on attachment 366681 :
a) Any line in that patch relevant to MD4 is probably not needed
b) The environment variable "NSS_ALLOW_WEAK_SIGNATURE_ALG" defined in attachment 366681 really only does one thing - it enables MD2 support . We may want to rename it to "NSS_ALLOW_WEAK_MD2_SIGNATURE_ALG" or something along those lines that contains the word "MD2".

In today's meeting, we discussed the need for fine-grain hash algorithm control with environment variables. This is needed not just to deal with today's attacks on MD2 and MD5, but may also be needed in the future for attacks against SHA-1.

At the same time, we have a short-term need (about one week) for a 3.12.3 release to address the MD2/MD5 issue. If we had a bit more time, having fine grain support would probably be the right thing to do.

I think short-term, we can use a slightly modified version of attachment 366681. I would be OK with it going in with the changes I outlined above in this comment (a and b), plus the addition of another PR_GetEnv call in SECOID_Init to disable MD5. That's probably the quickest path to getting things done.

An alternative is that we define the generic syntax today, but perhaps only implement the subcomponents that we need now (ie. just MD2 and MD5). But it may be hard to get agreement, a patch, review, and checkin by end of next week.
Nevertheless, it shouldn't be brain surgery, so I will take a crack at it.
The environment variable could be defined as follows :

name : NSS_HASH_ALG_SUPPORT

value : comma-separated list of [+/-]ALGNAME

This environment variable would not need to define every algorithm, it would only make changes from the default.

Eg.

NSS_HASH_ALG_SUPPORT=+MD2,-MD5 would enable MD2, which we decided to disable by default, and disable MD5, which we decided to leave on by default.

I think that's a simple enough syntax that can hopefully meet our current and future needs. We can add more algorithms to the supported list in the future.

[Nelson-bolyard (nelson-bolyard)]

Julien, please file a new separate RFE for your fine grained environment
variable suggestion in comment 51.

[Nelson-bolyard (nelson-bolyard)]

In today's weekly NSS meeting, I think we agreed to go forward with the
following steps (in no particular order):

1. Commit the patch attached to this bug, as it establishes a base API
for future controls, and solves an immediate vulnerability.

2. File an RFE to "completely disable certain hashes, not only for use in
signatures, but for all higher level security uses within NSS. Bug 482882.
Note that Bob and I oppose disabling hashes for non-cryptographic purposes.

3. File an RFE that requests that users/admins be able to manipulate
this table using an environment variable with a syntax that allows various
hashes to be selectively enabled/disabled. That is now bug 483113.

[Nelson-bolyard (nelson-bolyard)]

Comment on attachment 366681
Patch v3 - for review

Committed on trunk.

Checking in util/nssutil.def; new revision: 1.9; previous revision: 1.8
Checking in util/secoid.c; new revision: 1.47; previous revision: 1.46
Checking in util/secoid.h; new revision: 1.12; previous revision: 1.11
Checking in util/secoidt.h; new revision: 1.29; previous revision: 1.28
Checking in certhigh/certvfy.c; new revision: 1.69; previous revision: 1.68

[Bugzilla+nospam (bugzilla+nospam)]

Comment on attachment 366681
Patch v3 - for review

OK now that bug 483113 has been resolved.

[Dveditz (dveditz)]

(In reply to comment #53)
> Note that Bob and I oppose disabling hashes for non-cryptographic purposes.

Me three. Lots of old things clients may want to deal with still use MD5 hashes, and it's convenient to be able to get all our hashing done by NSS. We could of course reimplement MD5 in the client if we had to, but that seems like a shame when we've got a big crypto library sitting right there next to us.

[Timeless-bemail (timeless-bemail)]

fwiw, the final NSS_Set API seems clear enough for me, thanks. I'm sorry I didn't comment earlier.

[Johnath (johnath)]

This bug is marked blocking1.9.1. As I understand it, this change was included in the NSS_3_12_3_RC0 tag, which Kai pushed to the 1.9.1 branch in bug 481968 (changeset http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f160224c5221 ). Therefore, I'm marking this bug fixed1.9.1.

Please let me know if I've misunderstood.

Also, just to make sure I'm clear: the effect of this change for Firefox 3.5 is:

a) MD2 (and, trivially given comment 51, MD4) is immediately disabled as a supported hash by virtue of its inclusion here: http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/util/secoid.c#1864

b) We gain the ability to disable other algorithms (e.g. MD5) through the same mechanism

Is that accurate?

[Nelson-bolyard (nelson-bolyard)]

> Is that accurate?

Yes. Plus, we have the ability to disable algorithms through the use of an
environment variable. See Bug 483113.

[Eddy-nigg (eddy-nigg)]

500495 is security sensitive?

[Reed Loden (reed)]

(In reply to comment #60)
> 500495 is security sensitive?

Yes.

[Dveditz (dveditz)]

This was fixed on the 1.9.0 branch by landing the new NSS

[Abillings (abillings)]

Verified for 1.9.0.14 and 1.9.0.13 since the new NSS has been verified to be there.

I'd verify it for 1.9.1 except status1.9.1 wasn't set but a keyword incorrectly used.

[Samuel-sidler+old (samuel-sidler+old)]

(In reply to comment #63)
> I'd verify it for 1.9.1 except status1.9.1 wasn't set but a keyword incorrectly
> used.

This was fixed and verified on 1.9.1 prior to Firefox 3.5 releasing. The keyword was not use incorrectly.

[Abillings (abillings)]

Why is not it not marked "verified1.9.1" if it was verified then?

[Jamie Strandboge (jdstrand)]

This was fixed in 3.12.3, which is available in all releases.