Closed Bug 1303127 (CVE-2016-5284) Opened 8 years ago Closed 8 years ago

ESR-45/Tor Browser certificate pinning bypass for addons.mozilla.org and other built-in sites

Categories

(Core :: Security: PSM, defect)

45 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox48 --- wontfix
firefox49 --- fixed
firefox-esr45 49+ fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: esmedudoit, Unassigned)

References

(Depends on 2 open bugs, )

Details

(Keywords: sec-high)

User Agent: Mozilla/5.0 (Linux; Android 5.0.2; LG-D415 Build/LRX22G) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.105 Mobile Safari/537.36

Steps to reproduce:

Write a malicious extension to be payload. Have it signed by Mozilla using their automated process. Generate a forged certificate for addons.mozilla.org that validates up through any CA built in to the Firefox certificate store. MiTM traffic to addons.mozilla.org trying to update NoScript or HTTPS Everywhere. Serve your malicious extension instead of the requested update to the target. 


http://seclists.org/dailydave/2016/q3/51


Actual results:

Said "certificate issuer is not built-in", obviously not failing due to pinning reinforcement.


Expected results:

Should have said "failed to validate pinned certificate." Mozilla doesn't use normal HPKP for certs related to their operations (like addons.mozilla.org. Mozilla using a form of static key pinning. Validations weaknesses limited to statically pinned certs.
Editing bug summary to reflect the core bug rather than the results.

The "certificate issuer is not built-in" error is a red-herring: vestigial code left over from a previous hard-coded pinning prior to HPKP existing as a spec, let alone supported in Firefox. Most of that was removed; I guess not all of it, but it's effectively deadcode because "real" pinning is supposed to prevent us getting there.

Why didn't pinning help? The built-in pins have an expiration date that's supposed to be about 90 days out from the release, updated weekly. Turns out the automation was broken on the ESR branch: bug 1285849

That means the AMO pins expired on Sept 3 in releases based on 45.3 (see last line)
https://hg.mozilla.org/releases/mozilla-esr45/file/0a590ea8e1cc4138c9bc198ba0906f099b85c346/security/manager/ssl/StaticHPKPins.h#l1233

If addons.mozilla.org issued an HPKP header then this default expiration would have been extended with each connection so that can be considered a second bug.
Status: UNCONFIRMED → NEW
Component: Untriaged → Security: PSM
Depends on: 1285849
Ever confirmed: true
Keywords: sec-high
Product: Firefox → Core
Summary: Cross-platform RCE in Tor Browser w/bypassed certificate pinning in addons.mozilla.org → ESR-45/Tor Browser certificate pinning bypass for addons.mozilla.org and other built-in sites
Version: 50 Branch → 45 Branch
Confirmed that static pins are working again in ESR 45.4 using builds from
https://archive.mozilla.org/pub/firefox/candidates/45.4.0esr-candidates/build1/

This was, indeed, due to the preload update automation failure.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
The static pins in Firefox 48.0.x expired also, on Sept 10.
https://hg.mozilla.org/releases/mozilla-release/rev/643f46ea3eaf

The static pins in Firefox 49 are schedule to expire on Nov 5
https://hg.mozilla.org/releases/mozilla-release/rev/39e2e75ed0c4

But Firefox 50 isn't scheduled to be released until Nov 8 -- they will expire again!
https://wiki.mozilla.org/RapidRelease/Calendar#Future_branch_dates

This process is broken. I'm reopening the bug.
Unless we change how we're updating the pins in Firefox 50 will expire Dec 17, but we're not releasing Firefox 51 until January 24 -- five weeks with expired pins.
https://hg.mozilla.org/releases/mozilla-aurora/rev/66754db69a70
So, I guess we should at least:
1. Bump the amount of time pins are valid.
   The current limit was IIRC set during the time when releases were always 6 weeks apart; releases can be delayed by a week or more now.
   I seem to recall some opposition to making the validity period too long though, but maybe I'm misremembering.
2. Add a test that checks that our static pins don't prematurely expire.
   If we can't get automation fixed in time, we can as a last resort just commit changes that bump the expiry time manually.

keeler: Thoughts?
Flags: needinfo?(dkeeler)
(In reply to :Cykesiopka from comment #7)
> So, I guess we should at least:
> 1. Bump the amount of time pins are valid.
>    The current limit was IIRC set during the time when releases were always
> 6 weeks apart; releases can be delayed by a week or more now.

Right - I filed bug 1303414 for that.

>    I seem to recall some opposition to making the validity period too long
> though, but maybe I'm misremembering.

I think the concern was if a user had an old version of the browser and a site had since rolled over its keys and/or CA, it wouldn't work until the list expired. I think it's acceptable to increase the timespan on which this would be a problem from "a few days after a new release" to maybe even a month or two after a new release (or more?).

> 2. Add a test that checks that our static pins don't prematurely expire.
>    If we can't get automation fixed in time, we can as a last resort just
> commit changes that bump the expiry time manually.

I think this may be more of a dashboard/external tool task. We need to answer the question "Given what's in the trees and what versions are shipping when, will any preloaded information expire before (or near to when) a new version will be available?", which would rely on a lot of external information. I filed bug 1303415 on developing this.
Flags: needinfo?(dkeeler)
> Write a malicious extension to be payload. Have it signed by Mozilla using their automated process. Generate a forged certificate for addons.mozilla.org that validates up through any CA built in to the Firefox certificate store. MiTM traffic to addons.mozilla.org trying to update NoScript or HTTPS Everywhere. Serve your malicious extension instead of the requested update to the target.

Why does Firefox overwrite one extension over another? Shouldn't it be (1) Verifying the signature of the signed extension, (2) Verifying that the signed extension's extension ID (from within the signed extension) matches the extension ID of the extension it is updating, (3) Verify that the version number in the signed extension is larger than the version number of the currently-installed version? If it isn't doing #2 and #3 then that's the real bug, IMO.
(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #9)
> Shouldn't it be (1) Verifying the signature of the signed extension,

Does that.

> (2) Verifying that the signed extension's extension ID (from within the signed extension)
> matches the extension ID of the extension it is updating,

It should indeed. bug 1303418

> (3) Verify that the version number in the signed extension is larger than the version
> number of the currently-installed version?

We do that
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#6872

(although a few lines above a 'compatibility update' might be able to override that? needs more investigation)
> It should indeed. bug 1303418
Why make this bug hidden now?
Don't know why bug 1303418 was hidden, most likely the default "security bug" setting that we forgot to change for this public security issue. It's not hidden now.
We don't need to leave this bug open: the immediate issue is resolved.
* Firefox ESR 45.4 will be released Sept 20 and has an appropriate pin expiration time (bug 1285849)
* Firefox ESR 49 will be released Sept 20 and has an appropriate pin expiration time (bug 1303370)
* Firefox ESR 45.3 and Firefox 48 users who make a successful addon update check will
  get a HPKP header, re-establishing cert pins for that domain (bug 1303371 comment 4)

There are a number of future changes to make this more robust tracking in other bugs
* add HPKP to the rest of AMO and increase the expiration time (bug 1303371)
* verify the downloaded package updates the right add-on (bug 1303418)
* content-sign the update metadata (bug 1303183)
* make sure pre-loaded pin expiration times are long enough (bug 1303414)
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Alias: CVE-2016-5284
Why was my name not listed as "reporting" the bug? Having submitted the bug report, making sure to list Ryan Duff as a link to the Researcher. Looking at the summary, I see the list shows Ryan Duff as reporting the bug to Mozilla, which actually isn't factually correct. I would appreciate an explanation. Thank you.
Hi esmedudoit, sorry about that. That should be fixed shortly.
Thank you for your prompt reply. Much appreciated.
My name btw is Esme (first name) Dudoit (last name) 

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16)
> Hi esmedudoit, sorry about that. That should be fixed shortly.
(In reply to esmedudoit from comment #14)
> Why was my name not listed as "reporting" the bug?

We did not learn about the issue from this bug, we heard about the issue in online chatter (for example from Ryan's post). I searched for bugs first to avoid filing a duplicate and found yours. If my search had been better I should have found Nik Unger's bug 1302640 instead.
I did not use "chatter on the internet" to report this bug. I filed this bug report within the same hour that Ryan Duff posted his research. I filed an official Mozilla bug report. Ryan did not file ANY Mozilla bug reports.  Unlike Nik Ungers report I did specify that this bug affected Tor Browser. I find it very off putting and intellectually dishonest of you specifically (and not Liz Henry) that you specifically say this bug report had zero barring upon the resolution and patch. I think it is abhorrently deceitful that you published that Ryan Duff reported this specific bug (1303127 CVE-2016-5284) when it is blatantly untrue.








(In reply to Daniel Veditz [:dveditz] from comment #19)
> (In reply to esmedudoit from comment #14)
> > Why was my name not listed as "reporting" the bug?
> 
> We did not learn about the issue from this bug, we heard about the issue in
> online chatter (for example from Ryan's post). I searched for bugs first to
> avoid filing a duplicate and found yours. If my search had been better I
> should have found Nik Unger's bug 1302640 instead.
Furthermore, Nik Unger filed a bug report dated September 13th, I filed a bug report September 15, with a link to Ryan Duffs research. Ryan Duff irrefutably filed zero bug reports through official bugzilla channels, yet you clearly have his name written as reporting bug 1303127 (CVE-2016-5284) which as I've already stated clearly says it was reported by me Esme Dudoit. This is insulting and demeaning and it's a lie. Why do you feel the need to lie about such an inane detail is absurd. Nils report is a duplicate. You can not randomly attach the researchers name to my bug number. That's insane.








(In reply to Daniel Veditz [:dveditz] from comment #19)
> (In reply to esmedudoit from comment #14)
> > Why was my name not listed as "reporting" the bug?
> 
> We did not learn about the issue from this bug, we heard about the issue in
> online chatter (for example from Ryan's post). I searched for bugs first to
> avoid filing a duplicate and found yours. If my search had been better I
> should have found Nik Unger's bug 1302640 instead.
https://www.mozilla.org/en-US/security/advisories/mfsa2016-85/?utm_campaign=buffer&utm_content=buffer9fa71&utm_medium=social&utm_source=twitter.com This link notes who filed Bug Report #1303127 (CVE-2016-5284) the answer is me, Esme Dudoit. It did not list a bug report number for Nik Unger or a non-existent Bug Report # from Ryan Duff. The bug report #1303127 is Esme Dudoit. CVE-2016-5284 is Esme Dudoit, it is NOT Ryan Duff. Ryan Duff was the researcher whose work i linked to in MY bug report to Mozilla dated September 15, 2016. 





(In reply to Daniel Veditz [:dveditz] from comment #19)
> (In reply to esmedudoit from comment #14)
> > Why was my name not listed as "reporting" the bug?
> 
> We did not learn about the issue from this bug, we heard about the issue in
> online chatter (for example from Ryan's post). I searched for bugs first to
> avoid filing a duplicate and found yours. If my search had been better I
> should have found Nik Unger's bug 1302640 instead.
"CVE-2016-5284 - Add-on update site certificate pin expiration [HIGH]
Reporter: Ryan Duff
Description: Due to flaws in the process we used to update "Preloaded Public Key Pinning" in our releases, the pinning for add-on updates became ineffective in early September. An attacker who was able to get a mis-issued certificate for a Mozilla web site could send malicious add-on updates to users on networks controlled by the attacker. Users who have not installed any add-ons are not affected. [1303127]"

So this above is what's written: How can this corresponding bug report number be reported by Ryan Duff? When you click on the highlighted #1303127 - it says "reported by esmedudoit" the CVE number is attached to this specific bug report. Please make a change. If you insist upon lying, remove Ryan's name and leave it blank or use Nik Ungers bug report. But don't insult my intelligence and attach someone's name who has never filed a Mozilla bug report, to my statistically accurate bug report with my name on it.













(In reply to Daniel Veditz [:dveditz] from comment #19)
> (In reply to esmedudoit from comment #14)
> > Why was my name not listed as "reporting" the bug?
> 
> We did not learn about the issue from this bug, we heard about the issue in
> online chatter (for example from Ryan's post). I searched for bugs first to
> avoid filing a duplicate and found yours. If my search had been better I
> should have found Nik Unger's bug 1302640 instead.
If my bug report #1303127 CVE-2016-5284 was not the source of learning of the issue listed in https://www.mozilla.org/en-US/security/advisories/mfsa2016-85/?utm_campaign=buffer&utm_content=buffer9fa71&utm_medium=social&utm_source=twitter.com can you please replace it with the correct bug report number and CVE number. Please remove my Mozilla bug report number and the CVE number that corresponds to it from your blog and bug update news. It's embarrassing that Mozilla team would attach someone else's name to a bug I reported. When in fact "my bug report was not the bug that alerted you to this issue." As you stated "we learned of it from 'online chatter'" - you should make that distinction clear in your blog. Reported by: Online Chatter




"--- Comment #19 from Daniel Veditz [:dveditz] <dveditz@mozilla.com> 2016-09-21 02:11:54 PDT ---
(In reply to esmedudoit from comment #14)
> Why was my name not listed as "reporting" the bug?

We did not learn about the issue from this bug, we heard about the issue in
online chatter (for example from Ryan's post). I searched for bugs first to
avoid filing a duplicate and found yours. If my search had been better I should
have found Nik Unger's bug 1302640 instead."
So, please get another bug report number and cve number and have that bug report and that cve number list - Reporter: Ryan Duff or Reporter: Online Chatter or Reporter: Nik Unger. But please promptly remove my bug report number and CVE number. Thank you for your consideration and expediency in this matter.
For what it's worth, I did report the certificate pinning expiration piece (the actual bug) to Jeff Bryner (Director of InfoSec at Mozilla) before it was posted anywhere publicly. However, I wasn't the one who found that first (that credit goes to Nicolas Vigier at The Tor Project who privately reported it to me). Because of that, I requested my name be removed from the CVE. It's been replaced with "Multiple People" which I feel accurately reflects the situation because this was a complicated vulnerability and involved multiple pieces being reported.
Totally agree with you Ryan. That is an excellent resolution. Thank you for your honesty , consistency and humility. "Reporter: Multiple Persons" reflects transparency on Mozilla's end.



(In reply to Ryan Duff from comment #26)
> For what it's worth, I did report the certificate pinning expiration piece
> (the actual bug) to Jeff Bryner (Director of InfoSec at Mozilla) before it
> was posted anywhere publicly. However, I wasn't the one who found that first
> (that credit goes to Nicolas Vigier at The Tor Project who privately
> reported it to me). Because of that, I requested my name be removed from the
> CVE. It's been replaced with "Multiple People" which I feel accurately
> reflects the situation because this was a complicated vulnerability and
> involved multiple pieces being reported.
Totally agree with you Ryan.That is an excellent resolution.Thank you for your honesty , consistency and humility. "Reporter:Multiple People" reflects transparency on Mozilla's end.






(In reply to Ryan Duff from comment #26)
> For what it's worth, I did report the certificate pinning expiration piece
> (the actual bug) to Jeff Bryner (Director of InfoSec at Mozilla) before it
> was posted anywhere publicly. However, I wasn't the one who found that first
> (that credit goes to Nicolas Vigier at The Tor Project who privately
> reported it to me). Because of that, I requested my name be removed from the
> CVE. It's been replaced with "Multiple People" which I feel accurately
> reflects the situation because this was a complicated vulnerability and
> involved multiple pieces being reported.
You need to log in before you can comment on or make changes to this bug.