Closed Bug 1301956 Opened 8 years ago Closed 8 years ago

Add various mozilla sites to preloaded pins

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jvehent, Assigned: jvehent)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file, 5 obsolete files)

The attached patch adds the following sites to preloaded pins:
- download.mozilla.org
- crash-reports.mozilla.com
- crash-reports-xpsp2.mozilla.com
- crash-stats.mozilla.com
- (*.)telemetry.mozilla.org
- (*.)testpilot.firefox.com

All of which are controlled by Cloud Services and use either the Digicert Root or EV Root.

The patch also flips the "test_mode" for aus4 and aus5 from true to false. I imagine this will activate the pining in release, but would welcome feedback as my understanding of the impact is limited.
Attachment #8790096 - Flags: review?(dkeeler)
Attachment #8790096 - Flags: feedback?
Rob - any extra considerations here for aus4/aus5? I know we explicitly disabled the nsUpdateService pinning awhile back.
Flags: needinfo?(robert.strong.bugs)
In bug 1063111 it was agreed that we would not pin aus.
Flags: needinfo?(robert.strong.bugs)
:rstrong - I'd be surprised if users behind MITM proxies can obtain their updates at all, because the pinning currently covers download.cdn.mozilla.net which is used to serve the MAR files. As it stands, I think those users are already broken, but :keeler should confirm.
Comment on attachment 8790096 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

