From 4ba114ec05c6090708b51db5f7a196e95bce64e0 Mon Sep 17 00:00:00 2001 From: Volker dobler Date: Thu, 10 Jun 2021 11:54:58 +0200 Subject: [PATCH] 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 TryBot-Result: Gopher Robot Reviewed-by: Michael Knyszek Reviewed-by: Damien Neil --- src/net/http/cookiejar/jar.go | 38 ++++++++++++++++++++++++++---- src/net/http/cookiejar/jar_test.go | 30 +++++++++++++++++++---- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/net/http/cookiejar/jar.go b/src/net/http/cookiejar/jar.go index c2393a0741..87c38ceba9 100644 --- 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 b7267b1718..13d994aa39 100644 --- 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",