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: send User-Agent in proxy CONNECT #13290

Closed
cdcs opened this issue Nov 17, 2015 · 7 comments
Closed

net/http: send User-Agent in proxy CONNECT #13290

cdcs opened this issue Nov 17, 2015 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cdcs
Copy link

cdcs commented Nov 17, 2015

I am using a http proxy to connect to an https target. This proxy forbids connections from unknown user-agents.
I am building a new request to have control over the request headers:

req, err := http.NewRequest("GET", rdurl.String(), nil)
req.Header.Set("User-Agent", "Wget/1.9.1")

But even with this change in the user-agent, I am getting a 403 Forbidden answer from the proxy. When dumping the http connection with wireshark, I see that the go http stack is using the default user-agent instead of the one I specified on the request.
I think I could trace the problem back to the dialConn function in transport.go:

func (t *Transport) dialConn(cm connectMethod) (*persistConn, error){
        .....        
        conn := pconn.conn
        connectReq := &Request{
                Method: "CONNECT",
                URL:    &url.URL{Opaque: cm.targetAddr},
                Host:   cm.targetAddr,
                Header: make(Header),
        }
        ...
}

In this function, a new request is being built to the CONNECT to the proxy, with a new empty header being allocated. This header is only filled with the proxy authentication if any. I would expect this Header to be filled, as well, with the elements set in the upper request. Alternatively, the Header should be copied from the other request and the proxy authentication later appended.

Go Version: go1.5.1 linux/amd64
Thanks!

@bradfitz bradfitz changed the title net/http/transport: CONNECT to proxy ignores request headers, namely User-Agent net/http: CONNECT to proxy ignores request headers, namely User-Agent Nov 17, 2015
@minux
Copy link
Member

minux commented Nov 17, 2015 via email

@cdcs
Copy link
Author

cdcs commented Nov 17, 2015

Ok, that makes perfect sense.
The real question is that now you don't have any control over the User-Agent used for the CONNECT. In my specific use-case that makes all the difference, but I don't know how widespread is this specific need.

@glebk
Copy link

glebk commented Nov 21, 2015

The question of what headers to send to proxy is indeed an interesting one, and a good place to see this issue develop is in the CURL library, which (after some iterations, and a vulnerability) figured out what I think is the correct approach.

You simply have to specify explicitly the headers that you want sent to proxy.

There are several ways to achieve this, the most trivial being:

req, err := http.NewRequest("GET", rdurl.String(), nil)
req.Header.Set("User-Agent", "Wget/1.9.1")
req.ProxyHeader.Set("User-Agent", "Wget/1.9.1")

Going in a direction @minux suggests - defining a fixed (and hidden) list of headers which are also sent to proxy feels like a move in the wrong direction because the behavior is hard to debug for user, and the list of headers and their impact (over time) can be dangerous -- what others headers will we add, how will their function change over time, etc.

I understand that introducing a new field to Request struct is not something to be taken lightly, and I'll be happy to hear other thoughts, as well as help with the implementation and testing of this feature.

@bradfitz
Copy link
Contributor

What do browsers do?

I think whitelisting User-Agent might not be all that bad. If we stick with what browsers do, it might not be too unpredictable or unexpected.

@glebk
Copy link

glebk commented Nov 22, 2015

My understanding is that browsers will usually send only the User-Agent header to the proxy, so if the goal is to satisfy the ACL on the proxy that will most likely suffice. Even though Squid for example allows ACL also on Referer and MIME headers (in addition to User-Agent).

Mostly solving by whitelisting means we decide which of the headers it is OK to share with the proxy, but we lose the ability to share certain headers only with the proxy and not with destination host.

I don't know if that is something other people might find useful (but some do).

@rsc rsc changed the title net/http: CONNECT to proxy ignores request headers, namely User-Agent net/http: send User-Agent in proxy CONNECT Dec 28, 2015
@rsc rsc added this to the Go1.6Maybe milestone Dec 28, 2015
@bradfitz
Copy link
Contributor

bradfitz commented Jan 6, 2016

This is more involved than I thought it might be: it's not simply a matter of adding the User-Agent value to the empty header map, since the original request is not in scope where we dial. And we can't simply pass the original *http.Request to the dialConn code because of socket late binding.

I think we'll probably have to address this with new API surface, but that would have to wait until Go 1.7.

As a workaround in the meantime, perhaps you can just set the Transport.DialTLS hook and do the proxy connection and CONNECT request / response yourself (in your DialTLS hook). Looks like that should work.

@bradfitz bradfitz modified the milestones: Go1.7, Go1.6Maybe Jan 6, 2016
@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 7, 2016
@bradfitz bradfitz self-assigned this Nov 1, 2016
@gopherbot
Copy link

CL https://golang.org/cl/32481 mentions this issue.

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

7 participants