-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
net/http: support partitioned cookies #62490
Comments
SameSite=None
cookies and add partitioned cookies
Sure, updated. |
Change https://go.dev/cl/526435 mentions this issue: |
We don't want a deprecation notice on Also, partitioned cookies use Adding a
|
@neild I'm agree with you. my pr just adds a note for the SameSiteNoneMode. |
Any updates on this proposal? It would be useful for something I'm trying to work on. |
What more needs to be done to have this proposal accepted? The associated commit explains the changes clearly. |
Supporting the partitioned cookie attribute without actually implementing the cookie jar changes implementing the semantics of the partitioning this attribute signals sounds at least incomplete or even dangerous from a security perspective to me. So a more complete proposal should outline the full semantics and amount of changes to implement the support. Not only the encoding and decoding, but also creating the key to address the cookie jar elements and storing as well as accessing them in http.Client.Jar to reap all the benefits of such cookies. |
I can't understand why it's related to http.Client.Jar. the cookies could be parsed by |
This proposal implements server-side support. Client-side support is intended for browsers to block cross-site tracking. It's unclear if this use case would be meaningful for a Go HTTP client. If someone made an end user web browser in Go there is a lot they would have to add on to the existing http client such as this and CORS. |
I think it's not feasible to add it to http.Client.Jar.
there should no scenes like above in the go http client. |
Is there a way to prioritise this proposal since it is time critical? We are already receiving reports on broken websites for our router. In our scenario we read a cookie from the response provided by the backend to set an additional cookie for sticky sessions (to route further requests to the same backend). If we now rely on the |
@maxmoehl it seems like that is already the situation someone is in if they want to support older Go versions. A function could be made that would look for it in Unparsed and then in the future would use the attribute. It's not easy to write code to check for an attribute though- that requires reflection rather than just an interface check. So it would help with compatibility to provide getter and setter functions for new attributes like this. The getter and setter could be marked as deprecated in a future release. |
@ianlancetaylor @neild - Looks like the PR for this is on hold until this proposal is accepted. However it's unclear to me what if anything is required to get this proposal accepted. Can you elaborate the next steps needed to move this along? Thanks! |
This is becoming more and more urgent as 3P cookies are being phased out by all major vendors including Google, and CHIPS is a cross-platform solution solving this issue: |
Is the proposal simply to add a field as in #62490 (comment)? |
@rsc I'm not the original author but from what I can tell, yes. It also includes changes to relevant functions (String(), Valid(), etc.) https://go.dev/cl/526435 I would use this in an LMS plugin my team manages. Our tool is displayed in an iframe in the LMS and so needs to use Partitioned to set cookies. |
This proposal has been added to the active column of the proposals project |
Thank you @rsc! It is indeed only a boolean flag on a cookie which omits / includes this tag in the Set-Cookie directive |
Have all remaining concerns about this proposal been addressed? The proposal is to add a new In the Cookie parser, if the cookie says “; Partitioned”, the bool is set to true. In Cookie.String, if Partitioned is true, the string adds “; Partitioned”. In Cookie.Valid, if Partitioned is true and the cookie is not secure or the path is not /, Valid returns an error. |
Based on the discussion above, this proposal seems like a likely accept. The proposal is to add a new In the Cookie parser, if the cookie says “; Partitioned”, the bool is set to true. In Cookie.String, if Partitioned is true, the string adds “; Partitioned”. In Cookie.Valid, if Partitioned is true and the cookie is not secure or the path is not /, Valid returns an error. |
We did not end up deciding what needs to happen in net/http/cookiejar or http.CookieJar interface. At the least, net/http/cookiejar needs to be updated to preserve the Partitioned bool when given cookies and returning them, right? It is unclear to me whether the http.CookieJar interface also needs semantic changes. Does it? |
Another option, perhaps the right one, would be to add Partition to cookie for use on server side, and then defer what to do on client side for another day. Are we okay with only addressing the server side? What are the consequences of adding Partition on the server side but not the client side? |
Currently, We could also add support to We should add I think we should also add CHIPS support to |
Can we add CHIPS support to net/http/cookiejar without changing the |
Yes, defer the client cookie jar. This proposal implements server-side support. Client-side support is intended for browsers to block cross-site tracking, which is not a normal use case for a Go client. Let's stay focused on just delivering the server side for this proposal since we know how to do that without breaking anything and there are a lot of users of this server side. A new issue can be opened for the client CookieJar. |
I can't think of any that would have an impact. The client would not send the cookie from the embedded page if the parent page isn't the same as it was when the cookie was set (see: CHIPS). But the entire concept of embedded pages is missing from the go http client. The only thing a client could do is preserve the attribute when sending back the cookie, but that shouldn't have any real impact as the attribute is intended for the client to act upon, not the server. |
I believe we can add CHIPS support without changing the interface. The partition is "the site (scheme and registrable domain) of the top-level URL the browser was visiting at the start of the request to the endpoint that set the cookie" (https://developers.google.com/privacy-sandbox/3pcd/chips#cookie_partitioning_technical_design), and
Perhaps I'm missing something, though. |
It sounds like the "likely accept" version above is still okay. Thanks for the clarifications. |
No change in consensus, so accepted. 🎉 The proposal is to add a new In the Cookie parser, if the cookie says “; Partitioned”, the bool is set to true. In Cookie.String, if Partitioned is true, the string adds “; Partitioned”. In Cookie.Valid, if Partitioned is true and the cookie is not secure or the path is not /, Valid returns an error. |
Hi, there. Path=/ requirement has been removed, you can refer to privacycg/CHIPS#49 for the details. So the PR is updated as well. |
@dmitshur Can we add this issue to the Go 1.22.4 release ? |
See https://go.dev/wiki/MinorReleases. We try to backport as little as possible during minor releases, only critical problems, security fixes and important documentation fixes. This is a new API that'll be available in Go 1.23. |
The partitioned field is only available in go 1.23+ and so should be removed from the library as this version does not yet require v1.23+. See golang/go#62490 (comment) Fixes #277
The proposal is to add a new Partitioned bool field to the Cookie struct, set and used as follows:
In the Cookie parser, if the cookie says “; Partitioned”, the bool is set to true.
In Cookie.String, if Partitioned is true, the string adds “; Partitioned”.
In Cookie.Valid, if Partitioned is true and the cookie is not secure, Valid returns an error.
ref:
https://developer.chrome.com/docs/privacy-sandbox/third-party-cookie-phase-out/
The text was updated successfully, but these errors were encountered: