Closed Bug 1309707 Opened 8 years ago Closed 8 years ago

Distrust new certs chaining up to current WoSign/StartCom roots

Categories

(Core :: Security: PSM, defect, P1)

51 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: kwilson, Assigned: keeler)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [psm-assigned])

Attachments

(3 files, 1 obsolete file)

The remediation that we are considering taking in regards to the deception** by WoSign/Startcom includes the following steps that will require engineering and testing work.

1) Distrust certificates chaining up to the Affected Roots with a notBefore date after October 21. If additional back-dating is discovered (by any means) to circumvent this control, then Mozilla will immediately and permanently revoke trust in the Affected Roots.
-- This change will go into the Firefox 51 release train. 
-- The code will use the subject key id (hash of public key) to identify the Affected Roots, so that the control will also apply to cross-certs of the Affected Roots.

2) Add the previously identified backdated SHA-1 certs chaining up to the Affected Roots to OneCRL.

The Affected Roots are the following 6 root certificates.

WoSign
- CA 沃通根证书 - https://crt.sh/?caid=1450
- Certification Authority of WoSign - https://crt.sh/?caid=1425
- Certification Authority of WoSign G2 - https://crt.sh/?caid=5919
- CA WoSign ECC Root - https://crt.sh/?caid=5969

StartCom
- StartCom Certification Authority - https://crt.sh/?caid=84
- StartCom Certification Authority G2 - https://crt.sh/?caid=239

** https://docs.google.com/document/d/1C6BlmbeQfn4a9zydVi2UvjBGv6szuSB4sMYUcVrR8vQ/edit

The discussion is still in progress, but I am filing the bug now, so that the impacted folks may begin testing, planning, and implementation. With the understanding that there may be some tweaks to this plan.
Assignee: nobody → dkeeler
Priority: -- → P1
Whiteboard: [psm-assigned]
My apologies! I copy-pasted the Root Cert list from the google doc listed above, but I should have double-checked its accuracy!

There are 7 Affected Roots, as follows:


Subject: CN=CA 沃通根证书, OU=null, O=WoSign CA Limited, C=CN
Certificate Serial Number: 50706bcdd813fc1b4e3b3372d211488d
SHA-1 Fingerprint   
16:32:47:8D:89:F9:21:3A:92:00:85:63:F5:A4:A7:D3:12:40:8A:D6
SHA-256 Fingerprint   
D6:F0:34:BD:94:AA:23:3F:02:97:EC:A4:24:5B:28:39:73:E4:47:AA:59:0F:31:0C:77:F4:8F:DF:83:11:22:54

Subject: CN=Certification Authority of WoSign, OU=null, O=WoSign CA Limited, C=CN
Certificate Serial Number: 5e68d61171946350560068f33ec9c591
SHA-1 Fingerprint   
B9:42:94:BF:91:EA:8F:B6:4B:E6:10:97:C7:FB:00:13:59:B6:76:CB
SHA-256 Fingerprint   
4B:22:D5:A6:AE:C9:9F:3C:DB:79:AA:5E:C0:68:38:47:9C:D5:EC:BA:71:64:F7:F2:2D:C1:D6:5F:63:D8:57:08

Subject: CN=Certification Authority of WoSign G2, OU=null, O=WoSign CA Limited, C=CN
Certificate Serial Number: 6b25da8a889d7cbc0f05b3b17a614544
SHA-1 Fingerprint   
FB:ED:DC:90:65:B7:27:20:37:BC:55:0C:9C:56:DE:BB:F2:78:94:E1
SHA-256 Fingerprint   
D4:87:A5:6F:83:B0:74:82:E8:5E:96:33:94:C1:EC:C2:C9:E5:1D:09:03:EE:94:6B:02:C3:01:58:1E:D9:9E:16

Subject: CN=CA WoSign ECC Root, OU=null, O=WoSign CA Limited, C=CN
Certificate Serial Number: 684a5870806bf08f02faf6dee8b09090
SHA-1 Fingerprint   
D2:7A:D2:BE:ED:94:C0:A1:3C:C7:25:21:EA:5D:71:BE:81:19:F3:2B
SHA-256 Fingerprint   
8B:45:DA:1C:06:F7:91:EB:0C:AB:F2:6B:E5:88:F5:FB:23:16:5C:2E:61:4B:F8:85:56:2D:0D:CE:50:B2:9B:02

Subject: CN=StartCom Certification Authority, OU=Secure Digital Certificate Signing, O=StartCom Ltd., C=IL
Certificate Serial Number: 01
SHA-1 Fingerprint   
3E:2B:F7:F2:03:1B:96:F3:8C:E6:C4:D8:A8:5D:3E:2D:58:47:6A:0F
SHA-256 Fingerprint   
C7:66:A9:BE:F2:D4:07:1C:86:3A:31:AA:49:20:E8:13:B2:D1:98:60:8C:B7:B7:CF:E2:11:43:B8:36:DF:09:EA

Subject: CN=StartCom Certification Authority, OU=Secure Digital Certificate Signing, O=StartCom Ltd., C=IL
Certificate Serial Number: 2d
SHA-1 Fingerprint   
A3:F1:33:3F:E2:42:BF:CF:C5:D1:4E:8F:39:42:98:40:68:10:D1:A0
SHA-256 Fingerprint   
E1:78:90:EE:09:A3:FB:F4:F4:8B:9C:41:4A:17:D6:37:B7:A5:06:47:E9:BC:75:23:22:72:7F:CC:17:42:A9:11

Subject: CN=StartCom Certification Authority G2, OU=null, O=StartCom Ltd., C=IL
Certificate Serial Number: 3b
SHA-1 Fingerprint   
31:F1:FD:68:22:63:20:EE:C6:3B:3F:9D:EA:4A:3E:53:7C:7C:39:17
SHA-256 Fingerprint   
C7:BA:65:67:DE:93:A7:98:AE:1F:AA:79:1E:71:2D:37:8F:AE:1F:93:C4:39:7F:EA:44:1B:B7:CB:E6:FD:59:95
The 64 backdated WoSign SHA-1 certs are listed in the "Issue S" section of this report:
https://www.wosign.com/report/WoSign_Incident_Report_Update_07102016.pdf

Then there are the two certs issued by StartCom to Tyro:
https://crt.sh/?id=21427475
https://crt.sh/?id=24879110

Gerv
See Also: → 1309923
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

https://reviewboard.mozilla.org/r/85710/#review84356

I have a sense of unease about not testing that UTF-8 DN, but I'm telling myself that certs with it work in Firefox today, and it's a binary comparison going on, so it must be fine.

Long story short, it's unfortunate that we need to loop through that list of DNs an increasing percentage of the time as realtime advances - at least until we remove the roots, but I don't see an alternative.

LGTM.

::: security/certverifier/StartComAndWoSignData.inc:1
(Diff revision 1)
> +// /C=CN/O=WoSign CA Limited/CN=CA \xE6\xB2\x83\xE9\x80\x9A\xE6\xA0\xB9\xE8\xAF\x81\xE4\xB9\xA6

Interesting; I thought ASN.1 PrintableString's limited alphabet didn't permit UTF-8 characters. I hope nothing chokes on the UTF-8 here.
Attachment #8800898 - Flags: review?(jjones) → review+
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

https://reviewboard.mozilla.org/r/85710/#review84444

::: security/certverifier/NSSCertDBTrustDomain.cpp:750
(Diff revision 1)
> +    }
> +  }
> +  return false;
> +}
> +
> +// If a certificate in the given chain appears to have been issued by one of six

There are seven roots - see bug, comment 1. You'll need to add the last one and update this comment.

::: security/certverifier/NSSCertDBTrustDomain.cpp:755
(Diff revision 1)
> +// If a certificate in the given chain appears to have been issued by one of six
> +// roots that are not trusted to issue new certificates, verify that the
> +// end-entity has a notBefore date before 21 October 2016. If the value of
> +// notBefore is after this time, the chain is not valid.
> +static Result
> +CheckForStartComOrWoSign(const UniqueCERTCertList& certChain)

