net/http/cookiejar: allow cookies with an IP address in the domain attribute

A set domain attribute in a cookie in a Set-Cookie header is intended to
create a domain cookie, i.e. a cookie that is not only sent back to the
domain the Set-Cookie was received from, but to all subdomains thereof
too. Sometimes people set this domain attribute to an IP address. This
seems to be allowed by RFC 6265 albeit it's not really sensible as there
are no "subdomains" of an IP address.
Contemporary browsers allow such cookies, currently Jar forbids them.

This CL allows to persist such cookies in the Jar and send them back
again in subsequent requests. Jar allows those cookies that all
contemporary browsers allow (not all browsers behave the same and none
seems to conform to RFC 6265 in regards to these cookies, see below).

The following browsers in current version) were tested:
  - Chrome (Mac and Windows)
  - Firefox (Mac and Windows)
  - Safari (Mac)
  - Opera (Mac)
  - Edge (Windows)
  - Internet Explorer (Windows)
  - curl (Mac, Linux)

All of them allow a cookie to be set via the following HTTP header if
the request was made to e.g. http://35.206.97.83/ :

    Set-Cookie: a=1; domain=35.206.97.83

They differ in handling a leading dot "." before the IP address as in

    Set-Cookie: a=1; domain=.35.206.97.83

sets a=1 only in curl and in Internet Explorer, the other browsers just
reject such cookies.

As far as these internals can be observed the browsers do not treat such
cookies as domain cookies but as host cookies. RFC 6265 would require to
treat them as domain cookies; this is a) nonsensical and b) doesn't make
an observable difference. As we do not expose Jar entries and their
HostOnly flag it probably is still okay to claim that Jar implements a
RFC 6265 cookie jar.

RFC 6265 would allow cookies with dot-prefixed domains like
domain=.35.206.97.83 but it seems as if this feature of RFC 6265 is not
used in real life and not requested by users of package cookiejar (probably
because it doesn't work in browsers) so we refrain from documenting this
detail.

Fixes #12610

Change-Id: Ibd883d85bde6b958b732cbc3618a1238ac4fc84a
Reviewed-on: https://go-review.googlesource.com/c/go/+/326689
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/cookiejar/jar.go b/src/net/http/cookiejar/jar.go
index c2393a0741f26aa2443e6c4fc4ac9df33c66c5fa..87c38ceba905e02782f4fa80ac38dd8fc58427ab 100644 (file)
--- a/src/net/http/cookiejar/jar.go
+++ b/src/net/http/cookiejar/jar.go
@@ -121,7 +121,9 @@ func (e *entry) shouldSend(https bool, host, path string) bool {
        return e.domainMatch(host) && e.pathMatch(path) && (https || !e.Secure)
 }
 
-// domainMatch implements "domain-match" of RFC 6265 section 5.1.3.
+// domainMatch checks whether e's Domain allows sending e back to host.
+// It differs from "domain-match" of RFC 6265 section 5.1.3 because we treat
+// a cookie with an IP address in the Domain always as a host cookie.
 func (e *entry) domainMatch(host string) bool {
        if e.Domain == host {
                return true
@@ -455,10 +457,36 @@ func (j *Jar) domainAndType(host, domain string) (string, bool, error) {
        }
 
        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
+               // RFC 6265 is not super clear here, a sensible interpretation
+               // is that cookies with an IP address in the domain-attribute
+               // are allowed.
+
+               // RFC 6265 section 5.2.3 mandates to strip an optional leading
+               // dot in the domain-attribute before processing the cookie.
+               //
+               // Most browsers don't do that for IP addresses, only curl
+               // version 7.54) and and IE (version 11) do not reject a
+               //     Set-Cookie: a=1; domain=.127.0.0.1
+               // This leading dot is optional and serves only as hint for
+               // humans to indicate that a cookie with "domain=.bbc.co.uk"
+               // would be sent to every subdomain of bbc.co.uk.
+               // It just doesn't make sense on IP addresses.
+               // The other processing and validation steps in RFC 6265 just
+               // collaps to:
+               if host != domain {
+                       return "", false, errIllegalDomain
+               }
+
+               // According to RFC 6265 such cookies should be treated as
+               // domain cookies.
+               // As there are no subdomains of an IP address the treatment
+               // according to RFC 6265 would be exactly the same as that of
+               // a host-only cookie. Contemporary browsers (and curl) do
+               // allows such cookies but treat them as host-only cookies.
+               // So do we as it just doesn't make sense to label them as
+               // domain cookies when there is no domain; the whole notion of
+               // domain cookies requires a domain name to be well defined.
+               return host, true, nil
        }
 
        // From here on: If the cookie is valid, it is a domain cookie (with

diff --git a/src/net/http/cookiejar/jar_test.go b/src/net/http/cookiejar/jar_test.go
index b7267b17184cc65d7a9af6cc8bbda2f5bd2ec58f..13d994aa39347842fc1a197471e1b3d385dba420 100644 (file)
--- a/src/net/http/cookiejar/jar_test.go
+++ b/src/net/http/cookiejar/jar_test.go
@@ -306,8 +306,8 @@ var domainAndTypeTests = [...]struct {
        {"foo.sso.example.com", "sso.example.com", "sso.example.com", false, nil},
        {"bar.co.uk", "bar.co.uk", "bar.co.uk", false, nil},
        {"foo.bar.co.uk", ".bar.co.uk", "bar.co.uk", false, nil},
-       {"127.0.0.1", "127.0.0.1", "", false, errNoHostname},
-       {"2001:4860:0:2001::68", "2001:4860:0:2001::68", "2001:4860:0:2001::68", false, errNoHostname},
+       {"127.0.0.1", "127.0.0.1", "127.0.0.1", true, nil},
+       {"2001:4860:0:2001::68", "2001:4860:0:2001::68", "2001:4860:0:2001::68", true, nil},
        {"www.example.com", ".", "", false, errMalformedDomain},
        {"www.example.com", "..", "", false, errMalformedDomain},
        {"www.example.com", "other.com", "", false, errIllegalDomain},
@@ -328,7 +328,7 @@ func TestDomainAndType(t *testing.T) {
        for _, tc := range domainAndTypeTests {
                domain, hostOnly, err := jar.domainAndType(tc.host, tc.domain)
                if err != tc.wantErr {
-                       t.Errorf("%q/%q: got %q error, want %q",
+                       t.Errorf("%q/%q: got %q error, want %v",
                                tc.host, tc.domain, err, tc.wantErr)
                        continue
                }
@@ -593,6 +593,21 @@ var basicsTests = [...]jarTest{
                "a=1",
                []query{{"http://192.168.0.10", "a=1"}},
        },
+       {
+               "Domain cookies on IP.",
+               "http://192.168.0.10",
+               []string{
+                       "a=1; domain=192.168.0.10",  // allowed
+                       "b=2; domain=172.31.9.9",    // rejected, can't set cookie for other IP
+                       "c=3; domain=.192.168.0.10", // rejected like in most browsers
+               },
+               "a=1",
+               []query{
+                       {"http://192.168.0.10", "a=1"},
+                       {"http://172.31.9.9", ""},
+                       {"http://www.fancy.192.168.0.10", ""},
+               },
+       },
        {
                "Port is ignored #1.",
                "http://www.host.test/",
@@ -927,10 +942,17 @@ var chromiumBasicsTests = [...]jarTest{
        {
                "TestIpAddress #3.",
                "http://1.2.3.4/foo",
-               []string{"a=1; domain=1.2.3.4"},
+               []string{"a=1; domain=1.2.3.3"},
                "",
                []query{{"http://1.2.3.4/foo", ""}},
        },
+       {
+               "TestIpAddress #4.",
+               "http://1.2.3.4/foo",
+               []string{"a=1; domain=1.2.3.4"},
+               "a=1",
+               []query{{"http://1.2.3.4/foo", "a=1"}},
+       },
        {
                "TestNonDottedAndTLD #2.",
                "http://com./index.html",