Closed Bug 1305592 Opened 3 years ago Closed 3 years ago
Flare's "__cfduid" cookie is loading in the default container
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.  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  https://support.cloudflare.com/hc/en-us/articles/200170156-What-does-the-CloudFlare-cfduid-cookie-do-
Priority: -- → P1
Whiteboard: [userContextId][domsecurity-backlog] → [userContextId][domsecurity-backlog][OA]
Could this be the same issue as Bug 1280590?
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
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)
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.
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?
Yes, I believe that fixing Bug 1277803 will resolve this bug. So this should be dependent on Bug 1277803.
Depends on: 1277803
I've managed to reproduce this behaviour in PB while using codepen.io  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  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  http://codepen.io/#
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.
3 years ago
Assignee: nobody → tihuang
This link, 'http://codepen.io/.well-known/http-opportunistic', is loaded by the WellKnownChecker , 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?  http://searchfox.org/mozilla-central/source/netwerk/protocol/http/AlternateServices.cpp#900
I think Tim's patch in comment 11 is the right approach. Thanks all. I think 51 would also be impacted.
Attachment #8805014 - Attachment is obsolete: true
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+
Try looks good.
mcmanus could you fix also the review flag on mozreview so that we can push this via autoland, thanks!
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. https://reviewboard.mozilla.org/r/89130/#review89700 ship it
Comment on attachment 8805417 [details] Bug 1305592 - Make the WellKnownChecker to use correct originAttributes. https://reviewboard.mozilla.org/r/89132/#review89698
tomcat I have no idea how to set the landable flag. howto?
(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 :)
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.
thanks mark. That script seemed to make it r+ landable.
Now, the review flag is working correctly. Check in this again.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/c669673b4192 Make the WellKnownChecker to use correct originAttributes. r=mcmanus
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?
Sorry, the 'Feature/regressing bug #' should be 1301117
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+
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
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/
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.
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.
That's okay. Lets leave this in 52+, as long as its fixed properly there.
(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.
You need to log in before you can comment on or make changes to this bug.