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

net/http/cookiejar: Domain matching against IP address #12610

Closed
sebcat opened this issue Sep 14, 2015 · 19 comments
Closed

net/http/cookiejar: Domain matching against IP address #12610

sebcat opened this issue Sep 14, 2015 · 19 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@sebcat
Copy link

sebcat commented Sep 14, 2015

Hello,

The following test fails:

package foo 

import (
    "net/http"
    "net/http/cookiejar"
    "net/url"
    "testing"
)

func TestIPDomain(t *testing.T) {
    u, err := url.Parse("http://127.0.0.1/")
    if err != nil {
        t.Fatal(err)
    }   

    jar, err := cookiejar.New(nil)
    if err != nil {
        t.Fatal(err)
    }   

    c := &http.Cookie{Name: "foo", Value: "bar", Domain: "127.0.0.1"}
    jar.SetCookies(u, []*http.Cookie{c})
    cs := jar.Cookies(u)
    if len(cs) != 1 { 
        t.Fatalf("Got %v cookies, expected 1\n", len(cs))
    } else if cs[0].Name != "foo" || cs[0].Value != "bar" {
        t.Fatal("Invalid cookie name/value")
    }   
}

Further inspection shows that it fails with errNoHostname here:

if isIP(host) {
// According to RFC 6265 domain-matching includes not being
// an IP address.
// TODO: This might be relaxed as in common browsers.
return "", false, errNoHostname
}

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.

@vdobler
Copy link
Contributor

vdobler commented Sep 16, 2015

The situation is not really simple: RFC 6265 is not clear whether to allow or deny
domain cookies on IP addresses and browsers handle this case differently.
That was the reason for the TODO in line 450 [1] of the cookiejar implementation.
I have the impression that half a decade ago domain cookies on IP addresses
where commonly accepted, sometimes just to match the behavior of Internet Explorer
(see e.g. [2] [3] [4] [10]).

The current implementation in Chromium rejects domain cookies in IP addresses
[5] lines 457-477. If I my understanding of the Mozilla code [6] is correct then
Firefox silently converts a domain cookie for an IP address to a host cookie.
(I have not done tests on any browsers recently.)

My gut feeling is that cookies with an IP address as the domain-attribute should be
either rejected (an IP address is not a domain) or be converted to host cookie (i.e.
pretending the domain-attribute was not set, a forgivable user error).

RFC 6265 is not really clear here. I read the following

The string is a host name (i.e., not an IP address).

RFC 6265 section 5.1.3, second bullet, third star [7] as a definition of host name in
the sense of: "a host name is not an IP address".

Therefore my understanding of the algorithm to produce a canonicalized host name

Convert the host name to a sequence of individual domain name labels.

RFC 6265 section 5.1.2 step 1 [8] is that an IP address does not qualify as a
canonicalized host name.
As an IP address is not a canonicalized host name I think that RFC 6265 section 5.3
step 6 first case [9] applies:

If the canonicalized request-host does not domain-match the domain-attribute:
Ignore the cookie entirely and abort these steps.

and the cookie should be dropped as it is done currently.

[1]

if isIP(host) {
// According to RFC 6265 domain-matching includes not being
// an IP address.
// TODO: This might be relaxed as in common browsers.
return "", false, errNoHostname
}

[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
[8] http://tools.ietf.org/html/rfc6265#section-5.1.2
[9] http://tools.ietf.org/html/rfc6265#section-5.3
[10] https://bugzilla.mozilla.org/show_bug.cgi?id=125344

@sebcat
Copy link
Author

sebcat commented Sep 16, 2015

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).

RFC 6265 is not really clear here. I read the following

The string is a host name (i.e., not an IP address).

RFC 6265 section 5.1.3, second bullet, third star [7] as a definition of host name in
the sense of: "a host name is not an IP address".

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 the canonicalized request-host does not domain-match the domain-attribute:
Ignore the cookie entirely and abort these steps.

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).

@vdobler
Copy link
Contributor

vdobler commented Sep 16, 2015

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 domain=.1.2.3.4 as an illegal domain attribute. But if you follow RFC 6265 (as package cookiejar tries to) you would chop off the leading trail first (see 5.3.2), even before domain matching it. Chromiums behaviour here is not RFC 6265 compliant no matter if RFC 6265 allows domain cookies on IP addresses or not.

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.

@sebcat
Copy link
Author

sebcat commented Sep 16, 2015

For the following web service:

package main

import (
    "net/http"
    "log"
)

func CookieHandler(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Set-Cookie", "foo=bar; domain=127.0.0.1")
    w.Write([]byte("foo"))
}

func main() {
    http.HandleFunc("/", CookieHandler)
    log.Fatal(http.ListenAndServe(":8080", nil))
}

curl 7.40.0
Google Chrome 47.0.2503.0 dev
Mozilla Firefox 40.0.3

sets the cookie "foo" for 127.0.0.1

I will be able to test IE, Safari in the beginning of next week.

@sebcat
Copy link
Author

sebcat commented Sep 21, 2015

Safari 9.0 (11601.1.56)
IE 10.0.9200.17492 (Update version 10.0.31, Windows 2012 Server) *

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".

@dcormier
Copy link
Contributor

dcormier commented Mar 21, 2019

This certainly makes it challenging to use httptest.Server and try to set cookies with the Domain attribute set to the httptest.Server's address.

@jongiddy
Copy link

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.

@jayateertha043
Copy link

Please Golang allow ip address, domain setting cookie in next release .I had to comment out this whole part:

if isIP(host) {
// According to RFC 6265 domain-matching includes not being
// an IP address.
// TODO: This might be relaxed as in common browsers.
return "", false, errNoHostname
}

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.

https://stackoverflow.com/questions/67872836/use-cookies-txt-as-cookies-jar-client-side-in-golang?noredirect=1#comment119971669_67872836

@vdobler
Copy link
Contributor

vdobler commented Jun 7, 2021

@jayateertha043

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:

  • What if only one of the cookie-domain-attribute and the host is an IP address but the IP address ist that of the domain name on the other? What do browsers do in such cases? Do all browsers behave the same way?
  • Do browsers treat such cookies a domain-cookies or host-cookies? Does this matter? Do all browsers behave the same?
  • On which requests do browsers return cookies set with an IP address as the domain-attribute? Only tho the IP address? Or also to the domain name resolving tho this IP?
  • Are there any security considerations to take care of? If not: Why is the behaviour unproblematic?

@jongiddy
Copy link

jongiddy commented Jun 7, 2021

Do you consider such cookies domain- or host-cookies?

  • Do browsers treat such cookies a domain-cookies or host-cookies? Does this matter? Do all browsers behave the same?

It turns out that it doesn't matter - for IP addresses, host and domain cookies are the same.

RFC 6265 Sec 2.3 says:

The request-host is the name of the host, as known by the user agent,
to which the user agent is sending an HTTP request or from which it
is receiving an HTTP response (i.e., the name of the host to which it
sent the corresponding HTTP request)."

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 request-uri from https://datatracker.ietf.org/doc/html/rfc2616#section-5.1.2 which, via https://datatracker.ietf.org/doc/html/rfc2396, ultimately defines its host component as hostname | IPv4address.

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):

  • 5.3(4) sets the domain-attribute to an empty string
  • 5.3(6) sets the host-only-flag to true and the domain to the canonicalized request-host.

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.

  • What if only one of the cookie-domain-attribute and the host is an IP address but the IP address ist that of the domain name on the other? What do browsers do in such cases? Do all browsers behave the same way?
  • On which requests do browsers return cookies set with an IP address as the domain-attribute? Only tho the IP address? Or also to the domain name resolving tho this IP?

Domain name resolution does not occur for cookie management. This is also true for domain names: https://stackoverflow.com/questions/10020987/cname-and-cookies

  • Are there any security considerations to take care of? If not: Why is the behaviour unproblematic?

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.

@vdobler
Copy link
Contributor

vdobler commented Jun 8, 2021

@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

{"127.0.0.1", "", "127.0.0.1", true, nil},
. RFC 6265 is pretty clear here and this was never in question, so let's focus the discussion on cookies with a domain-attribute which is a IP address only.

I think we agree the other two cases (host-cookies) and cookies with a domain name as domain-attribute) work correctly.

It turns out that it doesn't matter - for IP addresses, host and domain cookies are the same

Basically yes, but:

  1. Are we sure all browsers actually do treat them as "the same" (i.e. not only same by reading RFC 6265 which is not absolutely clear.) While adhering to RFC 6265 is important the main issue here is that browsers do something differently with this type of cookies than net/http/cookiejar.

  2. From when on are these "the same":

Variant A

If on a Set-Cookie header the domain-attribute is a IP address:
    If domain-attribute != request-host:
        ignore the cookie
    Else:
        treat the domain attribute as absent and process it to the current rules for host-cookies.

Variant B

If on a Set-Cookie header the domain-attribute is a IP address:
        treat the domain attribute as absent and
        process it to the current rules for host-cookies
        that is just set it unconditional in the request-host (be it a domain or an IP)

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?

@neild
Copy link
Contributor

neild commented Jun 8, 2021

I don't see any ambiguity in RFC 6265.

5.3, step 6 clearly rejects a cookie only if the canonicalized request-host does not domain-match a non-empty domain-attribute. 5.1.3 clearly states that a string domain-matches a domain string if the two are identical.

The request-host is the name of the host to which the user agent sent the HTTP request (2.3). Canonicalization applies to DNS names, not IP addresses, so it is not explicitly stated that the request-host for a request to http://127.0.0.1/ is 127.0.0.1, however it is clear that this is the intent. (If this is not the case, then 5.6, step 6 makes no sense in the case where the domain-attribute is empty: "Set the cookie's domain to the canonicalized request-host.")

A request to http://127.0.0.1/ resulting in a cookie with a Domain attribute of 127.0.0.1 will therefore set the request-host to 127.0.0.1 (2.3), the domain-attribute to 127.0.0.1 (5.3, step 4). These strings are identical and therefore domain-match (5.1.3), so the cookie's host-only-flag is set to false and the domain is set to 127.0.0.1 (5.3, step 6).

Possibly contemporary browsers do something different, but the behavior as detailed by RFC 6265 clearly indicates that a Domain attribute containing an IP address will match on a request URL containing that exact IP address. (Not, of course, a request URL containing a DNS name that resolves to that address.)

@vdobler
Copy link
Contributor

vdobler commented Jun 9, 2021

@neild And what conclusion do you draw?

Should we just change Jar and allow "IP-Domain-Cookies"?

  • This particular issue here would be closed.
  • Browsers from 2015 seem to behave that way as sebcat's experiments showed.
  • jongiddy doesn't see any security implications.

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
statement

Possibly contemporary browsers do something different, but the behavior as detailed by RFC 6265 clearly indicates that a Domain attribute containing an IP address will match on a request URL containing that exact IP address.

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.

@vdobler
Copy link
Contributor

vdobler commented Jun 9, 2021

Testcode

package main

import (
	"fmt"
	"log"
	"net/http"
	"os"
)

var ipaddr = "127.0.0.1"

func CookieHandler(w http.ResponseWriter, r *http.Request) {
	w.Header().Add("Set-Cookie", "hostcookie=123")
	w.Header().Add("Set-Cookie", "legal=456; domain="+ipaddr)
	w.Header().Add("Set-Cookie", "illegal=xxx; domain=1.2.3.4")
	w.WriteHeader(200)
	for _, c := range r.Cookies() {
		fmt.Fprintln(w, c.Name, " --> ", c.Value)
	}
}

func main() {
	if ip := os.Getenv("IP_ADDR"); ip != "" {
		ipaddr = ip
	}
	http.HandleFunc("/", CookieHandler)
	log.Fatal(http.ListenAndServe(":80", nil))
}

Systems checked

Mac

  • Chrome Version 90.0.4430.212
  • Safari Version 14.1 (14611.1.21.161.5)
  • Firefox Version 89.0
  • Opera Version:76.0.4017.177
  • curl 7.54.0

Windows

  • Edge Version 90.0.818.62
  • Chrome Version 91.0.4472.77
  • Internt Explorer 11.3630.14393.0

Outcome

All show

hostcookie  -->  123
legal  -->  456

on the second request.

Chrome developer tools and curl's jar file hint that the legal cookie is treated as / has been converted to a host-cookie (like hostcookie).

@neild
Copy link
Contributor

neild commented Jun 9, 2021

And what conclusion do you draw?

In the absence of evidence that browsers behave differently, net/http/cookiejar should accept cookies with a IP-address Domain equal to the request-host.

By my read, RFC 6265 states that these cookies should have host-only-flag set to false, but it looks like your empirical tests indicate that browsers set it to true. Following the browser behavior seems okay, although I don't see how following the RFC behavior would result in in different behavior visible to clients.

The internal (*entry).domainMatch function claims to implement RFC 6265 domain-match, but it doesn't include the check to verify that the host is not an IP address. If we do accept cookies with HostOnly=true and an IP address in Domain, that function needs updating.

@vdobler
Copy link
Contributor

vdobler commented Jun 10, 2021

@neild

The internal (*entry).domainMatch function claims to implement RFC 6265 domain-match, but it doesn't include the check to verify that the host is not an IP address. If we do accept cookies with HostOnly=true and an IP address in Domain, that function needs updating.

Why?

If we accept a cookie Set-Cookie: a=1; domain=127.0.0.1 from 127.0.0.1 and process this as if it was a host cookie Set-Cookie: a=1 then e.HostOnly==true, e.Domain=="127.0.0.1" and domainMatch is basically e.Domain == host which is correct.

Domain matching in RFC 6265 section 5.1.3 is the OR of two bullet points.
The second bullet contains the condition "The string is a host name (i.e., not an IP address)" but the whole second bullet is there only for actual domain cookies which should be sent back to subdomains.
RFC 6265 is overly complicated here (again) in treating domain-cookies and host-cookies the same.

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 Set-Cookie: a=1; domain=.127.0.0.1 from 127.0.0.1 if the domain-attribute has leading dot as browsers reject them (albeit curl and IE11 don't).

@gopherbot
Copy link

Change https://golang.org/cl/326689 mentions this issue: net/http/cookiejar: allow cookies with an IP address in the domain attribute

@neild
Copy link
Contributor

neild commented Jun 10, 2021

Why?

Sorry; I got the flag backwards. I meant to say the opposite: If we accept cookies with HostOnly=false and an IP address in Domain, then domainMatch would need updating to implement the full domain-match algorithm and verify that the request-host string is not an IP address.

I agree that setting HostOnly=true on these cookies is equivalent from a user perspective and requires the fewest code changes.

@robx
Copy link

robx commented Jan 27, 2022

If changes are made here, it might be a good idea to consider IPv6 addresses. I.e., URIs of the form http://[::1]/ (see RFC 3986) and whether such host names would translate into domain=::1 or domain=[::1].

@dmitshur dmitshur modified the milestones: Unplanned, Go1.19 May 18, 2022
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label May 18, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests