net/http: Client should scope cookie to Request.Host before Request.URL #38988

colinclerk · 2020-05-10T20:40:09Z

What version of Go are you using (`go version`)?

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build866802505=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Created a httptest.Server with a handler that sets a cookie, then issued a request from a http.Client with a custom Host field.

https://play.golang.org/p/78mh9ZwYI_t

What did you expect to see?

I expected to see the cookie scoped to the Host field.

What did you see instead?

The cookie was scoped to httptest.Server.URL

I was seeing unexpected cookie behavior while testing and I believe this is the root of it:

go/src/net/http/client.go

Lines 170 to 186 in 57e32c4

    
           func (c *Client) send(req *Request, deadline time.Time) (resp *Response, didTimeout func() bool, err error) { 
        
           	if c.Jar != nil { 
        
           		for _, cookie := range c.Jar.Cookies(req.URL) { 
        
           			req.AddCookie(cookie) 
        
           		} 
        
           	} 
        
           	resp, didTimeout, err = send(req, c.transport(), deadline) 
        
           	if err != nil { 
        
           		return nil, didTimeout, err 
        
           	} 
        
           	if c.Jar != nil { 
        
           		if rc := resp.Cookies(); len(rc) > 0 { 
        
           			c.Jar.SetCookies(req.URL, rc) 
        
           		} 
        
           	} 
        
           	return resp, nil, nil 
        
           }

Instead of the client always using req.URL for the cookie jar, I believe it should first look at req.Host, then req.Header.Get("Host"), and then finally req.URL.

I think this would be consistent with the behavior of Request.Write described on the Request struct:

go/src/net/http/request.go

Lines 231 to 235 in 57e32c4

    
           // For client requests, Host optionally overrides the Host 
        
           // header to send. If empty, the Request.Write method uses 
        
           // the value of URL.Host. Host may contain an international 
        
           // domain name. 
        
           Host string

The text was updated successfully, but these errors were encountered:

toothrot · 2020-05-11T15:22:08Z

/cc @vdobler @bradfitz

vdobler · 2020-05-11T20:19:37Z

@colinclerk Thanks for this bug report. There might be a real issue here.

From https://tools.ietf.org/html/rfc6265#section-2.3

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

The term request-uri is defined in Section 5.1.2 of [RFC2616].

According to https://tools.ietf.org/html/rfc6265#section-5.4 domain matching works on the request-host (with the unclear definition above).
The request-uri consists of the Host header and the abs_path, and the request-host sounds more like the r.URL.Host.

On the other hand: curl seems to use the Host header....

colinclerk · 2020-05-12T00:43:16Z

@vdobler What is the intended purpose of allowing outbound requests where r.Host != r.URL.Host?

When I issue outbound requests where r.Host != r.URL.Host, my intent is to mimic DNS resolution:

r.Host is what my user has in their address bar
r.URL.Host is where the DNS for r.Host resolves

Having this separation allows me to mimic a server that has multiple hosts pointed at it, and generate different responses depending on the Host.

Since I'm imagining r.Host is what my user has in their address bar, it's also where I'm expecting cookies to be set. But maybe I'm thinking about this all wrong?

Another exercise that might be helpful is to think about things from the server's perspective. r.URL.Host doesn't exist on the server, so when it issues SetCookie it expects the cookie to be set on the incoming request's r.Host.

That's not what happens when using Client and setting r.Host manually:
https://play.golang.org/p/NJIuiMhPP55

chrisguiney · 2023-02-20T03:05:33Z

I just ran into this issue. It seems pretty clear to me that a new *url.URL using r.Host should be used to set/get cookies.

Say my application does routing based on subdomain, and I'm testing with httptest.Server:

httptest.Server creates some url like http://localhost:21345.

I want any request to connect to localhost:21345, so to accomplish that I set r.URL = server.URL()

I still need to let the server know what domain is being requested, and so I set r.Host = "customer1.example.com"

The request is issued, and everything looks like it works fine. Except cookies are written with domain=.localhost instead of .customer1.example.com. This still appears to work for subsequent requests to customer1.exmaple.com.

When doing a subsequent request for customer2.example.com however, http.Client still loads cookies from localhost, and so all the cookies intended to be scoped to customer1.example.com are now being sent to customer2.example.com.

Currently the only workaround is to maintain your own cookiejar, and load/store the cookies before and after the request manually.

Is this something the net/http maintainers would welcome a patch for? What would fixing the behavior mean for the compatibility promise? It seems to me that leaking data from one domain to another is a clear security concern. It doesn't seem like this is something that would be pervasive in a production application.

toothrot changed the title ~~http.Client should scope cookie to Request.Host before Request.URL~~ net/http: Client should scope cookie to Request.Host before Request.URL May 11, 2020

toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 11, 2020

toothrot added this to the Backlog milestone May 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http: Client should scope cookie to Request.Host before Request.URL #38988

net/http: Client should scope cookie to Request.Host before Request.URL #38988

colinclerk commented May 10, 2020

toothrot commented May 11, 2020

vdobler commented May 11, 2020

colinclerk commented May 12, 2020 •

edited

chrisguiney commented Feb 20, 2023

net/http: Client should scope cookie to Request.Host before Request.URL #38988

net/http: Client should scope cookie to Request.Host before Request.URL #38988

Comments

colinclerk commented May 10, 2020

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

toothrot commented May 11, 2020

vdobler commented May 11, 2020

colinclerk commented May 12, 2020 • edited

chrisguiney commented Feb 20, 2023

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

colinclerk commented May 12, 2020 •

edited