Open Bug 1478280 Opened 6 years ago Updated 2 years ago

samesite=strict prevents reading the cookie after a xhr request

Categories

(Core :: DOM: Security, defect, P3)

Product:
Core
Component:
DOM: Security
Version:
61 Branch
Type:
defect

Tracking

()

People

(Reporter: alexanderbh, Unassigned)

References

Details

(Whiteboard: [domsecurity-backlog1])

Attachments

(1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36 Steps to reproduce: 1. Make a xhr GET request to an URL that responds with a Set-Cookie xxx; samesite=strict header. 2. After the request is completed access document.cookie Actual results: The cookie from the GET request is not in document.cookie Expected results: I expect to be able to read the cookie. I am able to read the cookie if the request responds with samesite=lax.
Summary: Samesite strict prevents reading the cookie after an xhr request → Samesite strict prevents reading the cookie after a xhr request
Summary: Samesite strict prevents reading the cookie after a xhr request → samesite=strict prevents reading the cookie after a xhr request
NOTE: It works if I refresh the page manually after the XHR request is done. Then I can actually read the cookie. But not after the XHR request is done without refresh.
Hi, This is a bit technical for me, but I will give it a shot if you can tell me the exact steps you used to make the XHR GET request and what is the URL you were using that had the "samesite=strict" header.
Flags: needinfo?(alexanderbh)
It is not technical I think. But it requires you to make a XHR request where the website and the endpoint is on the same host. And then have the host response with a samesite=strict cookie. When you have that the following code will show that the cookie cannot be read: let xsrfRequest = { uri: "endpoint on the same host as this request is made from", options: { method: "GET", credentials: "include" } }; fetch(xsrfRequest.uri, xsrfRequest.options) .then(() => { console.log(document.cookie); // The cookie from the response above is not here! });
Hi, this is a bit to advanced for me, but I think the Network > Cookies component might be a start for this issue.
Component: Untriaged → Networking: Cookies
Product: Firefox → Core
Hello :ckerschb I saw you solved lots of samesite issues. Do you have any idea of this? Thanks.
Flags: needinfo?(ckerschb)
302 to :mgoodwin who is the right person for same-site questions. I am happy to help implement changes if needed though.
Flags: needinfo?(ckerschb) → needinfo?(mgoodwin)
:mgoodwin - can you comment?
(In reply to alexanderbh from comment #3) > It is not technical I think. But it requires you to make a XHR request where > the website and the endpoint is on the same host. And then have the host > response with a samesite=strict cookie. > > When you have that the following code will show that the cookie cannot be > read: > > let xsrfRequest = { > uri: "endpoint on the same host as this request is made from", > options: { > method: "GET", > credentials: "include" > } > }; > fetch(xsrfRequest.uri, xsrfRequest.options) > .then(() => { > console.log(document.cookie); // The cookie from the response > above is not here! > }); I'm not sure whether I'm misunderstanding the problem or just struggling to reproduce it. Can you try the following for me, please: * open the document in a new tab and open the devtools. * Switch to the "network" tab. * Navigate to https://computerist.org/1478280.html - observe that the request to this document has no cookies set. * Switch to the console tab in the devtools toolbox. * Press the button. Is this illustrating the behavior you want?
Flags: needinfo?(mgoodwin)
Aha. It occurs to me that this could be racy - if the cookie is set on the document *after* the XHR triggers the callback, it'll be empty.
Testing this on OS X confirms the behavior differs sometimes. I'd be greatly surprised if this is specific to SameSite and I'm not sure it's necessarily even a bug (is the order of the change to document.cookie / invocation of the callback even defined?). I'll see what I can discover.
I've updated the test document I linked to in comment #8 to include a similar test for a non-SameSite cookie. My intention was to test whether the behaviour is the same for all cookies that are set via XHR. Unfortunately I can no longer reproduce this issue on OS X today. (In reply to Mark Goodwin [:mgoodwin] from comment #10) (is the order of the change to document.cookie / invocation of > the callback even defined?). My reading of the spec suggests it may not be defined behaviour. Regardless, Chromium, Safari and Edge seem to behave consistently in the cookies being available; this, plus the fact that cookie values can't be extracted from the response headers, suggest to me that we should probably treat this as a bug. I'm pretty sure it's not SameSite specific, though.
(In reply to Mark Goodwin [:mgoodwin] from comment #11) > ... cookies that are set via XHR ... * via Fetch
Hi Kershaw, could you confirm my suspicion that this isn't a SameSite specific issue?
Flags: needinfo?(kershaw)
(In reply to Mark Goodwin [:mgoodwin] from comment #13) > Hi Kershaw, could you confirm my suspicion that this isn't a SameSite > specific issue? I agree with you this looks like not a SameSite specific issue. But I'd like to make sure if the test document in comment #8 is the same case as the reporter encountered. Hi Reporter, Could you take a look at comment #8 and let us know what do you think? Thanks.
Flags: needinfo?(kershaw)
I just found that I can reproduce the issue with the test html in comment #8. Assign to myself.
Assignee: nobody → kershaw
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
:mgoodwin, I found this is really a Samesite specific issue. With the test html in comment #8, the problem is that sometimes NS_IsSameSiteForeign(aChannel, aHostURI) [1] returns true and causes CookieService skip the Samesite cookie [2]. However, NS_IsSameSiteForeign should return false. In the case that NS_IsSameSiteForeign returns true, I found that the triggeringPrincipal's uri of the channel [3] is "moz-nullprincipal:{52c34ec8-649b-7e46-ac1d-259c021ce957}", so IsThirdPartyChannel thinks this is a cross origion request. Honestly, I have no idea why we could get a moz-nullprincipal uri. I think maybe we should treat this as an empty uri and this could also fix this bug. What do you think? [1] https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/netwerk/cookie/CookieServiceChild.cpp#580 [2] https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/netwerk/cookie/CookieServiceChild.cpp#408 [3] https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/netwerk/base/nsNetUtil.cpp#2180
Flags: needinfo?(mgoodwin)
(In reply to Kershaw Chang [:kershaw] from comment #16) > Honestly, I have no idea why we could get a moz-nullprincipal uri. I think > maybe we should treat this as an empty uri and this could also fix this bug. I'm also not clear on why we'd be getting a moz-nullprincipal URI; probably :ckerschb would have more of an idea. > What do you think? Surely treating this as an empty URI means that we'd be missing the check on *actual* cross site requests?
Flags: needinfo?(mgoodwin) → needinfo?(ckerschb)
(In reply to Kershaw Chang [:kershaw] from comment #16) > I found this is really a Samesite specific issue. > With the test html in comment #8, the problem is that sometimes > NS_IsSameSiteForeign(aChannel, aHostURI) [1] returns true and causes > CookieService skip the Samesite cookie [2]. What does 'sometimes' mean in that case? Same input variables or different input variables? Or is the code racy? > However, NS_IsSameSiteForeign should return false. > In the case that NS_IsSameSiteForeign returns true, I found that the > triggeringPrincipal's uri of the channel [3] is > "moz-nullprincipal:{52c34ec8-649b-7e46-ac1d-259c021ce957}", so > IsThirdPartyChannel thinks this is a cross origion request. > > Honestly, I have no idea why we could get a moz-nullprincipal uri. I think > maybe we should treat this as an empty uri and this could also fix this bug. A moz-nullprincipal usually reflects the security context of a opaque origin (e.g data: URI) or a sandboxed iframe. The NullPrincipal (which holds a URI of moz-nullprincipal:{...}) is only same-origin within itself and with nothing else. We sometimes use a NullPrincipal as a secure default in case we can't query the correct Principal for whatever reasons. E.g. when deserializing Principals and something goes wrong. If we have a live testcase then we could certainly trace this on down. Only looking at the testcode in comment 8 doesn't provide enough context to say where exactly the problem happens.
Flags: needinfo?(ckerschb)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #18) > What does 'sometimes' mean in that case? Same input variables or different > input variables? Or is the code racy? The code could be racy, or it could be something platform specific (I suspect the former but I could be wrong); when I was testing, I saw the problem on OS X but not Windows. I've since tried on Windows again and I seem to have the issue there too. Kershaw what OS were you testing on?
Flags: needinfo?(kershaw)
(In reply to Mark Goodwin [:mgoodwin] from comment #19) > (In reply to Christoph Kerschbaumer [:ckerschb] from comment #18) > > What does 'sometimes' mean in that case? Same input variables or different > > input variables? Or is the code racy? > > The code could be racy, or it could be something platform specific (I > suspect the former but I could be wrong); when I was testing, I saw the > problem on OS X but not Windows. I've since tried on Windows again and I > seem to have the issue there too. Kershaw what OS were you testing on? I was testing on OSX. Let me show you how do I reproduce this. 1. Run firefox with a whole new profile. 2. Navigate to https://computerist.org/1478280.html. 3. Open the console and click the "Samesite" button. You should see "document.cookie is: foo=bar; document is: https://computerist.org/1478280.html". 4. Close the firefox and start it again. 5. Repeat step 2 and 3. You should see "document.cookie is: document is: https://computerist.org/1478280.html". The cookie is missing because th request "https://computerist.org/same_site/test.txt" is loaded from cache and we don't cache sookie headers, so I think this is normal. 6. Leave the "1478280.html" tab open and go to preferences and clear all the caches and cookies. 7. Go back to "1478280.html" tab and clear the "Samesite" button again. You can see that the cookie string "foo=bar" is still missing, but the cookie string "wibble=blah" for non-samesite is still there. I can 100% reproduce with the steps above. I think maybe the NullPrincipal is caused because I cleared the cookies and caches at step 6? Anyway, I'll take a look why we got a NullPrincipal in channel's loadinfo. Or maybe someone knows? The reason why I use "sometimes" is that I think this is not the only case that a channel having a null triggering principal. Maybe we have more cases.
Flags: needinfo?(kershaw)
I finally figured out where the moz-nullprincipal uri was coming from. It's actually from [1]. Aparently, some documents could be loaded with null triggering principal. So, the problem is: If a document is loaded with a null triggering principal and it wants to read a Samesite cookie, cookie service will not return the cookie back because |isSameSiteForeign| at [2] is true. The reason why NS_IsSameSiteForeign returns true is that we use moz-nullprincipal [3] to check if this is a third party channel. I am not sure how to deal with moz-nullprincipal uri in NS_IsSameSiteForeign. Christoph, what do you think? [1] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/browser/components/sessionstore/ContentRestore.jsm#200-210 [2] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/netwerk/cookie/CookieServiceChild.cpp#580 [3] https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/netwerk/base/nsNetUtil.cpp#2180
Flags: needinfo?(ckerschb)
(In reply to Kershaw Chang [:kershaw] from comment #21) > I finally figured out where the moz-nullprincipal uri was coming from. It's > actually from [1]. > Aparently, some documents could be loaded with null triggering principal. Thanks for digging into the problem and finding the root cause. So it seems that Bug 1490257 regressed that. Within that bug we replaced the 'null' fallback for the triggeringPrincipal and created an explicit NullPrincipal. Now in the old world, the 'null' was passed all the way to docshell and got translated into a SystemPrincipal. You could check if that causes the problem by changing the following line within ContentRestore.jsm let triggeringPrincipal = loadArguments.triggeringPrincipal ? Utils.deserializePrincipal(loadArguments.triggeringPrincipal) - : Services.scriptSecurityManager.createNullPrincipal({}); + : Services.scriptSecurityManager.getSystemPrincipal()); Now moving forward, it would be interesting to figure out why there was no triggeringPrincipal set on the loadArguments. I guess we should file a follow up and investigate. If the above change fixes the problem then I think we should just perform that change, so we can uplift that change and then follow up on it.
Flags: needinfo?(ckerschb)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #22) > (In reply to Kershaw Chang [:kershaw] from comment #21) > > I finally figured out where the moz-nullprincipal uri was coming from. It's > > actually from [1]. > > Aparently, some documents could be loaded with null triggering principal. > > Thanks for digging into the problem and finding the root cause. So it seems > that Bug 1490257 regressed that. Within that bug we replaced the 'null' > fallback for the triggeringPrincipal and created an explicit NullPrincipal. > Now in the old world, the 'null' was passed all the way to docshell and got > translated into a SystemPrincipal. > > You could check if that causes the problem by changing the following line > within ContentRestore.jsm > > let triggeringPrincipal = loadArguments.triggeringPrincipal > ? > Utils.deserializePrincipal(loadArguments.triggeringPrincipal) > - : > Services.scriptSecurityManager.createNullPrincipal({}); > + : > Services.scriptSecurityManager.getSystemPrincipal()); > > Now moving forward, it would be interesting to figure out why there was no > triggeringPrincipal set on the loadArguments. I guess we should file a > follow up and investigate. If the above change fixes the problem then I > think we should just perform that change, so we can uplift that change and > then follow up on it. Actually, the triggeringPrincipal was set in the loadArguments. The null principal is coming from [1]. The problem in this bug is fixed with the following change. --- a/browser/components/newtab/lib/PlacesFeed.jsm +++ b/browser/components/newtab/lib/PlacesFeed.jsm @@ -237,17 +237,17 @@ class PlacesFeed { } /** * Open a link in a desired destination defaulting to action's event. */ openLink(action, where = "", isPrivate = false) { const params = { private: isPrivate, - triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({}), + triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), }; // Always include the referrer (even for http links) if we have one const {event, referrer, typedBonus} = action.data; if (referrer) { params.referrerPolicy = Ci.nsIHttpChannel.REFERRER_POLICY_UNSAFE_URL; params.referrerURI = Services.io.newURI(referrer); } However, I am not suref if the correct way to fix this bug is changing all the places where triggeringPrincipal is set to null principal. Or should we just add another check for moz-nullprincipal uri in NS_IsSameSiteForeign? What do you think, Christoph? [1] browser/components/newtab/lib/PlacesFeed.jsm#245
Flags: needinfo?(ckerschb)
(In reply to Kershaw Chang [:kershaw] from comment #24) > However, I am not suref if the correct way to fix this bug is changing all > the places where triggeringPrincipal is set to null principal. Or should we > just add another check for moz-nullprincipal uri in NS_IsSameSiteForeign? First, it's good that this change fixes the problem. I think we should just land that which converts that particular change introduced within Bug 1490257. And then we need to uplift that change, right? Second, I am slightly confused by what you mean with 'changing all the places' - what are all the places? Are there similar problems that I am not aware of or is it just that particular one? Third, we need to file a follow up bug which provides the correct triggeringPrincipal within PlacesFeed.jsm. Could you please file that and ni? jonathan kingston on it. He can help with fixing. Thanks!
Flags: needinfo?(ckerschb)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #25) > (In reply to Kershaw Chang [:kershaw] from comment #24) > > However, I am not suref if the correct way to fix this bug is changing all > > the places where triggeringPrincipal is set to null principal. Or should we > > just add another check for moz-nullprincipal uri in NS_IsSameSiteForeign? > > First, it's good that this change fixes the problem. I think we should just > land that which converts that particular change introduced within Bug > 1490257. And then we need to uplift that change, right? > > Second, I am slightly confused by what you mean with 'changing all the > places' - what are all the places? Are there similar problems that I am not > aware of or is it just that particular one? > > Third, we need to file a follow up bug which provides the correct > triggeringPrincipal within PlacesFeed.jsm. Could you please file that and > ni? jonathan kingston on it. He can help with fixing. > > Thanks! I'd rather fix the code in PlacesFeed.jsm in this bug since I am sure doing this can fix the problem here. I can't really tell if the changes made in Bug 1490257 could cause another potential problem. I think we should file a follow up bug for this.
Priority: -- → P2
Whiteboard: [necko-triaged]
When a document opened with a null triggeringPrincipal tries to read samesite cookies, this will fail because NS_IsSameSiteForeign returns true. To fix this, we have to provide the correct triggeringPrincipal at the first place where the document is loaded.
Given that the root cause of this bug is about triggeringPrincipal, it looks like I am not the right one to fix this bug. So, set the componment to dom:security and unassign myself.
Assignee: kershaw → nobody
Status: ASSIGNED → NEW
Component: Networking: Cookies → DOM: Security
Priority: P2 → --
Whiteboard: [necko-triaged]
Attachment #9028242 - Attachment is obsolete: true
Flags: needinfo?(alexanderbh)
Blocks: 1490257
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]

We are kinda experiencing a similiar problem but only when you get redirected from another domain to the domain which is setting the cookie.

So I can reproduce this if I have domain A (http://redirect.test.local) redirect by 302 to domain B (http://otherdomain.bug.local). After arriving on the http://otherdomain.bug.local we do a XHR call to an api which returns a Set-Cookie: test=test; domain=otherdomain.bug.local; path=/; samesite=strict then document.cookie will be empty. If you go directly to http://otherdomain.bug.local then we don't experience the problem.

I have a HAR file which shows the most simple flow I could make to reproduce it. If that's useful information please let me know and I will attach it.
I tested this with version 67.0 up to nightly.

Since I see nothing in this issue saying anything about redirects ect. is this the same bug or do I need to log a different one?

Flags: needinfo?(ckerschb)

(In reply to rutgerstorm from comment #29)

Since I see nothing in this issue saying anything about redirects ect. is this the same bug or do I need to log a different one?

I am pretty sure we are facing the same underlying issue as outlined in comment 27 and 28.

Jonathan, you have been working on providing the correct triggeringPrincipal for all top-level loads (Bug 1333030 and dependencies). It seems that in some cases we are falling back to NullPrincipal which causes to break same-site strict cookies.

Could you please take a look - I think we should fix the underlying triggeringPrincipal problem.

Flags: needinfo?(ckerschb) → needinfo?(jkt)

We have the same issue. We use an angularjs webapp. When samesite was set to strict angularjs was unable the read the cookie. With samesite set to lax everything works fine. It doesn't occur when working with a localhost uri. This is 100% reproducible in our code.

We are also seeing this. Though there are no XHR requests involved (so might be different, though sounds very similar). Angularjs webapp.

We do a redirect from A (sub.redirect-from.com) to B (sub.redirect-to.com), inside redirect-to.com we do:

document.cookie = 'test_cookie=true; domain=sub.redirect-to.com; path=/; samesite=strict; secure';
document.location.reload()

The reload does NOT send the cookie to the backend, and this cookie is ALSO not available in document.cookie causing this code to just infinitely reload. A user-reload will not fix it, but going to the URL bar and pressing enter does allow the document.cookie to be read.

Christoph, is Jonathan unavailable?

Flags: needinfo?(ckerschb)

(In reply to Tom Schuster [:evilpie] from comment #33)

Christoph, is Jonathan unavailable?

I just had a quick sync with Jonathan, he will take a look asap, but probably not before next week. Sorry for the delay here.

Flags: needinfo?(ckerschb)

From memory I think this was a triggering principal issue but I never got around to fixing it. Removing NI sorry.

Flags: needinfo?(jonathan)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: