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

Sticky cookie should be Secure and SameSite=None by default #6110

Closed
ottenhoff opened this issue Feb 16, 2024 · 4 comments
Closed

Sticky cookie should be Secure and SameSite=None by default #6110

ottenhoff opened this issue Feb 16, 2024 · 4 comments
Labels
bug 🐞 Something isn't working discussion 💬 The right solution needs to be found

Comments

@ottenhoff
Copy link
Contributor

ottenhoff commented Feb 16, 2024

A common legacy-app routing setup looks like this:

reverse_proxy tomcat_serverA:8080 tomcat_serverB:8080 {
    lb_policy cookie
}

The result might be:

$ curl -I https://demo.example/portal/
HTTP/2 200
cache-control: no-store, no-cache, must-revalidate, max-age=0, post-check=0, pre-check=0
content-type: text/html;charset=UTF-8
date: Fri, 16 Feb 2024 21:54:13 GMT
pragma: no-cache
server: Caddy
set-cookie: lb=b4b924ab173004e449881468ab25c0aa4197efd974ae8c007a7164869c261a69; Path=/

The problem for a Chrome user is that if they arrive from a third-party link, the lb cookie will not be sent to Caddy as the cookie was not set with SameSite=None property.

While setting SameSite=None is a fun debate for applications and their CSRF protections, Caddy doesn't have a potential CSRF vulnerability so SameSite=None as the default makes sense. We want the "sticky" user to always end up on the same upstream if they come from a third-party website, from a bookmark, or from a same-site link.

@francislavoie
Copy link
Member

You're probably right.

I'm not sure Secure should be set by default if the site might be served over HTTP. Maybe we could only include it if we know the connection is TLS (or X-Forwarded-Proto: https if the request is trusted).

For SameSite, would this be a breaking change for anyone if we change the default? We could make it an opt-in config if there's any risk of breakage.

@francislavoie francislavoie added bug 🐞 Something isn't working discussion 💬 The right solution needs to be found labels Feb 16, 2024
@ottenhoff
Copy link
Contributor Author

This would not be a breaking change for existing users. Here are the scenarios for an HTTPS site:

  1. Fresh user: would receive new cookie with Secure and SameSite=None
  2. Existing user with old cookie coming from same-site context: old cookie would continue to be presented to Caddy
  3. Existing user with old cookie coming from third-party context: old cookie would not be sent by browser so new cookie would be sent by Caddy with Secure and SameSite=None

I will provide a PR

@ottenhoff
Copy link
Contributor Author

Fixed in #6115

@mholt
Copy link
Member

mholt commented Feb 28, 2024

Thank you for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working discussion 💬 The right solution needs to be found
Projects
None yet
Development

No branches or pull requests

3 participants