-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FireMonkey] should honor current container cookies #378
Comments
By default, they dont send
Note: Manually grabbing FireMonkey currently does not manually add Greasemonkey manually grabs and adds the Violentmonkey also appears to manually grab & add the cookies at all times (container aware) but also has
Tampermonkey also appears to manually grab & add the cookies at all times but also has
|
Not a problem, right? All UserScript manager require cookies permission. Except FM, right now. As you noticed, all of them add cookies by default, This shows their intentions to pass authentication the right way, including the obvious privacy concern of not sending cookies from normal windows when in private mode. About the specificity of containers, both VM and TM honor the container of current tabs as it's pretty obvious they all should do, Not doing it is a bug that need to be fixed, as stated in TM release notes:
There's no sense in thinking that the right approach is that a script loaded in container tab should reuse cookies from normal tab by default, this is clearly wrong and even a privacy issue. Containers are designed to be isolated scopes from each others. Regarding GM... well... we all know that GM is essentially dead in development since 2018 and containers is a relatively new feature (started in like 2017), so it doesn't really mean much. |
You wont believe how much grief developers get from users over permissions. I have even had complaints about 'notifications' permission. Most users are not aware of permission risks, or lack thereof. The notices on AMO are sometimes more daunting than what they really are.
I have always been concerned about security in FireMonkey, therefore, I am not very keen on that idea. 📌 However, AFA containers and private browsing, I am currently discussing it with Firefox Security team and will try to implement. Further information: |
How should container XHR work?
When sending HTTP request to |
The UserScript is attached to the include/match rule. FM calls for Unless explicitly stated otherwise in the code via I can't imagine a reason to think that it's right for an UserScript to, by default, pass cookies from normal browsing when it was called from a webpage loaded in private mode. It's clearly a bug, a leak. And the same logic applies to containers. Containers are isolated from each others just like private browsing, the difference is that container data is stored. |
After talking with Mozilla security team, I am going to work something out on the cookies & private/container issue. |
Cookie isolation is done and will be in v2.35 |
After chatting with Mozilla security team, here is the latest .... GM xmlHttpRequest/fetch Cookies
|
Thanks. I only disagree with this:
There should be a way to set cookies in any mode. Or at least in container tabs. What if I have an UserScript that I want it to make requests using the same |
Originally, I had set it that way, but Mozilla team told me that "if the server replies with What do other managers do in container/private mode? Also ... Userscripts can't check if they are in container/private mode. Therefore the same userscript cookie will be sent in normal & container & private mode. |
What? I don't get it.
Tested in Violentmonkey, which in my opinion has the best code quality, and it does what it should in all cases, exactly as I'm requesting. By default, GM APIs honors the context of the tab (send default cookies when in default mode; send container cookies when in container tab...). And if I manually set So: GM.xmlHttpRequest({
url: 'http://www.example.com',
headers: {
'Test': 'right',
'Cookie': 'psiu=2',
}
}); In Violentmonkey, both
I'd say this has nothing to do with cookie isolation and the goal of this topic. Now we are talking about explicitly and deliberately user-defined cookies using |
Set-cookie is received on the HTTP response headers by the target server, not userscript HTTP request.
That would allow userscripts access to container/private browsing area. It maybe OK for the userscript developer but not for the users. Imagine userscripts sends some cookie that relates to a porn site.
Cookies sent via HTTP request are read (and can be stored) by the destination. That is the whole point of sending them. An example would be login cookies. Example: GM.xmlHttpRequest({
url: 'http://www.example.com',
headers: {
'Test': 'right',
'Cookies': 'visited=' + location.href,
}
}); |
I know that. My question is still up.
Access what?
Please, give an example of how this can be abused.
Everything sent is read by the server. They receive all headers plus the body in POST requests. Do what you say, UserScripts will still be able to send any data using other headers or request body. They can also read You said you implemented sending stored cookies for the next version. Cookies from normal mode are already sent by default. Cookies from containers will be sent by default in next version. Cookies from private browsing will be sent by default in next version (if the user has allowed FM to run in private). How is this different from allowing the UserScript to send custom cookies?
And what's the problem? Why should this be allowed when you're browsing in default container, but not in any other container? |
Currently, Firefox is isolating cookies of normal, container & private modes; for security and privacy reasons. Userscripts should follow the same principle. and isolate cookies of normal, container & private modes; for security and privacy reasons. If Firefox is not allowed to, should a userscript developer be allowed to? However, you can discuss it with Mozilla security team and if they agree, I will implement it. |
What I'm requesting doesn't break cookie isolation in any way. There's nothing being transmitted from one context to another. |
Cookie Isolation: Not sending the same cookie in different modes. Userscript is sending the same cookie in all modes. |
Cookie isolation: not leaking cookie from one mode to the other. Current FM doesn't work like this because default cookies are sent even when the script loads in a container tab or in private mode. So I opened this issue and now next version is supposed to fix it. When an UserScript wants to send a custom header like Remembering that sending |
As mentioned, it is done according to their recommendation. |
I'd bet you misunderstood their recommendation. Tried to find your chat in Discord, but it seems that you chatted elsewhere after starting there. Well, I give up. For me it makes almost no difference. As you may know, I use a fork of FM. I'll implement it as I think it's right and that's it. |
Just adding: I mentioned Violentmonkey, but tested now and Tampermonkey also has no problem with So it's a FM only issue. And for no understandable reason... |
📌 I am asking Mozilla security team:
Set-Cookie Reference:
Note:
|
And if the problem is when the server returns |
This was about a request for UI, for which it makes no sense every manager to be exactly the same. But you can see in multiple comments in VM's repo that they make efforts to keep up scripts compatibility compared to Tampermonkey, "which is the de facto standard" (said by VM's dev). |
I am also trying to verify it. |
I have already written the code to filter |
Latest update of FireMonkey Help Cookies IsolationBrowsing modes in Firefox can be divided into 3 distinct modes: normal, container, & private/incognito. JavaScript GM Normal Browsing Mode
Container or Private (Incognito) Browsing Mode
anonymous
|
It looks good, thank you. Just one detail: Both VM and TM don't "replace original Firefox cookies". Instead, they append/merge cookies set in Firefox has the cookies
Both VM and TM send But it would be good if there's also a way to do what you described: remove default cookies and send just the cookies set in Jfyi, still today Firefox works that way, there's a It would be a good feature for FM to have, an advantage over the others. |
TBH, I have been asking around about Firefox merging multiple cookie entries but got no answer. It appears that:
Are multiple Cookie headers allowed in an HTTP request? When testing, Firefox appear to only send one entry so only one of these are sent:
It would have been a lot easier to merge if Firefox would merge them. Merging the cookies manually is possible but adds more async processing.
Let me see what can be done .... |
Never seen something like two From what I understood from your update in Help, FM will replace cookie header entirely when And I suggested a new flag (default off) to allow to effectively replace cookie header value instead of merging. |
FM will manually set cookies anyway, right? This is needed to, for example, send container cookies when in a container tab. So I don't think it would add more processing. Simply put, currently (already written code for next version) it's:
I'm suggesting to be something like
or
Processing is the same. |
Yes.. flag with default
ATM, FM doesn't get the cookies if userscript has set them.. but to merge, it has to get it first. |
Right. But the most common and default is to get them. Because most requests in UserScripts doesn't set So it's not that it would add overhead, it will just do what it usually does. |
ATM,
|
Ah, right, I forgot that Fx send normal cookies by default. Looks like VM sends cookies even in normal tabs and this was never a problem. FM is already more efficient by not needing do get cookies for normal tabs. When the request has |
If the default is going to be merge, it would be better to have a flag that is NOT the default so only check if it is // mergeCookies with default false
if (init.hasOwnProperty('mergeCookies') && init.mergeCookies === false) {
// dont merge
}
// noMergeCookies with default true
if (init.noMergeCookies) {
// dont merge
}
You can also set Actually .... I might be able to use |
OK.
If you don't want to create a new property, you may use Also... for instance, |
So you propose to not send cookies by default? This is different from what you was proposing, also from how FM always behaved, not to mention other engines. Well, it's up to you. I understand your argument that other engines aren't following the spec. After all, no one is following because The only thing here is that this way you would increase FM's script incompatibility. There is already a lot of native incompatibility due to specificities of Fx's userScripts API, I use FM knowing that. I wouldn't recommend increasing this problem unnecessarily. But I understand your point. |
Actually, on testing, Firefox sends cookies all the time on XHR and So I have to rethink that. |
Yes. I forgot that. This explains why UserScripts were always that way. I guess the compatibility of some scripts is now safe in FM upon this fact. 🤭 So... reuse |
In other words, merge all the time unless anonymous is set to I need to update Help 🤦♂️
|
It is all done for v2.35
Therefore .....
|
Cookie Isolation also implemented in |
Are you planning on implement per-container UserScripts¹? It will be a major FireMonkey advantage. I've seen users asking about restricting extensions per container, and with that feature FM will be able to accommodate many cases. |
Yes ..... it will probably be for Firefox 97+ (release date 2022-02-08) A metadata entry format has to be decided for it. Maybe something like this:
or ...
Note:
|
v2.41 Note container (Firefox 97+)
Metadata Block
User Metadata
|
GM.xmlHttpRequest
andGM.fetch
always use "default" cookies, breaking scripts in container tab when cookie is relevant.For example, an UserScript performing
GM.fetch
in a tab from "Personal" container should, by default (when cookie isn't explicitly passed in{ headers }
), pass cookies from Personal store, not to leak cookies from default container. Cookies from current tab should always be used.This issue may affect private browsing too, but I haven't tested it.
For reference, Violentmonkey is doing it right, passing cookies from current tab container.
The text was updated successfully, but these errors were encountered: