-
Notifications
You must be signed in to change notification settings - Fork 18k
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/cookiejar: Domain matching against IP address #12610
Comments
The situation is not really simple: RFC 6265 is not clear whether to allow or deny The current implementation in Chromium rejects domain cookies in IP addresses My gut feeling is that cookies with an IP address as the domain-attribute should be RFC 6265 is not really clear here. I read the following
RFC 6265 section 5.1.3, second bullet, third star [7] as a definition of host name in Therefore my understanding of the algorithm to produce a canonicalized host name
RFC 6265 section 5.1.2 step 1 [8] is that an IP address does not qualify as a
and the cookie should be dropped as it is done currently. [1] go/src/net/http/cookiejar/jar.go Lines 447 to 452 in 7f0be1f
[2] https://code.google.com/p/chromium/issues/detail?id=3699 [3] https://codereview.chromium.org/18657/diff/1/net/base/cookie_monster_unittest.cc [4] https://bugzilla.mozilla.org/show_bug.cgi?id=125344 [5] http://src.chromium.org/viewvc/chrome/trunk/src/net/cookies/cookie_store_unittest.h [6] https://hg.mozilla.org/mozilla-central/file/3e8dde8f8c17/netwerk/cookie/nsCookieService.cpp#l3502 [7] http://tools.ietf.org/html/rfc6265#section-5.1.3 |
The Chromium test case you linked as [5] shows that Chromium allows a cookie to be set with a domain attribute if that attribute is an exact match (L470-L472 in the same file).
The third star is so that suffix matching is only applied on host names. 5.1.3 also states that if the two strings matches each other exactly, they domain-match. Regarding RFC6265 5.3:
If I visit the URL http://127.0.0.1/ in my browser, it is my understanding that "the canonicalized request-host" is "127.0.0.1". If the domain-attribute in the Set-Cookie header is set to 127.0.0.1, the canonicalized request-host domain-matches the domain-attribute under the rules of 5.1.3 (exact match). |
The comment in the Chromium test case is interesting: "This matches IE/Firefox, even though it seems a bit wrong." Maybe that behaviour is wrong. One more from the Chromium test case: Chromium treats As I said RFC 6265 is not clear here. I think silently converting the domain cookie to a host cookie is okay, but I do not think that domain cookies on IP addresses make sense at all. Would you have time to test the behaviour on Chrome, FF, IE, Safari and curl? I thinks showing that all these "browsers" handle domain attributes equal to the request IP address the same would add considerable weight to the proposed change. |
For the following web service:
curl 7.40.0 sets the cookie "foo" for 127.0.0.1 I will be able to test IE, Safari in the beginning of next week. |
Safari 9.0 (11601.1.56) sets the cookie "foo" for 127.0.0.1 for the previously mentioned test case. *: Added Expires attribute to make sure a persistent cookie file was created in "Temporary Internet Files". |
This certainly makes it challenging to use |
Having spent some time working out why a Go test behaved differently to a browser test, I ended up at this discussion. Even after following the referenced links, I find the argument for the current behavior to be unconvincing. RFC 6265 section 5.1.3 states that there is a match if "the domain string and the string are identical". The only way to make this exclude IP addresses is to infer a definition of the term "string" to mean "host name (i.e., not an IP address)". But if "string" is defined this way for the entire section, then why would the RFC need these exact words as a requirement of the alternative branch? It would be tautological. Combined with the other evidence that browsers do support exact matches on IP addresses (the Chromium tests, Firefox code, and the last comment by @sebcat), my opinion is that this should be fixed in the Go library. |
Please Golang allow ip address, domain setting cookie in next release .I had to comment out this whole part: go/src/net/http/cookiejar/jar.go Lines 447 to 452 in 7f0be1f
to make my code run. Most of the browsers allow ip address for cookie setting, kindly try to relax that behaviour or atleast mention that somewhere,It took me nearly 1 day to figure out what was going bad. |
You seem to imply that domain-cookies should unconditional be setable on IP addresses if the cookie's domain-attribute is an IP address and equal to the host which also must be an IP address? Do you consider such cookies domain- or host-cookies? You mention "Most of the browsers allow ip address for cookie setting". Could you provide evidence of the details, especially:
|
It turns out that it doesn't matter - for IP addresses, host and domain cookies are the same. RFC 6265 Sec 2.3 says:
This does not appear to exclude IP addresses, since a HTTP request can be sent to an IP address and the IP address would be the only name known by the user agent. This section also refers to https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.2 defines a "canonicalized host name" and refers to domain name RFC's. So although it is not definitive, we could probably say that this refers to a domain name, and specifically not an IP address. This is backed up by section 5.1.3 which says "The string is a host name (i.e., not an IP address)" So in RFC 6265 "request-host" includes IP addresses and "host name" does not. When considering Host cookies (cookies without a Domain attribute):
The phrase "canonicalized request-host" isn't defined, but it is a "request-host" and not just a "host name". The most sensible way to interpret it is as a request-host that has been canonicalized if it is a host name. So we get a cookie with host-only-flag=true and domain=canonicalized request-host (in this case an IP address). When we come to send the cookie Sec 5.4(1) says, since the host-only-flag is true, we send the cookie when the canonicalized request-host is identical to the cookie's domain (when sending to the same IP address). When considering a Domain cookie (cookie with a Domain attribute) returned from an IP address, running through the same sections, since the second part of sec 5.1.3 is inoperable if the request-host is an IP address, the cookie is ignored unless the cookie domain is identical to the request-host, both for the receiving and sending of the cookie. So a cookie from an IP address with the domain set to the IP address is equivalent to an IP address host cookie.
Domain name resolution does not occur for cookie management. This is also true for domain names: https://stackoverflow.com/questions/10020987/cname-and-cookies
The security considerations appear to be the same as those for domain names. e.g. there are potential problems if you use a request-host that reaches a different host on different networks. But the lack of true domain cookies for IP addresses eliminates one class of problems: accidentally giving access to an untrusted subdomain. |
@jongiddy Thanks for the detailed analysis. Please note that the problem are cookies with a domain-attribute only. Host-cookies and IP addresses work well, see e.g. the test in go/src/net/http/cookiejar/jar_test.go Line 259 in 7f0be1f
I think we agree the other two cases (host-cookies) and cookies with a domain name as domain-attribute) work correctly.
Basically yes, but:
Variant A
Variant B
I think you seem to read RFC 6265 as Variant A which is fine, but again: Is this what browsers do? It seems browser and net/http/cookiejar treat Set-Cookies with a domain-attribute which is an IP address differently. I think RFC 6265 is not crystal clear here and I agree that your interpretation is possible (and your arguments are a bit more convincing than mine in #12610 (comment)). But any change here should be okay with RFC 6265, and (more import) compliant with how browsers treat these cookies. I do not think that Variant B would be what RFC 6265 intends, but it could be that browsers do behave that way. And in this case we probably should do B and not A. I do not think that interpretations of RFC 6265's text are helpful in making any progress here. RFC 6258 is not clear here. This is unfortunate. Interpreting it both ways is possible. Progress here will be made once we understand how browsers handle these cookies. Does anybody have insights here? Do we have to make a test here? |
I don't see any ambiguity in RFC 6265. 5.3, step 6 clearly rejects a cookie only if the canonicalized The A request to Possibly contemporary browsers do something different, but the behavior as detailed by RFC 6265 clearly indicates that a |
@neild And what conclusion do you draw? Should we just change
Do it and call it a day? Rating the ambiguity of RFC 6265 on a scale from "crystal clear" to "inconsistent" is a nice intellectual exercise but does it (alone) help in deciding what to do with net/http/cookiejar.Jar? I understand your last
that you argue to change Jar, no matter what contemporary browsers do? Please do not get me wrong: I'm basically convinced that Jar should be changed in regard to "IP-Domain-Cookies", but I'd like to see more evidence that it will do good to existing (and future) users of net/http/cookiejar.Jar than just "RFC 6265 is clear here". Jar is "broken" since several years and complaints about that "bug" are rare. I just want to see any reason why "fixing" Jar won't introduce other problems for existing users. I'll retry sebcats experiments on 2021 browses. If their behaviour matches your reading of RFC 6265 I'll prepare a CL. |
Testcode
Systems checkedMac
Windows
OutcomeAll show
on the second request. Chrome developer tools and curl's jar file hint that the |
In the absence of evidence that browsers behave differently, By my read, RFC 6265 states that these cookies should have The internal |
Why? If we accept a cookie Domain matching in RFC 6265 section 5.1.3 is the OR of two bullet points. Or do you have any example which would trigger some misbehaviour in (*entry).domainMatch if we do not include the check? Note that I intend to reject a cookie |
Change https://golang.org/cl/326689 mentions this issue: |
Sorry; I got the flag backwards. I meant to say the opposite: If we accept cookies with I agree that setting |
If changes are made here, it might be a good idea to consider IPv6 addresses. I.e., URIs of the form |
Hello,
The following test fails:
Further inspection shows that it fails with errNoHostname here:
go/src/net/http/cookiejar/jar.go
Lines 447 to 452 in 7f0be1f
I believe the comment refers to RFC6265 5.1.3. It states that the domain string must either be identical with the string it's matched against, OR that all of the three subsequently listed conditions are met, one of which are that the string being matched must be a host name.
In the case of the matched string being an IP address and the domain string being the same IP address, the first condition is met. I therefor believe that L447-L452 should be removed.
The text was updated successfully, but these errors were encountered: