1048793 - Experiments #0::manifest fetch failed certificate checks. 'issuerName' value incorrect

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Matthew N. [:MattN]]

Attachment: Screen Shot of cert that was served on https://telemetry-experiment.cdn.mozilla.net

I saw the following errors in a new profile on 34.0a1 (2014-08-04) Nightly.

Expected certificate attribute 'issuerName' value incorrect, expected: 'CN=Cybertrust Public SureServer SV CA,O=Cybertrust Inc', got: 'CN=DigiCert High Assurance CA-3,OU=www.digicert.com,O=DigiCert Inc,C=US'.
Certificate checks failed. See previous errors for details. CertUtils.jsm:109
1407228935643	Browser.Experiments.Experiments	ERROR	Experiments #0::manifest fetch failed certificate checks: [{}] Log.jsm:760
1407228935646	Browser.Experiments.Experiments	ERROR	Experiments #0::_loadManifest - failure to fetch/parse manifest (continuing anyway): Error: Experiments - manifest fetch failed certificate checks: [Exception... "Certificate checks failed. See previous errors for details."  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: validateCert :: line 110"  data: no] Log.jsm:760

I think this is referring to the cert of https://telemetry-experiment.cdn.mozilla.net and it definitely is DigiCert instead of Cybertrust.

Any ideas about what's up?

Comment 1

[Jake Maul [:jakem]]

I thought this was going to be pinned to a range of cert providers, not just one?

This sort of disconnect *precisely* why I hate certificate pinning. Vastly more so when it involves a 3rd party (a CDN vendor, in this case).


To answer your question, yes, this did move from Akamai (Cybertrust) to Edgecast (Digicert) in the last week or so. We are in the process of cancelling our SSL CDN service with the former.

As to why it's not pinned such that multiple providers are allowed, I haven't a clue. That is something that we always express as a very important feature in any pinning scheme.

Comment 2

[Benjamin Smedberg]

You only gave us the one certificate name to pin. I'm happy to pin multiple if you have a list. Otherwise should we just switch it to the new certificate?

Georg can you please postpone your other work to pick this up?

Comment 3

[Georg Fritzsche [:gfritzsche]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Georg can you please postpone your other work to pick this up?

Sure, will do.

Comment 4

[Georg Fritzsche [:gfritzsche]]

Kamil, can you take QA for this?

Comment 5

[Chris Turra [:cturra]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> You only gave us the one certificate name to pin. I'm happy to pin multiple
> if you have a list. Otherwise should we just switch it to the new
> certificate?

which issuer have you pinned? does your issuer pinning chain back to the root ca certificate or are you pinning on the intermediate?

currently, our main ca is digicert, so we'll definitely want to include them.

Comment 6

[Georg Fritzsche [:gfritzsche]]

Currently it's Cybertrust here:
http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/browser/app/profile/firefox.js#l1700

Should i switch to DigiCert like here?
http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/browser/app/profile/firefox.js#l1678

Comment 7

[Chris Turra [:cturra]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #6)
> 
> Should i switch to DigiCert like here?
> http://hg.mozilla.org/mozilla-central/annotate/71497ed2e0db/browser/app/
> profile/firefox.js#l1678

yes, please switch to using digicert. the reason this has changed recently is because we moved from one CDN provider (akamai) to another (edgecast) for the delivery of the telemetry content. akamai used cybertrust, edgecast digicert. 

my concern here is that if down the road we decide to split the cdn traffic with another vendor, we'll run into this again. sadly, i don't know how we can work around that -- i just wanted to express my concern for potential future breakage.

Comment 8

[Georg Fritzsche [:gfritzsche]]

Attachment: Use DigiCert

Comment 9

[Marco Mucci [:MarcoM]]

Forwarding to Gavin to consider including in the iteration.

Comment 10

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

Comment 2 has this covered - this is a time-critical fix, so we are adding it.

Comment 11

[Kamil Jozwiak [:kjozwiak]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> Kamil, can you take QA for this?

I'll go through early next week when I get back from a conference, flying back to TO on Friday morning.

Comment 12

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8468591 [details] [diff] [review]
Use DigiCert

Apparently cviecco is on PTO, David, Freddy suggested you might be able to review this?

Comment 13

[Georg Fritzsche [:gfritzsche]]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #11)
> I'll go through early next week when I get back from a conference, flying
> back to TO on Friday morning.

Is there any other QA contact in case it makes the next Nightly?

Comment 14

[[:mmc] Monica Chew (no longer reading bugmail)]

We should not be pinning *.cdn.mozilla.* like this at all. It is redundant with real certificate pinning, which is enforced at the network layer.

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

Comment 15

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8468591 [details] [diff] [review]
Use DigiCert

Review of attachment 8468591 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/profile/firefox.js
@@ +1697,5 @@
>  
>  // Telemetry experiments settings.
>  pref("experiments.enabled", true);
>  pref("experiments.manifest.fetchIntervalSeconds", 86400);
>  pref("experiments.manifest.uri", "https://telemetry-experiment.cdn.mozilla.net/manifest/v1/firefox/%VERSION%/%CHANNEL%");

To expand on what Monica said, this host is already pinned to the certificates indicated at http://hg.mozilla.org/mozilla-central/annotate/aa1617678a90/security/manager/boot/src/StaticHPKPins.h#l442 in a way that is more robust than this mechanism (it would actually be trivial for a rogue CA to defeat these checks).

@@ +1699,5 @@
>  pref("experiments.enabled", true);
>  pref("experiments.manifest.fetchIntervalSeconds", 86400);
>  pref("experiments.manifest.uri", "https://telemetry-experiment.cdn.mozilla.net/manifest/v1/firefox/%VERSION%/%CHANNEL%");
>  pref("experiments.manifest.certs.1.commonName", "*.cdn.mozilla.net");
> +pref("experiments.manifest.certs.1.issuerName", "CN=DigiCert High Assurance CA-3,OU=www.digicert.com,O=DigiCert Inc,C=US");

These two prefs should probably just be removed.

Comment 16

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8468591 [details] [diff] [review]
Use DigiCert

Review of attachment 8468591 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/profile/firefox.js
@@ -1699,5 @@
>  pref("experiments.enabled", true);
>  pref("experiments.manifest.fetchIntervalSeconds", 86400);
>  pref("experiments.manifest.uri", "https://telemetry-experiment.cdn.mozilla.net/manifest/v1/firefox/%VERSION%/%CHANNEL%");
>  pref("experiments.manifest.certs.1.commonName", "*.cdn.mozilla.net");
> -pref("experiments.manifest.certs.1.issuerName", "CN=Cybertrust Public SureServer SV CA,O=Cybertrust Inc");

I recommend setting  manifest.cert.checkAttributes = false instead of updating this list as nmaul and cturra have already expressed can change. Note that in PublicKeyPinningService (which has nothing to do with CertUtils.jsm), cdn.mozilla.* is already pinned to the Digicert cert, and CertUtils.jsm is providing no additional assurance by enforcing these checks.

http://mxr.mozilla.org/mozilla-central/source/security/manager/boot/src/StaticHPKPins.h#442

Comment 17

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8468591 [details] [diff] [review]
Use DigiCert

Review of attachment 8468591 [details] [diff] [review]:
-----------------------------------------------------------------

Bit of a collision there, I think :)

Comment 18

[Georg Fritzsche [:gfritzsche]]

Attachment: Uplift patch: Skip CertUtils check

Per bug 1002586 and bug 744204 we have the pinning service on Firefox 32.
This simply skips the CertUtils checks in browser/experiments, which is apparently fine as PublicKeyPinningService already provides better functionality.

Comment 19

[Georg Fritzsche [:gfritzsche]]

Attachment: Trunk patch: Remove CertUtils usage in experiments

The previous patch is uplift-friendly. This one for trunk just strips the CertUtils usage in browser/experiments as PublicKeyPinningService apparently already provides these checks.

Comment 20

[Georg Fritzsche [:gfritzsche]]

checkin-needed for the "trunk patch" for when the trees re-open.

Comment 21

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8469508 [details] [diff] [review]
Uplift patch: Skip CertUtils check

This is a simple change, we want this on aurora & beta soon to be able to properly run experiments.

Approval Request Comment
[Feature/regressing bug #]: Experiments certificate pinning vs. CDN provider switch.
[User impact if declined]: We won't be able to push experiments to users (at least new profiles).
[Describe test coverage new/current, TBPL]: Manually checked that setting the prefs resolves the issue.
[Risks and why]: Low-risk - this just sets two pref which result in just skipping certificate checking code (which per the above discussion is redundant anyway).
[String/UUID change made/needed]: None.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Matt, can you take a look since I think Kamil is still traveling?

Comment 23

[Matt Wobensmith [:mwobensmith][:matt:]]

I can see the issue in nightly, and I can verify that changing the two prefs (specified in the patch) will allow the manifest to download without errors in the browser console.

Comment 24

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/55f86db6466c

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/55f86db6466c

Comment 26

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6f0a6a5bcf47
https://hg.mozilla.org/releases/mozilla-beta/rev/7aa9781a3758

Comment 27

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified fixed on today's Fx33 and Fx34. I confirmed that the error no longer appears, and that the experiments appear to run after the manifest loads.

Verified the presence also of the prefs in Fx33, although it appears the solution for Fx34 was to omit the offending cert checking code instead. 

Verification on Fx32 pending.

Comment 28

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue from comment #0 using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/32.0b4/win32/en-US/

Attempted to install the experiments listed below and received the same error message described in comment #0

Went through verification using the following builds:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/latest-beta/win32/en-US/

I went through all the current experiments that are on the beta channel and ensured they installed without any cert issues:

- Search Experiment - PASSED
- OOPP container unload timeout tester - PASSED
- Automatic Translation (PL locale) - PASSED
- Automatic Translation (TR locale) - PASSED
- Automatic Translation (VI locale) - PASSED