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

VERIFIED FIXED in Firefox 32

Status

()

defect
P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: MattN, Assigned: gfritzsche)

Tracking

unspecified
Firefox 34
Points:
2
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox32+ verified, firefox33+ verified, firefox34+ verified)

Details

()

Attachments

(3 attachments, 1 obsolete attachment)

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?
Flags: needinfo?(nmaul)
Flags: needinfo?(benjamin)
Flags: needinfo?(benjamin) → needinfo?(cturra)
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.
Flags: needinfo?(nmaul)
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?
Assignee: nobody → georg.fritzsche
Points: --- → 2
Flags: needinfo?(cturra) → needinfo?(nmaul)
Priority: -- → P1
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Georg can you please postpone your other work to pick this up?

Sure, will do.
Kamil, can you take QA for this?
Flags: needinfo?(kamiljoz)
(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.
Flags: needinfo?(nmaul)
(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.
Flags: needinfo?(cturra)
Posted patch Use DigiCert (obsolete) — Splinter Review
Attachment #8468591 - Flags: review?(cviecco)
Iteration: --- → 34.2
Flags: firefox-backlog+
Forwarding to Gavin to consider including in the iteration.
Flags: needinfo?(gavin.sharp)
Comment 2 has this covered - this is a time-critical fix, so we are adding it.
Flags: needinfo?(gavin.sharp)
Status: NEW → ASSIGNED
QA Whiteboard: [qa?]
(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 on attachment 8468591 [details] [diff] [review]
Use DigiCert

Apparently cviecco is on PTO, David, Freddy suggested you might be able to review this?
Attachment #8468591 - Flags: review?(cviecco) → review?(dkeeler)
(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?
QA Whiteboard: [qa?] → [qa+]
QA Contact: kamiljoz
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 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.
Attachment #8468591 - Flags: review?(dkeeler) → review-
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
Attachment #8468591 - Flags: review- → review?(dkeeler)
Comment on attachment 8468591 [details] [diff] [review]
Use DigiCert

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

Bit of a collision there, I think :)
Attachment #8468591 - Flags: review?(dkeeler) → review-
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.
Attachment #8468591 - Attachment is obsolete: true
Attachment #8469508 - Flags: review?(dkeeler)
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.
Attachment #8469511 - Flags: review?(dkeeler)
Attachment #8469508 - Flags: review?(dkeeler)
Attachment #8469511 - Flags: review?(dkeeler)
Attachment #8469511 - Attachment description: Remove CertUtils usage in experiments → Trunk patch: Remove CertUtils usage in experiments
Attachment #8469508 - Attachment description: Skip CertUtils check → Uplift patch: Skip CertUtils check
checkin-needed for the "trunk patch" for when the trees re-open.
Keywords: checkin-needed
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.
Attachment #8469508 - Flags: approval-mozilla-beta?
Attachment #8469508 - Flags: approval-mozilla-aurora?
Matt, can you take a look since I think Kamil is still traveling?
Flags: needinfo?(mwobensmith)
Flags: needinfo?(mwobensmith)
QA Contact: kamiljoz → mwobensmith
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.
Flags: needinfo?(kamiljoz)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/55f86db6466c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Attachment #8469508 - Flags: approval-mozilla-beta?
Attachment #8469508 - Flags: approval-mozilla-beta+
Attachment #8469508 - Flags: approval-mozilla-aurora?
Attachment #8469508 - Flags: approval-mozilla-aurora+
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.
QA Contact: mwobensmith → kamiljoz
Iteration: 34.2 → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.