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.

Sign up for GitHub

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

Jump to bottom

x/net/http/httpproxy: document proxy authentication support #61505

Closed
gberche-orange opened this issue Jul 21, 2023 · 3 comments
Closed

x/net/http/httpproxy: document proxy authentication support #61505

gberche-orange opened this issue Jul 21, 2023 · 3 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@gberche-orange
Copy link

gberche-orange commented Jul 21, 2023 •
edited

Many tools built with golang rely on the environment variables documented in https://pkg.go.dev/golang.org/x/net/http/httpproxy#Config to let their users specify the http proxy to use (e.g. HTTPS_PROXY and NO_PROXY), and don't offer other configuration mechanism to configure http proxies used by outgoing http requests they make.

Besides, many CLIs such as curl have adopted the convention of extracting proxy authentication basic auth username and password from the http proxy url such as HTTPS_PROXY=https://user:pwd@fqdn , see https://curl.se/docs/manpage.html#-x

Using an environment variable to set the proxy has the same effect as using the -x, --proxy option. [...]
User and password that might be provided in the proxy string are URL decoded by curl. This allows you to pass in special characters such as @ by using %40 or pass in a colon with %3a.

Currently, the x/net/http/httpproxy#Config does not document the explicit support for username and password that would translate in http request made to comply with http proxy client authentication specified in https://datatracker.ietf.org/doc/html/rfc9110#section-11.7

This proposal suggests to document and implement as required such client proxy authentication support in proxy environment variables so that the golang binaries ecosystem relying on environment variables can leverage proxy authentication.

Note that the Deprecation of userinfo in http(s) URIs in RFC9110 (as I understand it), does not preclude using user info in URIs for configuration such as environment variables, but rather aims at preventing propagating Uris with user info from untrusted sources (e.g. in emails messages in the context of phishing attacks).
@gopherbot gopherbot added this to the Proposal milestone Jul 21, 2023
@seankhliao seankhliao changed the title proposal: /x/net/http/httpproxy: support proxy authentication in env vars eg HTTPS_PROXY=https://user:pwd@host x/net/http/httpproxy: support proxy authentication in env vars eg HTTPS_PROXY=https://user:pwd@host Jul 21, 2023
@seankhliao seankhliao removed this from the Proposal milestone Jul 21, 2023
@seankhliao
Copy link
Member

Moving out of proposals because there's nothing to change.
httpproxy already supports user:pass in urls, sending them in Proxy-Authorization.
Given that we document support for a complete URL, I'm not sure there's anything to do.

@seankhliao seankhliao added Documentation WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 21, 2023
@seankhliao seankhliao changed the title x/net/http/httpproxy: support proxy authentication in env vars eg HTTPS_PROXY=https://user:pwd@host x/net/http/httpproxy: document proxy authentication support Jul 21, 2023
@seankhliao seankhliao added this to the Unreleased milestone Jul 21, 2023
@gberche-orange gberche-orange mentioned this issue Jul 24, 2023
Closed
@gberche-orange
Copy link
Author

gberche-orange commented Jul 24, 2023 •
edited

Thanks @seankhliao for your review of this issue, and analysis that implementation already exists.

Given that we document support for a complete URL, I'm not sure there's anything to do.

I have looked for existing test coverage of the user info in the proxy url as a substitute for explicit documentation, but did not to find it so far, in http proxy related tests below

go/src/net/http/transport_test.go

Lines 3111 to 3186 in 7141d1e

var proxyFromEnvTests = []proxyFromEnvTest{
{env: "127.0.0.1:8080", want: "http://127.0.0.1:8080"},
{env: "cache.corp.example.com:1234", want: "http://cache.corp.example.com:1234"},
{env: "cache.corp.example.com", want: "http://cache.corp.example.com"},
{env: "https://cache.corp.example.com", want: "https://cache.corp.example.com"},
{env: "http://127.0.0.1:8080", want: "http://127.0.0.1:8080"},
{env: "https://127.0.0.1:8080", want: "https://127.0.0.1:8080"},
{env: "socks5://127.0.0.1", want: "socks5://127.0.0.1"},
// Don't use secure for http
{req: "http://insecure.tld/", env: "http.proxy.tld", httpsenv: "secure.proxy.tld", want: "http://http.proxy.tld"},
// Use secure for https.
{req: "https://secure.tld/", env: "http.proxy.tld", httpsenv: "secure.proxy.tld", want: "http://secure.proxy.tld"},
{req: "https://secure.tld/", env: "http.proxy.tld", httpsenv: "https://secure.proxy.tld", want: "https://secure.proxy.tld"},
// Issue 16405: don't use HTTP_PROXY in a CGI environment,
// where HTTP_PROXY can be attacker-controlled.
{env: "http://10.1.2.3:8080", reqmeth: "POST",
want: "<nil>",
wanterr: errors.New("refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")},
{want: "<nil>"},
{noenv: "example.com", req: "http://example.com/", env: "proxy", want: "<nil>"},
{noenv: ".example.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
{noenv: "ample.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
{noenv: "example.com", req: "http://foo.example.com/", env: "proxy", want: "<nil>"},
{noenv: ".foo.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
}
func testProxyForRequest(t *testing.T, tt proxyFromEnvTest, proxyForRequest func(req *Request) (*url.URL, error)) {
t.Helper()
reqURL := tt.req
if reqURL == "" {
reqURL = "http://example.com"
}
req, _ := NewRequest("GET", reqURL, nil)
url, err := proxyForRequest(req)
if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
t.Errorf("%v: got error = %q, want %q", tt, g, e)
return
}
if got := fmt.Sprintf("%s", url); got != tt.want {
t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
}
}
func TestProxyFromEnvironment(t *testing.T) {
ResetProxyEnv()
defer ResetProxyEnv()
for _, tt := range proxyFromEnvTests {
testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
os.Setenv("HTTP_PROXY", tt.env)
os.Setenv("HTTPS_PROXY", tt.httpsenv)
os.Setenv("NO_PROXY", tt.noenv)
os.Setenv("REQUEST_METHOD", tt.reqmeth)
ResetCachedEnvironment()
return ProxyFromEnvironment(req)
})
}
}
func TestProxyFromEnvironmentLowerCase(t *testing.T) {
ResetProxyEnv()
defer ResetProxyEnv()
for _, tt := range proxyFromEnvTests {
testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
os.Setenv("http_proxy", tt.env)
os.Setenv("https_proxy", tt.httpsenv)
os.Setenv("no_proxy", tt.noenv)
os.Setenv("REQUEST_METHOD", tt.reqmeth)
ResetCachedEnvironment()
return ProxyFromEnvironment(req)
})
}
}

The following lines seem to collectively implement client proxy authen from user info in proxy env vars, but they are not convenient to be used as a documentation
https://github.com/golang/net/blob/5e678bb28c36ba4aef595a4e468e51eda5d71c12/http/httpproxy/proxy.go#L151

go/src/net/http/transport.go

Lines 859 to 869 in 7141d1e

func (cm *connectMethod) proxyAuth() string {
if cm.proxyURL == nil {
return ""
}
if u := cm.proxyURL.User; u != nil {
username := u.Username()
password, _ := u.Password()
return "Basic " + basicAuth(username, password)
}
return ""
}

go/src/net/http/transport.go

Lines 1665 to 1668 in 7141d1e

if pa := cm.proxyAuth(); pa != "" {
pconn.mutateHeaderFunc = func(h Header) {
h.Set("Proxy-Authorization", pa)
}

In order to make it easier for the community of tools implicitly relying on
x/net/http/httpproxy env vars behavior, such as https://bosh.io/docs/cli-global-flags/#http-proxy, I've submitted golang/net#185 to explicitly document the existing support. I hope this can be useful.

@gopherbot
Copy link

Change https://go.dev/cl/512555 mentions this issue: net/http: document setting of Proxy-Authorization header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

net/http: document userinfo details regarding HttpProxy.Config gberche-orange/net
3 participants
@gberche-orange @gopherbot @seankhliao