If the function is called CheckForStartComOrWoSign, it should not return Success if it fails to find StartCom or WoSign. 

If you want to do it that way round, you should call the function CheckIsNotStartComOrWoSign.
(In reply to Gervase Markham [:gerv] from comment #5)
> > +// If a certificate in the given chain appears to have been issued by one of six
> 
> There are seven roots - see bug, comment 1. You'll need to add the last one
> and update this comment.

Ah, good point. There are seven roots but only six distinct distinguished names. I'll update the comment.

> ::: security/certverifier/NSSCertDBTrustDomain.cpp:755
> (Diff revision 1)
> > +// If a certificate in the given chain appears to have been issued by one of six
> > +// roots that are not trusted to issue new certificates, verify that the
> > +// end-entity has a notBefore date before 21 October 2016. If the value of
> > +// notBefore is after this time, the chain is not valid.
> > +static Result
> > +CheckForStartComOrWoSign(const UniqueCERTCertList& certChain)
> 
> If the function is called CheckForStartComOrWoSign, it should not return
> Success if it fails to find StartCom or WoSign. 
> 
> If you want to do it that way round, you should call the function
> CheckIsNotStartComOrWoSign.

The rationale here was to match the style in mozilla::pkix itself. For example, CheckExtendedKeyUsage checks that the certificate is valid with respect to how we process the extended key usage extension, with the important detail that if it isn't even present, the function will return Success (because the checks passed). Perhaps CheckStartComAndWoSignPolicy or similar might be more clear?
(In reply to J.C. Jones [:jcj] from comment #4)
> I have a sense of unease about not testing that UTF-8 DN, but I'm telling
> myself that certs with it work in Firefox today, and it's a binary
> comparison going on, so it must be fine.
> 
> Long story short, it's unfortunate that we need to loop through that list of
> DNs an increasing percentage of the time as realtime advances - at least
> until we remove the roots, but I don't see an alternative.
> 
> LGTM.
> 
> ::: security/certverifier/StartComAndWoSignData.inc:1
> (Diff revision 1)
> > +// /C=CN/O=WoSign CA Limited/CN=CA \xE6\xB2\x83\xE9\x80\x9A\xE6\xA0\xB9\xE8\xAF\x81\xE4\xB9\xA6
> 
> Interesting; I thought ASN.1 PrintableString's limited alphabet didn't
> permit UTF-8 characters. I hope nothing chokes on the UTF-8 here.

That AVA is a UTF8String, but the others are PrintableString. This is basically why I didn't include a test for this case - our test infrastructure supports setting the string type on the distinguished name as a whole but not on a per-AVA basis, which is what we would need to implement here. Since mozilla::pkix does a byte-for-byte comparison and (more importantly) doesn't attempt string prep/decoding/etc., this shouldn't be an issue. If you think it's important I can implement the necessary infrastructure, though.
(In reply to David Keeler [:keeler] (use needinfo?) from comment #7)
> [...] If you think it's important I can implement the necessary
> infrastructure, though.

No, you're good to go as-is. If some CA certs' DNs got modified by an adversarial CA, they wouldn't chain to the roots we have anyway.
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

https://reviewboard.mozilla.org/r/85710/#review84878

I decided to review the entire patch out of curiosity...
r- because this won't build on gcc, but I would've given r+ otherwise, assuming Gerv's comments are addressed.

::: security/certverifier/NSSCertDBTrustDomain.cpp:742
(Diff revision 1)
>  
> +static bool
> +CertIsStartComOrWoSign(const CERTCertificate* cert)
> +{
> +  for (const DataAndLength& dn : StartComAndWoSignDNs) {
> +    if (cert->derSubject.len == dn.len &&

Hmm, https://groups.google.com/d/msg/mozilla.dev.security.policy/BV5XyFJLnQM/_DwiB1PDGQAJ says this:
"The code will use the subject key id (hash of public key) to identify the Affected Roots".

I guess we should probably update the plan, since as you noted the use of DNs is to allow us to actually test the code.

(Unless I'm not interpreting correctly, and the line actually means "we will use the pub key hashes to revoke the various roots if evidence of back dating is discovered", in which case, nevermind.)

::: security/certverifier/NSSCertDBTrustDomain.cpp:743
(Diff revision 1)
> +static bool
> +CertIsStartComOrWoSign(const CERTCertificate* cert)
> +{
> +  for (const DataAndLength& dn : StartComAndWoSignDNs) {
> +    if (cert->derSubject.len == dn.len &&
> +        PodEqual(cert->derSubject.data, dn.data, dn.len)) {

This needs a corresponding `#include "mozilla/PodOperations.h"`.

::: security/certverifier/StartComAndWoSignData.inc:2
(Diff revision 1)
> +// /C=CN/O=WoSign CA Limited/CN=CA \xE6\xB2\x83\xE9\x80\x9A\xE6\xA0\xB9\xE8\xAF\x81\xE4\xB9\xA6
> +static const uint8_t CA沃通根证书DN[72] = {

Looks like gcc refuses to compile this line (certainly 6.2.1 in Fedora 24):
> /moz/mozilla-inbound/security/certverifier/StartComAndWoSignData.inc:2:24: error: stray ‘\346’ in program
> static const uint8_t CA沃通根证书DN[72] = {
>                        ^

::: security/manager/ssl/tests/unit/test_startcom_wosign.js:2
(Diff revision 1)
> +// This Source Code Form is subject to the terms of the Mozilla Public
> +// License, v. 2.0. If a copy of the MPL was not distributed with this
> +// file, You can obtain one at http://mozilla.org/MPL/2.0/.

Should this be the Public Domain license instead?
Attachment #8800898 - Flags: review-
Regarding this plan to use the subject key id (hash of public key) of the Affected Roots to distrust certificates chaining up to the Affected Roots with a notBefore date after October 21.

Please confirm if the following statements are correct. 

1) This will impact certs with one of the subject key id's regardless of whether those certs are manually imported or in the default root store.

2) If the CA wants to issue new certs to customers and have those certs trusted by Firefox, the CA will need to create a new root cert with a different subject key id, and have the end-users manually import the new root cert.

3) If the new root cert is cross-signed by one of the Affected Roots, and the new root cert is manually imported by an end-user, both new and old certs will validate correctly. 

Thanks,
Kathleen
(In reply to Kathleen Wilson from comment #10)
> Regarding this plan to use the subject key id (hash of public key) of the
> Affected Roots to distrust certificates chaining up to the Affected Roots
> with a notBefore date after October 21.

So this change actually uses their X.500 Distinguished Names to target the Affected Roots, not the SPKIs of the roots. As Keeler pointed out, if we use the SPKI, then we cannot write automated tests of the code, as we cannot generate a hash collision for these certificates.

So I guess the first question is: Do we want it to be the SPKI enough that we let it land without automated tests?
Flags: needinfo?(kwilson)
If you distrust based on the DN, it will require that future roots, which potentially get included again, must use a different DN. Is that acceptable?

It might be useful to require that future roots use different DNs, as it makes it easier for humans to distinguish future roots from these distrusted roots.
(In reply to Kathleen Wilson from comment #10)
> Regarding this plan to use the subject key id (hash of public key) of the
> Affected Roots to distrust certificates chaining up to the Affected Roots
> with a notBefore date after October 21.
> 
> Please confirm if the following statements are correct. 
> 
> 1) This will impact certs with one of the subject key id's regardless of
> whether those certs are manually imported or in the default root store.

The proposed implementation uses subject distinguished names instead of key ids, but yes - this applies to built-in and imported certificates.

> 2) If the CA wants to issue new certs to customers and have those certs
> trusted by Firefox, the CA will need to create a new root cert with a
> different subject key id, and have the end-users manually import the new
> root cert.

Again, it would be a different distinguished name instead of a key id (they could actually re-use existing keys, unless that's prohibited by some policy), but yes.

> 3) If the new root cert is cross-signed by one of the Affected Roots, and
> the new root cert is manually imported by an end-user, both new and old
> certs will validate correctly.

New certificates for which there is a path to the new root that doesn't traverse any certificates sharing a distinguished name with the affected roots will validate correctly.

If there exists a path from old certificates to the new root that again doesn't involve any distinguished names from the affected roots, those certificates will validate correctly as well.
(In reply to Kai Engert (:kaie) from comment #12)
> If you distrust based on the DN, it will require that future roots, which
> potentially get included again, must use a different DN. Is that acceptable?
> 
> It might be useful to require that future roots use different DNs, as it
> makes it easier for humans to distinguish future roots from these distrusted
> roots.

That does seem like a beneficial result, yes.
(In reply to J.C. Jones [:jcj] from comment #11)
> So this change actually uses their X.500 Distinguished Names to target the
> Affected Roots, not the SPKIs of the roots. As Keeler pointed out, if we use
> the SPKI, then we cannot write automated tests of the code, as we cannot
> generate a hash collision for these certificates.
> 
> So I guess the first question is: Do we want it to be the SPKI enough that
> we let it land without automated tests?

I think it's OK to use the Distinguished Name. 

Just to make sure I fully understand, by Distinguished Name, do you mean just the Subject CN, or the full Subject?
i.e. is it the following for the Affected Roots?
1) CN=CA 沃通根证书, OU=null, O=WoSign CA Limited, C=CN 
2) CN=Certification Authority of WoSign, OU=null, O=WoSign CA Limited, C=CN 
3) CN=Certification Authority of WoSign G2, OU=null, O=WoSign CA Limited, C=CN 
4) CN=CA WoSign ECC Root, OU=null, O=WoSign CA Limited, C=CN 
5) CN=StartCom Certification Authority, OU=Secure Digital Certificate Signing, O=StartCom Ltd., C=IL 
6) CN=StartCom Certification Authority G2, OU=null, O=StartCom Ltd., C=IL
Flags: needinfo?(kwilson)
Yes, the full subject for each certificate. Here's the output I'm getting (in no particular order) from openssl:

subject= /C=IL/O=StartCom Ltd./OU=Secure Digital Certificate Signing/CN=StartCom Certification Authority
subject= /C=IL/O=StartCom Ltd./CN=StartCom Certification Authority G2
subject= /C=IL/O=StartCom Ltd./OU=Secure Digital Certificate Signing/CN=StartCom Certification Authority
subject= /C=CN/O=WoSign CA Limited/CN=Certification Authority of WoSign
subject= /C=CN/O=WoSign CA Limited/CN=CA \xE6\xB2\x83\xE9\x80\x9A\xE6\xA0\xB9\xE8\xAF\x81\xE4\xB9\xA6
subject= /C=CN/O=WoSign CA Limited/CN=Certification Authority of WoSign G2
subject= /C=CN/O=WoSign CA Limited/CN=CA WoSign ECC Root

(if you utf8-decode the hex you get '沃通根证书')
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

https://reviewboard.mozilla.org/r/85710/#review84878

Thanks for having a look!

> This needs a corresponding `#include "mozilla/PodOperations.h"`.

Added.

> Looks like gcc refuses to compile this line (certainly 6.2.1 in Fedora 24):
> > /moz/mozilla-inbound/security/certverifier/StartComAndWoSignData.inc:2:24: error: stray ‘\346’ in program
> > static const uint8_t CA沃通根证书DN[72] = {
> >                        ^

Huh - interesting. Well, anyway, I replaced it with an ascii identifier.

> Should this be the Public Domain license instead?

Sure - sounds good.
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

https://reviewboard.mozilla.org/r/85710/#review86486

Looks good!
Attachment #8800898 - Flags: review?(cykesiopka.bmo) → review+
See Also: → 1311995
Can someone comment on how the switch from using public key hash to using DNs to identify the certificates affects the situation with cross-signing? A couple of these roots are cross-signed by another CA Mozilla trusts.
https://wiki.mozilla.org/CA:WoSign_Issues#Cross_Signing

Gerv
If I'm reading the code correctly, each certificate in a chain is looked at. If any certificate has a subject name that matches one from the blacklist, the date based check is enforced. Given that a cross signed intermediate must have the equivalent subject name, to be considered as an issuer certificate, this should identify the cross intermediates, too.

However, that leads to another question:

The current code is based on the binary encoding of the subject name. I believe a subject name could be encoded in multiple ways, e.g. with a different order of the name elements.

I don't know the strategy that PSM and NSS use to identify a potential issuer certificate. Will they require an identical binary encoding, or will they accept alternative encodings of the same name?

If they accepted alternative encodings, and if the cross signed intermediates used an alternative binary encoding, then we'd have to add the alternative encodings to the blacklist.
mozilla::pkix requires that the subject's issuer be byte-for-byte identical to the issuer's subject, so it won't consider issuers with alternative encodings to be valid.
David, thanks for this clarification.
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77880cde0de1
revoke StartCom and WoSign certificates issued after 21 October 2016 r=Cykesiopka,jcj
https://hg.mozilla.org/mozilla-central/rev/77880cde0de1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Attached file 1309707_broken_sites.txt (obsolete) —
I ran TLS Canary to look for problems w/r/t blocked intermediates, and found no breakage there.

For the change w/r/t notBefore date, we now break 32 sites as of today. I assume that number will rise if they continue to issue certificates from these roots. 

e.g.,
https://extensiblewebmanifesto.org/
Hi Kathleen, should we be concerned with the above?
Flags: needinfo?(kwilson)
(In reply to Matt Wobensmith [:mwobensmith][:matt:] from comment #28)
> Hi Kathleen, should we be concerned with the above?

Are you able to provide the SSL cert notBefore and notAfter dates for those sites?
(just from a testing perspective)
Flags: needinfo?(kwilson)
(In reply to Kathleen Wilson from comment #29)

> Are you able to provide the SSL cert notBefore and notAfter dates for those
> sites? (just from a testing perspective)

Normally yes, I capture this data. However, the error sec_error_revoked_certificate occurs at the protocol level, and the canary never gets a chance to see what's in the cert itself.

In any case, I obtained the info above using another method, so here goes.
Attachment #8805186 - Attachment is obsolete: true
Thanks!

It'll be interesting to keep an eye on this over time, but there isn't anything we can do about it. Those folks will need to get new certs at some point in the near future.
Hi,

although I know this situation is getting us in difficulties, and also our customers and relying parties, all these customers are somehow informed of the issue and some have decided to go forward and some others didn´t. Due to this situation we´re loosing some customers, important projects and contracts, but you have to understand that we can´t stop issuing certificates. And for sure we´re trying to find a solution to these customers but it´s not easy.
In the meantime we´re continuing with our work plan sent to the mozilla dev security mailing list to be able to regain the trust in StartCom from a newly system as stated in the plan. 

Best regards
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: users won't be protected against new certificates issued by CAs we currently do not trust to issue new certificates
[Describe test coverage new/current, TreeHerder]: has tests
[Risks and why]: low - we're doing a strict match against a small, specific list of CAs. There shouldn't be any false positives.
[String/UUID change made/needed]: none
Attachment #8800898 - Flags: approval-mozilla-aurora?
Comment on attachment 8800898 [details]
bug 1309707 - revoke StartCom and WoSign certificates issued after 21 October 2016

This is important security issue for users. Users won't be protected without this fix. Take it in 51 aurora.
Attachment #8800898 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm hitting conflicts uplifting this to aurora. Could we get a rebased patch?
Flags: needinfo?(dkeeler)
Attached patch patch for auroraSplinter Review
Sure - this should do it.
Flags: needinfo?(dkeeler)
You need to log in before you can comment on or make changes to this bug.