471715 - Add cert to nssckbi to override rogue md5-collision CA cert

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Daniel Veditz [:dveditz]]

At 25c3 a paper was presented about md5 collisions where the presenters created a valid but forged intermediate CA cert signed by a trusted root in major browsers. Although the risk from this one expired cert may be small, it is not zero; we should add a replacement cert that removes trust from it.

As it happens Nelson has created a cert that does just that:
https://bugzilla.mozilla.org/attachment.cgi?id=354880

This needs to get added to the built-in cert store.

Paper is here: http://www.win.tue.nl/hashclash/rogue-ca/

Test site is
https://i.broke.the.internet.and.all.i.got.was.this.t-shirt.phreedom.org/

Without the replacement cert you get an overridable ssl error (expired, unless you set your clock to Aug 2004). With the replacement cert you get a hard SSL error that you can't set an exception for.

Comment 1

[Jeff Walden [:Waldo]]

Is it really necessary for this to be hidden?  The need seems dubious to me given 2004-ness.

Comment 2

[Jesse Ruderman]

Fixing bug 471723 might be simpler and more robust.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

This is a request to add a new cert to NSS's built-in list of CA certs.
I think that, procedurally, the right way to do this is to have Frank
first approve this idea.  There is already a bug filed against the 
CA Certificates component that can serve this purpose, so I will 
change this bug to be an NSS bug, and mark it as blocked by bug 471586
(which I will reopen).

Comment 5

[Daniel Veditz [:dveditz]]

(In reply to comment #0)
> With the replacement cert you get a hard SSL
> error that you can't set an exception for.

Someone who really, really, had to see the test site could manually set the trust bits on the pre-installed cert.

Comment 6

[Daniel Veditz [:dveditz]]

Waiting on a call on bug 471586 before we can call this one a blocker. (the CA component doesn't support client blocking flags)

Comment 7

[Daniel Veditz [:dveditz]]

Frank says "OK" in bug 471586... Nelson, can you turn the cert into an appropriate certdata patch so we can get this into FF 3.0.6?

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

As noted in comment 0, I have produced a CA cert that (presently) has the 
effect of overriding the rogue CA cert announced in December.  

This bug requests that that certificate be added to nssckbi, the trusted 
certs list for NSS, as an UNTRUSTED cert, with trust flag 
nssTrustLevel_NotTrusted.  As such, I believe this bug should be assigned 
to Kai, because he maintains the root CA list.  I review patches for that list.

Note that this is at best a short term solution.  For reasons I won't 
elaborate here, the right long term solution involves the resolution of  
bug 470994 (which see).  I wish I could elevate the priority of that work.
When that bug is completed, then the long term solution for this CA will be
to take the real rogue CA cert and install it in nssckbi (replacing this 
"replacement" cert) with that same NotTrusted flag.

Comment 9

[Kai Engert [:KaiE:]]

Attachment: patch v1

like this?

addbuiltin -n "Forged Certificate 25c3" -t ,, < /tmp/333054c.der >> certdata.txt

manually edited certdata.txt:
changed CKT_NETSCAPE_TRUST_UNKNOWN
     to CKT_NETSCAPE_UNTRUSTED

gmake generate

increased version number of roots module

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 357067 [details] [diff] [review]
patch v1

we may want to alter the nickname. 
I'd suggest adding "DO NOT TRUST THIS". 
But that aside, this looks like it should work.

Comment 11

[Kai Engert [:KaiE:]]

(In reply to comment #10)
> we may want to alter the nickname. 
> I'd suggest adding "DO NOT TRUST THIS". 

Seems unnecessary.
The nickname I used is only shown in the raw NSS database, when used with raw NSS tools.

Our application code derives its own label for display purposes from the certificate, I see "MD5 Collisions Inc. (http:...)" in the UI.

Comment 12

[Kai Engert [:KaiE:]]

Went to 
https://i.broke.the.internet.and.all.i.got.was.this.t-shirt.phreedom.org/

Having system date set to August 2004.

Testing Firefox 3.1 beta

without patch:
page loads fine (shows content)

with patch:
Error code: sec_error_bad_signature
(can not add exception)

Comment 13

[Wan-Teh Chang]

Comment on attachment 357067 [details] [diff] [review]
patch v1

I suggest using "MD5 Collisions rogue CA 25c3" or
"MD5 Collisions forged CA 25c3" as the nickname.
Otherwise, I can't tell what this cert is by just
looking at certdata.txt.

Comment 14

[Daniel Veditz [:dveditz]]

Patch fixes this for me, too.

Like Wan-Teh I'd like something that shows up in the client to say the cert is bogus. I'm not sure what we use for determining a cert is a replacement but I'm guessing we can't change the CN -- unfortunate since that is what appears in the UI. Could we add an O= field ("FORGED CERT--UNTRUSTED" maybe?) so that at least that could be seen if people double-click to view the cert?

If you Edit the trust bits on the new built-in cert then it's a fully-working (expired) CA.

Comment 15

[Kai Engert [:KaiE:]]

The UI string gets derived at runtime from the cert.

Dan, if you want a different UI, we'll have to repeat all steps (including Nelson's steps) in order to produce a new cert. PSM's current logic displays the cert's CN field as a label by default (and falls back to other fields if CN is missing).

Your proposal to add O= requires that Nelson produces a new cert.

Wan-Teh's proposal is limited to the technical list at the source code level, and the name displayed when looking at NSS with low level tools.

Dan, do you want to wait for this? If yes, we'll need to agree on the exact strings to be used for the new cert, before repeating the work.

Finally, I'm worried this string work is not helpful for most users. The contents of the cert obviously won't get localized. Therefore only users who can speak english will have a chance to notice this embedded warning anyway.

Comment 16

[Kai Engert [:KaiE:]]

FYI there are tryserver builds with the patch here:

https://build.mozilla.org/tryserver-builds/2009-01-14_17:[e-mail]-bug471715test1/

I propose we proceed using the existing patch, only adding Wan-Teh's low level nickname comment.

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

In reply to comment 14, a replacement cert must have the same exact subject
name in its entirety, with no additions, deletions or changes.  If any change
it made, the cert will not be identified as the issuer.

Comment 18

[Kai Engert [:KaiE:]]

Attachment: Patch v2

In order to address Wan-Teh's request, I had to repeat the patch creation procedure, because the string length is generated into the C file...

I used
  addbuiltin -n "MD5 Collisions Forged Rogue CA 25c3" -t ,,
and did the trust hand editing etc.

I also realized that we forgot to increase the ckbi version number in both places, fixed.

Comment 19

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 357235 [details] [diff] [review]
Patch v2

r=nelson
Glad you caught that mistake in nssckbi.h!

Comment 20

[Wan-Teh Chang]

Comment on attachment 357235 [details] [diff] [review]
Patch v2

r=wtc.  Thanks, Kai.

Comment 21

[Kai Engert [:KaiE:]]

fixed

cvs commit: Examining .
Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.52; previous revision: 1.51
done
Checking in certdata.txt;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.txt,v  <--  certdata.txt
new revision: 1.51; previous revision: 1.50
done
Checking in nssckbi.h;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.h,v  <--  nssckbi.h
new revision: 1.18; previous revision: 1.17
done

Comment 22

[Samuel Sidler (old account; do not CC)]

This was fixed in 1.9.0.6 by bug 473835.

Comment 23

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.6 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009011604 GranParadiso/3.0.6pre.

If I set my date to 8/16/2004 and check https://i.broke.the.internet.and.all.i.got.was.this.t-shirt.phreedom.org/ with Firefox 3, I get their happy page. Same check with the 1.9.0.6 build above, and I get Error code: sec_error_bad_signature. Both give errors with a current date as well.

Comment 24

[Alexander Sack]

is there anything we can do for 1.8 branches here?

Comment 25

[Samuel Sidler (old account; do not CC)]

(In reply to comment #24)
> is there anything we can do for 1.8 branches here?

We can probably do the same thing... Apply this patch to NSS and do an NSS rev for 1.8.0 and 1.8.1. I'm not sure what version of NSS they're using though. And, specifically, don't most distros ship NSS/NSPR systemwide? We'd probably want to rev the systemwide version of NSS not the one in Firefox.

Comment 26

[Nelson Bolyard (seldom reads bugmail)]

Most Linux distros are using hopelessly down-rev versions of NSS.

Comment 27

[Nelson Bolyard (seldom reads bugmail)]

Or worse, the distro's copy of NSS is commonly modified in some way that 
makes Mozilla's own distributions of Firefox unable to use the distro's NSS.  
One distro changes the names of the shared libraries.  Another prepends the 
string "nss" to all symbols exported from NSS shared libraries.

Comment 28

[Alexander Sack]

(In reply to comment #25)
> (In reply to comment #24)
> > is there anything we can do for 1.8 branches here?
> 
> We can probably do the same thing... Apply this patch to NSS and do an NSS rev
> for 1.8.0 and 1.8.1. I'm not sure what version of NSS they're using though.
> And, specifically, don't most distros ship NSS/NSPR systemwide? We'd probably
> want to rev the systemwide version of NSS not the one in Firefox.

I am mostly concerned about fixing the mozilla cvs tree here; distros have to do their homework based on what we did then.

Comment 29

[Alexander Sack]

(In reply to comment #27)
> Or worse, the distro's copy of NSS is commonly modified in some way that 
> makes Mozilla's own distributions of Firefox unable to use the distro's NSS.  

Not really relevant to this bug, but anyway: The good news about this is that - as promissed - ubuntu is now back to "upstream" library SOnames. Previously you could use builds done against mozilla nss on ubuntu; now you should also be able to use ubuntu builds with mozilla nss.

Comment 30

[:Gavin Sharp [email: [e-mail]]]

As I understand it this was fixed on the 1.9.1 branch by bug 473835. Kai: please confirm?

Comment 31

[Kai Engert [:KaiE:]]

(In reply to comment #30)
> As I understand it this was fixed on the 1.9.1 branch by bug 473835. Kai:
> please confirm?

confirming

Comment 32

[Kai Engert [:KaiE:]]

Attachment: Patch v2 for NSS 3.11 branch

The previous patch v2 has two reviews, I conclude it is ok to land it on the 3.11 branch, too. In particular as we have attempted to keep the root list on both branches in synch (they are currently, besides this addition).

Here is a patch for the 3.11 branch, it's nearly identical.

The only difference, I have made the respective change to the version number.

Comment 33

[Kai Engert [:KaiE:]]

Comment on attachment 358918 [details] [diff] [review]
Patch v2 for NSS 3.11 branch

Patch added to 3.11 branch:

Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.36.24.15; previous revision: 1.36.24.14
done
Checking in certdata.txt;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.txt,v  <--  certdata.txt
new revision: 1.37.24.14; previous revision: 1.37.24.13
done
Checking in nssckbi.h;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.h,v  <--  nssckbi.h
new revision: 1.14.2.8; previous revision: 1.14.2.7
done

Comment 34

[Alexander Sack]

Thank you Kai, could you create a tag that we can use for 1.8.1.21 client.mk?

Comment 35

[Kai Engert [:KaiE:]]

(In reply to comment #34)
> Thank you Kai, could you create a tag that we can use for 1.8.1.21 client.mk?

NSS_3_11_10_WITH_CKBI_1_67_RTM

This is the recent 3.11.10 release, plus this fix to the root list in addition.

Note I'm not aware of anyone having tested 3.11.10 with Firefox 2.0.x already. Feedback is welcome.


Out of curiousity, are you still backporting to the old Mozilla-1.8.0 branch / Firefox 1.5? If yes, what NSS version are you currently using there?

Comment 36

[Alexander Sack]

So, we have:

on 1.8.1: NSS_3_11_9_RTM
on 1.8.0: NSS_3_11_5_RTM

Comment 37

[Samuel Sidler (old account; do not CC)]

Kai: Can you file a new bug for moving 1.8.1 to 3.11.10? I'd like to track that separately. If we're taking a new NSS (beyond something like NSS_3_11_9_WITH_CKBI_1_67_RTM) I want to make sure it gets enough testing on our end and ensure we have a good idea of exactly what changed in that NSS release.

Likewise, we can look at moving 1.8.0 up to whatever is needed to get this change in, but we should track that in a new bug -- the same bug as the 1.8.1-specific bug if we're intending to move 1.8.0 to 3.11.10.

Comment 38

[Alexander Sack]

(In reply to comment #37)
> Kai: Can you file a new bug for moving 1.8.1 to 3.11.10? I'd like to track that
> separately. If we're taking a new NSS (beyond something like
> NSS_3_11_9_WITH_CKBI_1_67_RTM) I want to make sure it gets enough testing on
> our end and ensure we have a good idea of exactly what changed in that NSS
> release.
> 
I filed bug 476157 ... Kai, could you give a summary of changes between .9 and
.10 there? Thanks!

Comment 39

[Kai Engert [:KaiE:]]

I don't have such a list, but I have commented in your new bug 476157

Comment 40

[Paolo Milani]

How is blacklisting this one certificate a solution? I do not know the details, but it seems that any skilled person can generate another such rogue certificate through an md5 collision.

I think a more solid solution would be to consider all certificates that have an md5 hash in their trust path as using weak crypto, and treat them as such.

Comment 41

[Al Billings [:abillings - ex-MoCo]]

Paolo, that would be nice but given the number of sites using them, it isn't going to happen overnight.

Comment 42

[Wan-Teh Chang]

We can consider banning MD5 and MD2 intermediate CA certs
sooner.  I collected some stats from Google Chrome's Dev
channel users (users of weekly trunk builds, so a biased
sample) last month at
http://dev.chromium.org/developers/md5-certificate-statistics

Comment 43

[Al Billings [:abillings - ex-MoCo]]

I suggest opening a new bug for this new issue rather than overloading this bug for one particular issue.

Comment 44

[Nelson Bolyard (seldom reads bugmail)]

I think Paolo wants Bug 483113.

Comment 45

[Paolo Milani]

Thanks for all the replies (even though I came to the party 1 month late!)

I agree we probably need a new bug for this. Should I open it? I know nothing of the mozilla code, so I would not be able to give any information on the HOW.

A fix for bug 483113 would definitely help for clueful users. At least I would know what to recommend to a web banking web site. Instead of saying: DNS is broken, certificates are too, just give up, I could at least tell them to recommend this configuration tweak to their customers. But I think a better solution would be to really treat md5 as the weak crypto that it is.

From the stats Wan-Teh provided, as a mitigating strategy it would be possible right away to at least not allow md5s in the intermediate steps of the certificate chain (that is, for signing an intermediate CA's certificate). There are only 58 out of 496090 SSL sessions using md5 there (as opposed to 33134 out of 496090 in total). This would greatly increase the cost of launching this type of attack, since a new md5 collision would be needed for each victim site (and one md5 collision takes 18 hours on 200 PS3s in the paper...).

Still, I think users have a right to know that the sites they are using have weak security. Maybe a gradual increase in strictness on the GUI side would help in the transition phase. Start with only an unthreatening warning message, and escalate over several months to the totally annoying behavior firefox has for self-signed certs. I am thinking firefox here but I expect that is not the only impacted software so I can't really propose anything specific (which is why I am not filing a new bug yet, maybe someone better informed can do that).

Comment 46

[Nelson Bolyard (seldom reads bugmail)]

Paolo, this bug is not the right place for this discussion.  The discussion
on Mozilla's policy on MD5 has already taken place in Mozilla's crypto 
newsgroup (and mailing list).  You should read the archives of the discussion.
You may restart the discussion there, if you wish, but a bug is not the place
to try to persuade people to change the policy.

Comment 47

[Kai Engert [:KaiE:]]

I wish we had documentation, how to generate such kind of override certs :(
I don't know how Nelson did that.
I suspect he didn't use a tool, but might have used C code to call NSS APIs to create it.

Comment 48

[Kai Engert [:KaiE:]]

Never mind, now I understand what was done (binary editing).

Comment 49

[Wan-Teh Chang]

For future reference, the "MD5 Collisions Forged Rogue CA 25c3" certificate
that we added to certdata.txt differs from the real certificate in two places.

1. The serial number changed from 65 (0x41) to 66 (0x42).

2. The last digit of the not-before and not-after UTC times changes from
'0' (ASCII code 0x30) to '1' (ASCII code 0x31).

In pretty-print form:

Real cert:

        Serial Number: 65 (0x41)
        ...
        Validity:
            Not Before: Sat Jul 31 00:00:00 2004
            Not After : Thu Sep 02 00:00:00 2004

Cert added to certdata.txt:

        Serial Number: 66 (0x42)
        ...
        Validity:
            Not Before: Sat Jul 31 00:00:01 2004
            Not After : Thu Sep 02 00:00:01 2004

The cert added to certdata.txt is newer than the real cert and
will be favored by NSS in (classic) certificate verification,
but it has an invalid signature because the above three bytes in
tbsCertificate were changed.