For the new hosts, we should probably start them out in test mode and gather some data before turning them on for real (see e.g. https://people.mozilla.org/~dkeeler/pinning-dashboard/ )

::: security/manager/tools/PreloadedHPKPins.json
@@ +207,5 @@
> +    // Crash report sites
> +    { "name": "crash-reports.mozilla.com", "include_subdomains": false,
> +      "pins": "mozilla_services", "test_mode": false, "id": 10 },
> +    { "name": "crash-reports-xpsp2.mozilla.com", "include_subdomains": false,
> +      "pins": "mozilla_services", "test_mode": false, "id": 10 },

IDs should probably be unique. If we want to bucket these together when analyzing the telemetry we get back, we can do that (but otherwise I imagine it would be good to be able to differentiate between these hosts if we encounter a problem).
Attachment #8790096 - Flags: review?(dkeeler) → review-
(In reply to Julien Vehent [:ulfr] from comment #3)
> :rstrong - I'd be surprised if users behind MITM proxies can obtain their
> updates at all, because the pinning currently covers
> download.cdn.mozilla.net which is used to serve the MAR files. As it stands,
> I think those users are already broken, but :keeler should confirm.

If a user is behind a MITM proxy and has imported the corresponding root certificate, they should be able to download from pinned sites (by default we don't enforce pinning for non-built-in root certificates for exactly this reason).
(In reply to Julien Vehent [:ulfr] from comment #3)
> :rstrong - I'd be surprised if users behind MITM proxies can obtain their
> updates at all, because the pinning currently covers
> download.cdn.mozilla.net which is used to serve the MAR files. As it stands,
> I think those users are already broken, but :keeler should confirm.
That's a bummer but at least with aus not pinned it is still possible to notify these users about updates so it still shouldn't be pinned. There are other security mitigations in app update and aus should not be pinned.

(In reply to David Keeler [:keeler] (use needinfo?) from comment #5)
> (In reply to Julien Vehent [:ulfr] from comment #3)
> > :rstrong - I'd be surprised if users behind MITM proxies can obtain their
> > updates at all, because the pinning currently covers
> > download.cdn.mozilla.net which is used to serve the MAR files. As it stands,
> > I think those users are already broken, but :keeler should confirm.
> 
> If a user is behind a MITM proxy and has imported the corresponding root
> certificate, they should be able to download from pinned sites (by default
> we don't enforce pinning for non-built-in root certificates for exactly this
> reason).
Already understood
Given the fact local roots bypass the pins, what reason would we have to leave AUS out of the pinning?
Flags: needinfo?(robert.strong.bugs)
Currently we rely on mar signing to prevent security exploits from both the web as well as after the file has been downloaded an is local to the system. Given that, what value is there to cert pinning for app update?

The last time this was discussed which was in bug 1063111 we decided that app update should not be pinned and it was my understanding there are circumstances such as described in comment #5 where the user hasn't imported the corresponding root certificate and possibly others where the update check would fail. Since certificate pinning won't provide any additional security over mar signing and there are cases where the update check would fail and prevent us from notifying users of updates which provide security as well as other fixes I don't see the value of having certificate pinning for aus and I actually think the end result is less secure for the case where users won't be able to be notified of updates due to any issues with certificate pinning.
Flags: needinfo?(robert.strong.bugs)
The main value is defense in depth. In bug 1303418, we found an issue in the way the addons update process works. Addons are signed, yet the issue worked around the signing to allow replacing a signed addon with another signed addon. This could happen to MAR files as well. The point of pining aus*.mozilla.org is adding an extra layer of security that would prevent such a vulnerability from being exploited in the wild (eg. on Tor exit nodes) because the attackers couldn't obtain a valid cert from Digicert.

I think there is value in pinning AUS *if* we can get enough assurance it's not breaking users. What about running in reporting mode only for 18 weeks, then revisiting the feasibility of pinning AUS?
It is possible that this could happen to mar files as well but we *know* that it does happen to users that haven't imported the root certificate. If the known issues with certificate pinning were addressed such as the requirement noted in comment #5 as well as others then I would be much more willing to do this. For example, in the past there was an entire country (I forget which) that had a proxy which broke update checks for people that didn't import the certificate. Also, digicert was exploited many years ago which could also happen again and stating that it couldn't be exploited is as I see it incorrect. We've been running in reporting only mode since around the time since cert pinning was introduced. If you could provide the data source for this I can evaluate the existing data to see what if any cost to security there would be if those people aren't notified of updates.
:keeler - would you know where to find the telemetry data for aus4.mozilla.org/id=3 ?
Flags: needinfo?(dkeeler)
aus5 is our main host
Yes, sorry, I thought aus5 wasn't yet listed, but it is: aus5.mozilla.org/id=7
BTW: the way add-ons were signed (haven't checked for quite some time and this excludes the original and very old method for signing xpi's) involved both a server file and the add-on to verify so with add-ons it makes much more sense to use cert pinning and it used a cert pinning hack via CertUtils.jsm to accomplish this at least before cert pinning was available within Firefox. Mar signing does not rely on a server file beyond the mar to download as the add-ons manager did and probably still does.
I've updated the telemetry dashboard at https://people.mozilla.org/~dkeeler/pinning-dashboard/ (it might take a while to load the first time. Also let me know if it's broken in some way.)
If you want to dig into the data yourself, the telemetry probe you're looking for is CERT_PINNING_MOZ_TEST_RESULTS_BY_HOST. aus4 corresponds to buckets 6 (failures) and 7 (successes). aus5 is buckets 14 (failures) and 15 (successes).
Flags: needinfo?(dkeeler)
Assigning to :ulfr to reflect who actually wrote the patch.
Assignee: dkeeler → jvehent
Priority: -- → P1
Whiteboard: [psm-assigned]
According to the dashboard, an average of 0.025% of users run into pinning violations for aus5. That's about the same amount for AMO and half of services.m.c. Numbers are pretty stable for the past 7 months.

I don't recall the exact numbers, but I think we leave well over 5% of our users behind during updates, for various reasons. 0.025% seems like a very low number in comparison, but I don't feel qualified to make that call.
We've had bug reports for the pinning hack we used to have which would happen with pinning as well. Since there is a cost to pinning this should be a product driver and security team decision with the understanding that yes, there may be a bug with mar signing that may be exploited just as CA's have been exploited and that from the app update side we won't be able to do anything about the loss of the ability to update to Firefox versions which include security fixes for the systems that fail because of this change.

David, could you get product drivers call on this?

Dan, what is your take on this?
Flags: needinfo?(dveditz)
Flags: needinfo?(ddurst)
Bug 1300633 is an example as to why I don't want aus pinning and it is why I disabled the cert pinning hack I implemented in CertUtils.jsm from app update as soon as we had mar signing. If it weren't for this we would be having an app update firedrill where some people wouldn't be able to get updates which include security fixes instead of a gmp firedrill.
(In reply to Julien Vehent [:ulfr] from comment #9)
> In bug 1303418, we found an issue in the way the addons update process works.
> Addons are signed, yet the issue worked around the signing to allow replacing
> a signed addon with another signed addon.

The cases are different: if you tried to update addon-A and the MITM instead gave you addon-B, we only validated that addon-B was a correctly signed addon-B when we should have been checking that addon-B was a correctly signed addon-A (which would have failed). For application updates we do check that it's signed by a particular signature, and isn't a downgrade. The worst a MITM can do in application updates is a DoS (or, if you're a couple of versions behind it could give you one that's not quite up to date, but that's not any worse than being more out of date).

I would _much_ rather have signed content than pin a cert. It would be better to also sign the update metadata response from AUS, but signed MARs and the checking we're doing is OK.
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #20)
> I would _much_ rather have signed content than pin a cert. It would be
> better to also sign the update metadata response from AUS, but signed MARs
> and the checking we're doing is OK.

This has come up more than once in the past year. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1304782 for it.
> I would _much_ rather have signed content than pin a cert.

I certainly agree with that, and not proposing we get rid of signed content :)

> Bug 1300633 is an example as to why I don't want aus pinning

Should we consider the entire product delivery pipeline as a whole, and decide whether or not it should be pinned? It still makes little sense to me we'd want AUS unpinned but download.m.o and download CDN pinned. Or am I missing something?
The user will be notified that there is an update that they should manually install if there is a problem with downloading the mar file from download.mo.o. If they have trouble checking for updates they will be notified after 10 consecutive failures to check for updates.
Ok, so we do want to leave AUS in testing mode, to collect telemetry, and continue pinning the rest of the pipeline. I'll rework the patch and put in some comments.
Flags: needinfo?(jvehent)
Flags: needinfo?(ddurst)
Updated patch.
- Removed the `mozilla` pinset entirely and uses `mozilla_services` only
- Added Digicert's EV root to `mozilla_services`
- Reset AUS 4 & 5 to test mode and moved them to the `mozilla_services` pinset
- Set telemetry & subdomains to test mode so we can collect pin violations via telemetry
- Added TestPilot, CrashReports & Sync
Attachment #8790096 - Attachment is obsolete: true
Attachment #8790096 - Flags: feedback?
Flags: needinfo?(jvehent)
Attachment #8797173 - Flags: review?(dkeeler)
Comment on attachment 8797173 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

Ok - r=me given we're confident that we're only using DigiCert and that we've thought about the consequences of if we ever want to move away from using DigiCert.
In fact, I think this is the kind of thing that we need more than one person signing off on, so I'm also going to ask Richard to take a look at this.

::: security/manager/tools/PreloadedHPKPins.json
@@ +75,4 @@
>        "name": "mozilla_services",
>        "sha256_hashes": [
>          "DigiCert Global Root CA"
> +        "DigiCert High Assurance EV Root CA",

I'm a little concerned with pinning to exactly one CA (in the business-entity sense). a) are we absolutely certain that all of the hosts we're pinning use only DigiCert? b) what's our plan for if DigiCert is hacked/goes rogue/etc.? (they're probably too big to fail in terms of pulling the root from our trust store, but what if we want to move away from using them?)
Attachment #8797173 - Flags: review?(rlb)
Attachment #8797173 - Flags: review?(dkeeler)
Attachment #8797173 - Flags: review+
> a) are we absolutely certain that all of the hosts we're pinning use only DigiCert?

Based on a year of certificates collected in the TLS Observatory, we only use Digicert certificates when setting up endpoints on *.firefox.com and *.services.mozilla.com. This doesn't include CT logs so there's is a possibility I missed one, but I think it's highly unlikely.

> b) what's our plan for if DigiCert is hacked/goes rogue/etc.?

I agree this is a concern. We don't currently have a relationship with another vendor, but we do use AWS for most of our services so I can see us using "Amazon Root CA {1,2,3,4}" as backups. That also means trusting "Starfield Class 2 Certification Authority" to get the cross-signature of the AWS CA. I don't know how much trust we want to put in these 2 CAs.
Comment on attachment 8797173 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

Would be r+ if not for the below.

::: security/manager/tools/PreloadedHPKPins.json
@@ +75,4 @@
>        "name": "mozilla_services",
>        "sha256_hashes": [
>          "DigiCert Global Root CA"
> +        "DigiCert High Assurance EV Root CA",

I agree with keeler.  We need to find a backup CA to pin before we make this change.  Julien, I think some combination of you, Travis, and IT are the right folks to do the vendor selection (Amazon seems like a fine choice to me, FWIW).  Please re-r? once you've added a backup.
Attachment #8797173 - Flags: review?(rlb) → review-
Flags: needinfo?(jvehent)
I updated the patch to add Comodo's EV intermediate "ECC Extended Validation Secure Server CA", based on discussion with various folks. We don't currently have a relationship with this CA, so it's strictly added as a backup for Cloud Services properties should something dramatic happen to Digicert.

:keeler - Could you confirm I added the intermediate to the extra_certificates array correctly? I don't have a way to test the parsing of this file.
Attachment #8797173 - Attachment is obsolete: true
Flags: needinfo?(jvehent)
Attachment #8801180 - Flags: review?(rlb)
Attachment #8801180 - Flags: review?(dkeeler)
Comment on attachment 8801180 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

Yes, that's correct (you can run security/manager/tools/genHPKPStaticPins.js with xpcshell to verify).
I'm a little dubious about having an EV CA as a backup, though. If Comodo issues only EV certificates from that CA, if we ever do have to use it, the replacement process will be expensive (wildcards aren't allowed for EV and it looks like Comodo EV certs are $99 each) and slow (since the CA has to actually verify an identity, there's no automatic issuance). If Comodo doesn't issue only EV certificates from that CA, then I don't think there's a marginal benefit to going with an EV CA. Why not just use LetsEncrypt as a backup and keep the domain blacklist in place until we actually do need certificates from them?

::: security/manager/tools/PreloadedHPKPins.json
@@ +215,4 @@
>    ],
>  
> +  "extra_certificates": [
> +      "MIIDojCCAyigAwIBAgIQBh1GQ7QStdjXFUmdhVOqAzAKBggqhkjOPQQDAzCBhTELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UEBxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxKzApBgNVBAMTIkNPTU9ETyBFQ0MgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkwHhcNMTMwNDE1MDAwMDAwWhcNMjgwNDE0MjM1OTU5WjCBkjELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UEBxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxODA2BgNVBAMTL0NPTU9ETyBFQ0MgRXh0ZW5kZWQgVmFsaWRhdGlvbiBTZWN1cmUgU2VydmVyIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEV3AaPyeTQy0aWXXkBJMR42DsJ5pnbliJe7ndaHzCDslVlY8ofpxeFiqluZrKKNcJeBU/Jl1YI9jLMyMZKsfSoaOCAWkwggFlMB8GA1UdIwQYMBaAFHVxpxlIGbydnepBR9+UxEh3mdN5MB0GA1UdDgQWBBTTTsMZulhZ0Rxgt2FTRzund4/4ijAOBgNVHQ8BAf8EBAMCAYYwEgYDVR0TAQH/BAgwBgEB/wIBADA+BgNVHSAENzA1MDMGBFUdIAAwKzApBggrBgEFBQcCARYdaHR0cHM6Ly9zZWN1cmUuY29tb2RvLmNvbS9DUFMwTAYDVR0fBEUwQzBBoD+gPYY7aHR0cDovL2NybC5jb21vZG9jYS5jb20vQ09NT0RPRUNDQ2VydGlmaWNhdGlvbkF1dGhvcml0eS5jcmwwcQYIKwYBBQUHAQEEZTBjMDsGCCsGAQUFBzAChi9odHRwOi8vY3J0LmNvbW9kb2NhLmNvbS9DT01PRE9FQ0NBZGRUcnVzdENBLmNydDAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuY29tb2RvY2EuY29tMAoGCCqGSM49BAMDA2gAMGUCMQDmPWS98nREWdt4xB83r9MVvgG5INpKHi6V1dUYlCqvSvXXjK0QvZSrOB7cj9RavGgCMG2xJNG+SvlTWEYpmK7eXSgmRUgoBDeQ0yDKlnxmeeOBnnCaDIxAcA3aCj2Gtdt3sA=="

It might be helpful to include a comment with the subject DN here or something.
Attachment #8801180 - Flags: review?(dkeeler) → review+
Comment on attachment 8801180 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

Sorry for the delay.  Will try to be more timely in the next cycle.

::: security/manager/tools/PreloadedHPKPins.json
@@ +77,4 @@
>          "DigiCert Global Root CA",
>          "DigiCert High Assurance EV Root CA",
> +        // Backup EV intermediate, just in case, not normally in use
> +        "COMODO ECC Extended Validation Secure Server CA"

According to crt.sh, it looks like this CA is only issuing EV certificates [1].  It seems like we should not limit ourselves to only issuing EV if DigiCert goes bad for some reason.  Suggest either:

1. Pinning to the COMODO root, "COMODO ECC Certification Authority" [2] instead of this one
2. Pinning to the "COMODO ECC Domain Validation Secure Server CA" [3] in addition to this one

I also note on [2] that there are multiple versions of both the EV and DV CAs.  Should we be pinning to all of the variants?

[1] https://crt.sh/?Identity=%25&iCAID=1389
[2] https://crt.sh/?caid=1388
[3] https://crt.sh/?caid=1582
Attachment #8801180 - Flags: review?(rlb) → review-
I'm changing direction for the backup root and decided to use Let's Encrypt instead of Comodo. We have a blacklist in place with LE that protects our sites and we have a documented process to lift that blacklist (documented at [1]). Given this, I think we should use LE as a backup and continue using Digicert as primary CA.

:keeler - could you confirm that "ISRG Root X1" is the right name for the LE root? This is the CKA_LABEL I found in security/nss/lib/ckfw/builtins/certdata.txt.

[1] https://mana.mozilla.org/wiki/display/WebOps/Using+Let's+Encrypt+at+Mozilla#UsingLet%27sEncryptatMozilla-Whichdomainscan%27tuseLet%27sEncrypt
Attachment #8801180 - Attachment is obsolete: true
Flags: needinfo?(dkeeler)
Attachment #8812172 - Flags: review?(rlb)
Attachment #8812172 - Flags: review?(dkeeler)
Yes, "ISRG Root X1" is the name of the LE root. The one issue is that it looks like LE setups are still sending the intermediate that chains to the "DST Root CA X3" root rather than the ISRG root. I don't know when that's expected to change (maybe only when the ISRG root is in other root programs?). Richard or JC might know.
Flags: needinfo?(dkeeler)
Comment on attachment 8812172 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

r=me modulo determining if comment 33 is a concern.
Attachment #8812172 - Flags: review?(dkeeler) → review+
Comment on attachment 8812172 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

::: security/manager/tools/PreloadedHPKPins.json
@@ +77,5 @@
>          "DigiCert Global Root CA",
>          "DigiCert High Assurance EV Root CA",
> +        // Backup root with Let's Encrypt is not normally in use and
> +        // requires disabling Mozilla's sites blacklisting before use
> +        "ISRG Root X1"

I agree with Keeler that this is probably not the right pin, if only because it's not guaranteed which path the client will choose.

I would suggest pinning to the four LE intermediates instead.  Probably sufficient to have X3 and X4. 

https://letsencrypt.org/certificates/
Attachment #8812172 - Flags: review?(rlb) → review-
Updated version using LE's X3 and X4 intermediates as backups (X1/2 are marked as retired so I didn't include them). I added both the ISRG and Identrust signed certs to extra_certificates, hopefully this is the right way to do it.
Attachment #8812172 - Attachment is obsolete: true
Attachment #8812448 - Flags: review?(rlb)
Attachment #8812448 - Flags: review?(dkeeler)
Comment on attachment 8812448 [details] [diff] [review]
Add Mozilla resources to preloaded pins

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

r=me with unnecessary extra certificates removed (see comment)

::: security/manager/tools/PreloadedHPKPins.json
@@ +217,3 @@
>    ],
> +  "extra_certificates": [
> +    // Subject: C=US, O=Let's Encrypt, CN=Let's Encrypt Authority X3

You should only need one of each of these (the only important things are the subject DN and the subject key).
Attachment #8812448 - Flags: review?(dkeeler) → review+
Final patch. r+ from keeler carries over. pending r? from rbarnes.
Attachment #8812448 - Attachment is obsolete: true
Attachment #8812448 - Flags: review?(rlb)
Attachment #8812910 - Flags: review?(rlb)
Attachment #8812910 - Flags: review?(rlb) → review+
:keeler - any chance you could check this in? I'm not familiar with the process to build and merge into central...
Flags: needinfo?(dkeeler)
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1cb42c8da5b
add more Mozilla resources to preloaded pins r=keeler,rbarnes DONTBUILD NPOTB
I pushed the patch. This won't actually make a code difference until the automated task runs this Saturday.
Flags: needinfo?(dkeeler)
Er, I guess the task is running daily now, so it should be even sooner than that.
https://hg.mozilla.org/mozilla-central/rev/f1cb42c8da5b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: