Closed
Bug 1305592
Opened 8 years ago
Closed 8 years ago
CloudFlare's "__cfduid" cookie is loading in the default container
Categories
(Core :: DOM: Security, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla52
People
(Reporter: kjozwiak, Assigned: timhuang)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [userContextId][domsecurity-backlog][OA])
Attachments
(3 files, 1 obsolete file)
21.97 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
mcmanus
:
review+
gchang
:
approval-mozilla-aurora+
|
Details |
203.59 KB,
image/png
|
Details |
It looks like one of the "__cfduid" cookies that CloudFlare uses is always being loaded in the default container. When you load a website that uses CloudFlare inside a container, two "__cfduid" cookies will be created. The first cookie will load inside the container that's currently being used, the second one will always load in the default container. The "__cfduid" cookie that loads into the default container seems to differ for each website, so it's never the same. I'm not really sure if this could be used to fingerprint users across different containers as the cookies differ. I've searched around and it looks like the "__cfduid" cookie is used to override any security restrictions based on the IP address the visitor is coming from. [1] STR: * open a fresh install/profile of m-c * open a personal container via "file -> new container tab" * load https://www.reddit.com * open "about:preferences#privacy" inside a new tab * take a look "redditstatic.com" and you'll notice that the "__cfduid" cookie loaded inside the default (None) container [1] https://support.cloudflare.com/hc/en-us/articles/200170156-What-does-the-CloudFlare-cfduid-cookie-do-
Updated•8 years ago
|
Priority: -- → P1
Whiteboard: [userContextId][domsecurity-backlog] → [userContextId][domsecurity-backlog][OA]
Reporter | ||
Comment 2•8 years ago
|
||
Alternative STR (a little easier to see the cookie under about:preferences#privacy): * clear history and make sure you're using a fresh profile using m-c * open a new personal container via File -> New Container Tab -> Personal" * load https://www.meetup.com/ in the personal container * go into about:preferences#privacy and take a look at the meetup.com container in the cooke manager You'll see two "__cfduid" cookies. One being loaded in the "Personal" container and the other being loaded in the "None" container. Video of the potential issue occurring: * https://youtu.be/ewk8Z19yHBg
I ran the STR through an intercepting proxy (burp suite/ZAP etc) to see what request caused the cookie to be set. From this I was able to see that the cookie in question was a request to /favicon.ico. So maybe this is a regression of bug 1270678? (In reply to Kamil Jozwiak [:kjozwiak] from comment #2) > Alternative STR (a little easier to see the cookie under > about:preferences#privacy): > > * clear history and make sure you're using a fresh profile using m-c > * open a new personal container via File -> New Container Tab -> Personal" > * load https://www.meetup.com/ in the personal container > * go into about:preferences#privacy and take a look at the meetup.com > container in the cooke manager > > You'll see two "__cfduid" cookies. One being loaded in the "Personal" > container and the other being loaded in the "None" container. > > Video of the potential issue occurring: > * https://youtu.be/ewk8Z19yHBg
Comment 4•8 years ago
|
||
I don't think it's bug 1280590. That one is about session restore and how nsICookieManager2.cookieExists() works. Tim, do you mind to take a look at this bug and see if it's a regression of bug 1270678?
Flags: needinfo?(amarchesini) → needinfo?(tihuang)
Assignee | ||
Comment 5•8 years ago
|
||
There are two different favicon requests, one is triggered by Places library and another is triggered by XUL image. The Bug 1270678 actually fixes the Places library to use correct originAttributes when loading favicon, but the part of XUL image still has a problem that it uses default originAttributes to load favicon since XUL uses system principal. So, this problem happens whenever the favicon tries to set some cookies. The Bug 1277803 is trying to fix this issue. So, I think it is not a regression of bug 1270678.
Flags: needinfo?(tihuang)
Comment 6•8 years ago
|
||
Does this mean this is a duplicate of bug 1277803? Or maybe dependent on bug 1277803. Do you think fixing that will resolve this bug too?
Flags: needinfo?(tihuang)
Assignee | ||
Comment 7•8 years ago
|
||
Yes, I believe that fixing Bug 1277803 will resolve this bug. So this should be dependent on Bug 1277803.
Depends on: 1277803
Flags: needinfo?(tihuang)
Reporter | ||
Comment 8•8 years ago
|
||
I've managed to reproduce this behaviour in PB while using codepen.io [1] but I couldn't reproduce it with the other websites that have been mentioned in comment#0 and comment#2. When loading codepen.io in a PB window with the latest version of m-c, you'll see the following cookie being created under about:preferences#privacy: * "1","codepen.io","","__cfduid","d0b05a42f5623362aea987f6d9d0798641475695849",".codepen.io","/" etc... I've also created a video to illustrate the problem: * https://youtu.be/_7BV_wXay0A STR: * launch a clean profile using the latest version of m-c * ensure you have no cookies listed under about:preferences#privacy * open a new PB window via the hamburger menu * load codepen.io [1] into the PB window * open about:preferences#privacy and click on "remove individual cookies.." * you'll notice a "__cfduid" cookie that was created using the default container (None) * restart m-c and take a another look under about:preferences#privacy and you'll notice that the cookie hasn't been removed [1] http://codepen.io/#
Assignee | ||
Comment 9•8 years ago
|
||
I've tested websites in comment#0 and comment#2 with patches of Bug 1277803. The result shows that there is no more '__cfduid' cookie in the default container. So, the Bug 1277803 does resolve the issue of the favicon. However, it doesn't resolve this 'http://codepen.io/' issue. And I found that the '__cfduid' here comes from 'http://codepen.io/.well-known/http-opportunistic', but not the favicon. I have no clue that where and how this link been loaded. I will dig into it more.
Updated•8 years ago
|
Assignee: nobody → tihuang
Assignee | ||
Comment 10•8 years ago
|
||
This link, 'http://codepen.io/.well-known/http-opportunistic', is loaded by the WellKnownChecker [1], which uses the system principal to load. So, it will have the default originAttributes. However, I think I could fix this by assigning the originAttributes into the loadInfo. This makes the WellKnownChecker using correct originAttributes with the system principal. However, I am wondering that do we have to use the system principal here? Maybe we could use the content principal since we have the origin and the originAttributes. So we could use the BasePrincipal::CreateCodebasePrincipal() to create the content principal, using this principal to load. Patrick, What do you think about this? [1] http://searchfox.org/mozilla-central/source/netwerk/protocol/http/AlternateServices.cpp#900
Flags: needinfo?(mcmanus)
Assignee | ||
Comment 11•8 years ago
|
||
Comment 12•8 years ago
|
||
I think Tim's patch in comment 11 is the right approach. Thanks all. I think 51 would also be impacted.
Flags: needinfo?(mcmanus)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8805014 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. https://reviewboard.mozilla.org/r/89132/#review89408
Attachment #8805417 -
Flags: review?(mcmanus) → review+
Comment 16•8 years ago
|
||
mcmanus could you fix also the review flag on mozreview so that we can push this via autoland, thanks!
Flags: needinfo?(mcmanus)
Comment 17•8 years ago
|
||
mozreview-review |
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. https://reviewboard.mozilla.org/r/89130/#review89700 ship it
Comment 18•8 years ago
|
||
mozreview-review |
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. https://reviewboard.mozilla.org/r/89132/#review89698
Comment 19•8 years ago
|
||
tomcat I have no idea how to set the landable flag. howto?
Flags: needinfo?(mcmanus)
Comment 20•8 years ago
|
||
(In reply to Patrick McManus [:mcmanus] from comment #19) > tomcat I have no idea how to set the landable flag. howto? the mozreview ui says "A suitable reviewer has not given a "Ship It!"" i guess its releated to http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/reviewboard.html#reviewing-code maybe mark can help here more :)
Flags: needinfo?(mcote)
Updated•8 years ago
|
Keywords: checkin-needed
Comment 21•8 years ago
|
||
Patrick: to be landable, someone with L3 access has to give an r+. If you do have L3, then your account is likely not associated with your LDAP entry. You can fix that via a simple ssh command: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install.html#manually-associating-your-ldap-account-with-mozreview.
Flags: needinfo?(mcote)
Comment 22•8 years ago
|
||
thanks mark. That script seemed to make it r+ landable.
Assignee | ||
Comment 23•8 years ago
|
||
Now, the review flag is working correctly. Check in this again.
Keywords: checkin-needed
Comment 24•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c669673b4192 Make the WellKnownChecker to use correct originAttributes. r=mcmanus
Keywords: checkin-needed
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c669673b4192
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. Approval Request Comment [Feature/regressing bug #]: 1305592 [User impact if declined]: The opportunistic encryption will use incorrect cookies when users use different containers other than the default one. [Describe test coverage new/current, TreeHerder]: None [Risks and why]: Low risk since the change is minor. [String/UUID change made/needed]: None
Attachment #8805417 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 27•8 years ago
|
||
Sorry, the 'Feature/regressing bug #' should be 1301117
Comment 28•8 years ago
|
||
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. This patch can prevent users from using incorrect cookies. Take it in 51 aurora.
Attachment #8805417 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•8 years ago
|
status-firefox51:
--- → affected
Comment 29•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/d6580e1fcc08
Reporter | ||
Comment 30•8 years ago
|
||
I went through the verification process using the original test cases: * bug#1305592comment#0 (reddit.com) * bug#1305592comment#1 (meetup.com) * bug#1305592comment#8 (codepen.io) In addition the the original test cases, I also went through the following websites and ensured that all the cookies were being loaded in the appropriate container: * upwork.com, fiverr.com, medium.com, getbootstrap.com, laravel.com Platforms being used: * Win 10 x64 - PASSED ** fx52.0a1, buildId: 20161111030203, changeset: d38d06f85ef5 * macOS 10.12.1 x64 - PASSED ** fx52.0a1, buildId: 20161111030203, changeset: d38d06f85ef5 * Ubuntu 16.04.1 LTS x64 - PASSED ** fx52.0a1, buildId: 20161111030203, changeset: d38d06f85ef5
Reporter | ||
Comment 31•8 years ago
|
||
It seems like this hasn't beed fixed in fx51 which is now in the beta channel. It's working correctly under fx53.0a1 and fx52.0a2. I went through the following test case: * install the latest version of fx51.0b1 * enable containers via privacy.userContext.enabled;true * install the SQLite Manager add-on ** this is optional, you can just use the cookie manager via about:preferences * load codepen.io inside any container You'll notice that even though codepen.io was being loaded within a container, there's two "__cfduid" cookies being loaded from codepen.io. One is being loaded via "^userContextId=1" and the other is being loaded via the default container: * "1","codepen.io","^userContextId=1","__cfduid","dc53bfe86b7e8cb6aefda82daa636eef71479234693",".codepen.io"........ * "14","codepen.io","","__cfduid","dedb56e30431a36ffbadd943f39bd37201479234697",".codepen.io"...... When you go through the above examples using fx53.0a1 or fx52.0a2, all of the cookies are using the correct userContextId. Tim, would you be able to take a quick look and see what's happening when you get back? As per comment#29, this should be working correctly in fx51.0b1. Are we missing patches in the beta channel that might be causing this not to work as expected? Builds being used: * fx53.0a1, buildId: 20161115030213, changeset: 5e7676832766 - PASSED ** https://archive.mozilla.org/pub/firefox/nightly/2016/11/2016-11-15-03-02-13-mozilla-central/ * fx52.0a2, buildId: 20161115004018, changeset: 401aa89aeb11 - PASSED ** https://archive.mozilla.org/pub/firefox/nightly/2016/11/2016-11-15-00-40-18-mozilla-aurora/ * fx51.0b1, buildId: 20161114042748, changeset: f455459b2ae5 - FAILED ** https://archive.mozilla.org/pub/firefox/candidates/51.0b1-candidates/
Flags: needinfo?(tihuang)
Reporter | ||
Comment 32•8 years ago
|
||
Assignee | ||
Comment 33•8 years ago
|
||
I have checked the code. The wellKnownChecker is fixed in the beta, but the favicon does not. So, this cookie must be set through the favicon. I suppose that we have to uplift patches in Bug 1270678 for the favicon as well to fix this problem in beta.
Flags: needinfo?(tihuang)
Reporter | ||
Comment 34•8 years ago
|
||
Thoughts? Should we just leave this be and let the patches ride the trains? As Tim mentioned, we would need to uplift the patches from Bug#1270678. Bug#1270678 looks like it's also dependent on several other patches which might complicate things even further and increase the risk of regressing.
Flags: needinfo?(tanvi)
Comment 35•8 years ago
|
||
That's okay. Lets leave this in 52+, as long as its fixed properly there.
Flags: needinfo?(tanvi)
Reporter | ||
Comment 36•8 years ago
|
||
(In reply to Tanvi Vyas - behind on bugzilla [:tanvi] from comment #35) > That's okay. Lets leave this in 52+, as long as its fixed properly there. Sounds good. I'll mark this as verified as Tim confirmed it's correctly fixed in fx51 in comment#33.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•