Skip to content
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

Support whitelisting specific cookies #469

Closed
wants to merge 2 commits into from

Conversation

infinity0
Copy link

Some public websites use cookies for non-secure stuff like:

  • setting UI options such as mobile
  • setting some anonymous ID to help fight spam, e.g. fanyi.baidu.com

This PR allows the server operator to whitelist specific cookies used for these purposes.

@infinity0 infinity0 changed the title Support allowCookies Support whitelisting specific cookies Oct 4, 2023
@Rob--W
Copy link
Owner

Rob--W commented Oct 4, 2023

Cookie support is out of scope.

The proposed implantation is insecure, for more details see https://github.com/Rob--W/cors-anywhere/issues/400#issuecomment-1005116934 and many of other previous attempts (linked through the comments).

@Rob--W Rob--W closed this Oct 4, 2023
@infinity0
Copy link
Author

infinity0 commented Oct 7, 2023

The proposed implantation is insecure

It's only insecure if the server operator specifically chooses to whitelist security-relevant cookies. So is the whole idea of CORS-anywhere, it's insecure if the server operator uses it improperly.

I'm not sure what your threat model is here? If you are trying to protect users against a malicious server operator, they can just run this PR without telling their users anyway.

@infinity0
Copy link
Author

BTW there is some protection on the client side, the developer of the app that connects to the server needs to specify credentials: 'include' in the options to fetch(). So yes they would have to know to trust the server operator that only cookies that are irrelevant for security are whitelisted, and yes the whitelisted cookies are shared across all proxied domains. I can update the relevant docs to talk about this, if that's acceptable.

@Rob--W
Copy link
Owner

Rob--W commented Oct 7, 2023

BTW there is some protection on the client side, the developer of the app that connects to the server needs to specify credentials: 'include' in the options to fetch(). So yes they would have to know to trust the server operator that only cookies that are irrelevant for security are whitelisted, and yes the whitelisted cookies are shared across all proxied domains.

And it's not just a matter of security. Cookie features such as domain cookies (and flags like SameSite, HttpOnly, etc) cannot be enforced. And when a redirect happens, the cookie can too easily leak to an unintended domain. And the size of cookies is constrained by the browser, so you cannot rely on the cookeis being there when many domains are used.

I can update the relevant docs to talk about this, if that's acceptable.

Feel free to maintain your own fork with this functionality. This is explicitly out of scope for the main project, because no matter the documentation, people are going to use it incorrectly with bad consequences.

@infinity0
Copy link
Author

Cookie features such as domain cookies (and flags like SameSite, HttpOnly, etc) cannot be enforced. [..]

Yes I understand this, my point is that a lot of cookies don't actually make use of these features to work properly, or seem to make use of these features but actually you can break them and things still work, e.g. the example sites I gave in my first post.

Feel free to maintain your own fork with this functionality. [..]

Yes ok fine, I can see your point too, you'd rather be safe than have to deal with other people misusing it.

@infinity0
Copy link
Author

infinity0 commented Oct 8, 2023

@Rob--W I've just had another idea - how about returning those whitelisted cookies in X-Cors-Headers headers instead? Then it's up to the client code how to deal with them. In addition, headers from X-Cors-Headers on the outgoing request can be passed onto the remote as well, so client code can set extra cookies not ordinarily available in the browser.

This way, there is no chance of leaking existing credentials, the credentials flag would be pointless as the client code would be dealing with the remote cookies using custom logic, not the browser's own cookie-handling logic.

This is basically what https://github.com/Zibri/cloudflare-cors-anywhere does, except it chooses to use "Cors-Received-Headers" for the incoming headers, which IMO is wrong as it doesn't start with X-.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants