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

Jump to bottom

net/http, x/net/http/httpproxy: http_proxy is being used for https requests #40909

Closed
tkopecek opened this issue Aug 19, 2020 · 11 comments
Closed

net/http, x/net/http/httpproxy: http_proxy is being used for https requests #40909

tkopecek opened this issue Aug 19, 2020 · 11 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.16

Comments

@tkopecek
Copy link

Go handles http_proxy/https_proxy/no_proxy in non-standard way. According to source comment http_proxy is used even for https urls. This is counterintuitive and not-working if it is not overriden.

My usecase is that I've local squid running with http_proxy exported. Nevertheless, squid is configured to handle also https but it is not propagated because it is using untrusted self-signed certificate. Go tries to connect to https via the proxy and fails with the reasonable certificate signed by unknown authority message. But at first place it shouldn't have used that proxy at all.

Code failing on this is referenced here
@tkopecek tkopecek mentioned this issue Aug 19, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/249440 mentions this issue: http/httpproxy: match http scheme when selecting http_proxy

@ALTree ALTree changed the title http_proxy is being used for https requests x/net: http_proxy is being used for https requests Aug 20, 2020
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 20, 2020
@gopherbot gopherbot added this to the Unreleased milestone Aug 20, 2020
@bradfitz
Copy link
Contributor

That CL's code seems fine, @fraenkel, but it's a behavior change away from the documented behavior (and changes the documented behavior), so the decision on whether to do this should be made intentionally.

I'm pretty sure the old behavior (of HTTP_PROXY also applying to "https" scheme URLs when HTTPS_PROXY was not present) was intentional but I don't have the time to go digging through git history to figure out whose behavior we were copying at the time, but I thought we were.

/cc @rsc who might also remember and should decide who makes this decision.

@ghost
Copy link

ghost commented Aug 21, 2020

One can tunnel any protocol through an HTTP proxy: https://wiki.squid-cache.org/Features/HTTPS#CONNECT_tunnel

@fraenkel
Copy link
Contributor

While CONNECT is the mechanism used, this is about the environment variables. all_proxy was meant to be the catch all but that is not implemented.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 6, 2020

@neild, this should probably go in now-ish with release notes for Go 1.16 so we can see if anybody is surprised during the rcs/betas.

@neild
Copy link
Contributor

neild commented Oct 13, 2020

Googling for http_proxy and https_proxy turns up mostly references to curl and wget in the first page of results, and I've confirmed that curl/wget don't use the value of the http_proxy environment variable for HTTPS connections. So while on one hand this is a behavioral change, it appears to be a change that brings http/httpproxy's behavior in line with the most common definition of these environment variables.

Tentatively SGTM.

gopherbot pushed a commit to golang/net that referenced this issue Oct 16, 2020
@fraenkel @neild
http/httpproxy: match http scheme when selecting http_proxy
7b1cca2 
Protocol specific proxies must match based on scheme.

If the https proxy is no configured, and the proxy for a https URL is
requested, no proxy should be returned.

Updates golang/go#40909

Change-Id: I62dfcf95d819c634e8f2862e891877a4eb55fca7
Reviewed-on: https://go-review.googlesource.com/c/net/+/249440
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
@dmitshur dmitshur modified the milestones: Unreleased, Go1.16 Nov 23, 2020
@dmitshur dmitshur changed the title x/net: http_proxy is being used for https requests x/net/http/httpproxy: http_proxy is being used for https requests Nov 23, 2020
@dmitshur
Copy link
Contributor

Closing as fixed. (We still need to document this change in Go 1.16 release notes, but that's tracked separately.)

The code change has happened in x/net in CL 249440, which got pulled into the standard library in CL 266898.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 23, 2020
@dmitshur dmitshur changed the title x/net/http/httpproxy: http_proxy is being used for https requests net/http, x/net/http/httpproxy: http_proxy is being used for https requests Nov 23, 2020
@seankhliao seankhliao mentioned this issue Mar 25, 2021
Closed
@taylanisikdemir
Copy link

taylanisikdemir commented Mar 31, 2021 •
edited

@tkopecek, @dmitshur Due to this breaking change in go1.16, our existing setup with HTTP_PROXY=my.proxy.endpoint stopped working and all https requests bypassed the proxy. Please mention this change as warning on https://golang.org/pkg/vendor/golang.org/x/net/http/httpproxy/#FromEnvironment

@dmitshur
Copy link
Contributor

@taylanisikdemir This change is documented in the Go 1.16 release notes, see the 5th paragraph at https://golang.org/doc/go1.16#net/http.

thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 21, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
6e9a2e6 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme.

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*
this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah mentioned this issue Jul 21, 2021
Draft
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 21, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
86970eb 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme.

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*
this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 22, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
50661f5 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme.

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 22, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
3475981 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme.

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 22, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
ce64c61 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme. For other schemes,
golang's standard behavior is preserved (and depends on the Go version used).

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 22, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
8bdd714 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme. For other schemes,
golang's standard behavior is preserved (and depends on the Go version used).

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 26, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
83d3fba 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme. For other schemes,
golang's standard behavior is preserved (and depends on the Go version used).

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 28, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
6850769 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme. For other schemes,
golang's standard behavior is preserved (and depends on the Go version used).

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/go-connections that referenced this issue Jul 28, 2021
@thaJeztah
sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior
a937e0c 
TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the
pre-go1.16 behavior for URLs using the 'tcp://' scheme. For other schemes,
golang's standard behavior is preserved (and depends on the Go version used).

Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other
scheme would use HTTP_PROXY. However, golang/net@7b1cca2
(per a request in golang/go#40909) changed this behavior to only use
HTTP_PROXY for `http://` schemes, no longer using a proxy for any other
scheme.

Docker uses the `tcp://` scheme as a default for API connections, to indicate
that the API is not "purely" HTTP. Various parts in the code also *require*

this scheme to be used. While we could change the default and allow http(s)
schemes to be used, doing so will take time, taking into account that there
are many installs in existence that have tcp:// configured as DOCKER_HOST.

This function detects if the `tcp://` scheme is used; if it is, it creates
a shallow copy of req, containing just the URL, and overrides the scheme with
'http', which should be sufficient to perform proxy detection.
For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without
altering the request.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thomasgl-orange thomasgl-orange mentioned this issue Aug 4, 2021
Merged
@SamKhGit SamKhGit mentioned this issue Sep 10, 2021
Merged
@ivandeex ivandeex mentioned this issue Oct 4, 2021
Merged
5 tasks
@medyagh medyagh mentioned this issue Nov 22, 2021
Closed
@patrickdillon patrickdillon mentioned this issue Mar 25, 2022
Merged
wadahiro added a commit to wadahiro/nproxy that referenced this issue Aug 2, 2022
@wadahiro
fix: detect proxy from environment on go 1.16
d202426 
See golang/go#40909
@dfawley dfawley mentioned this issue Sep 6, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/428775 mentions this issue: net/http: clarify proxy selection from environment

@gopherbot
Copy link

Change https://go.dev/cl/428795 mentions this issue: http/httpproxy: remove comment on https proxy precedance

gopherbot pushed a commit that referenced this issue Sep 6, 2022
@seankhliao @gopherbot
net/http: clarify proxy selection from environment
90f04dd 
For #40909
Fixes #54890

Change-Id: I00218bc1606eedb6194a3a7b81fd4d3f75325280
Reviewed-on: https://go-review.googlesource.com/c/go/+/428775
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
gopherbot pushed a commit to golang/net that referenced this issue Sep 7, 2022
@seankhliao @gopherbot
http/httpproxy: remove comment on https proxy precedance
1e95f45 
For golang/go#40909
For golang/go#54890

Change-Id: I1de1803f8fd00f54290404a8760d9f704ff766c3
Reviewed-on: https://go-review.googlesource.com/c/net/+/428795
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
WeiminShang added a commit to WeiminShang/net that referenced this issue Nov 16, 2022
@WeiminShang @gopherbot
http/httpproxy: remove comment on https proxy precedance
e4d29b1 
For golang/go#40909
For golang/go#54890

Change-Id: I1de1803f8fd00f54290404a8760d9f704ff766c3
Reviewed-on: https://go-review.googlesource.com/c/net/+/428795
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
sparrc added a commit to sparrc/amazon-ecs-agent that referenced this issue Jan 28, 2023
@sparrc
Pin httpproxy package
1071a0f 
golang.org/x/net/http/httpproxy changed it's behavior regarding the
HTTP_PROXY and HTTPS_PROXY env vars.

it was to fix this issue: golang/go#40909
and this is the change: https://go-review.googlesource.com/c/net/+/249440

After this change the HTTP_PROXY env var will be ignored for HTTPS requests,
you have to also set HTTPS_PROXY in order for HTTPS requests from the agent
to be proxied. This means that if there are customers setting HTTP_PROXY
but not HTTPS_PROXY, their requests will no longer be proxied.

Pin to an older version of the httpproxy package to avoid this behavior
change.

Copying the file in-repo so that we can upgrade the rest of
golang.org/x/net without issue, and also so that any potential future
uses of x/net/http/httpproxy do not collide with maintaining this
previous desired behavior.
sparrc added a commit to sparrc/amazon-ecs-agent that referenced this issue Jan 28, 2023
@sparrc
Pin httpproxy package
9083ac2 
golang.org/x/net/http/httpproxy changed it's behavior regarding the
HTTP_PROXY and HTTPS_PROXY env vars.

it was to fix this issue: golang/go#40909
and this is the change: https://go-review.googlesource.com/c/net/+/249440

After this change the HTTP_PROXY env var will be ignored for HTTPS requests,
you have to also set HTTPS_PROXY in order for HTTPS requests from the agent
to be proxied. This means that if there are customers setting HTTP_PROXY
but not HTTPS_PROXY, their requests will no longer be proxied.

Pin to an older version of the httpproxy package to avoid this behavior
change.

Copying the file in-repo so that we can upgrade the rest of
golang.org/x/net without issue, and also so that any potential future
uses of x/net/http/httpproxy do not collide with maintaining this
previous desired behavior.
sparrc added a commit to sparrc/amazon-ecs-agent that referenced this issue Jan 28, 2023
@sparrc
Pin httpproxy package
462356c 
golang.org/x/net/http/httpproxy changed it's behavior regarding the
HTTP_PROXY and HTTPS_PROXY env vars.

it was to fix this issue: golang/go#40909
and this is the change: https://go-review.googlesource.com/c/net/+/249440

After this change the HTTP_PROXY env var will be ignored for HTTPS requests,
you have to also set HTTPS_PROXY in order for HTTPS requests from the agent
to be proxied. This means that if there are customers setting HTTP_PROXY
but not HTTPS_PROXY, their requests will no longer be proxied.

Pin to an older version of the httpproxy package to avoid this behavior
change.

Copying the file in-repo so that we can upgrade the rest of
golang.org/x/net without issue, and also so that any potential future
uses of x/net/http/httpproxy do not collide with maintaining this
previous desired behavior.
sparrc added a commit to aws/amazon-ecs-agent that referenced this issue Jan 30, 2023
@sparrc
Pin httpproxy package
822c4eb 
golang.org/x/net/http/httpproxy changed it's behavior regarding the
HTTP_PROXY and HTTPS_PROXY env vars.

it was to fix this issue: golang/go#40909
and this is the change: https://go-review.googlesource.com/c/net/+/249440

After this change the HTTP_PROXY env var will be ignored for HTTPS requests,
you have to also set HTTPS_PROXY in order for HTTPS requests from the agent
to be proxied. This means that if there are customers setting HTTP_PROXY
but not HTTPS_PROXY, their requests will no longer be proxied.

Pin to an older version of the httpproxy package to avoid this behavior
change.

Copying the file in-repo so that we can upgrade the rest of
golang.org/x/net without issue, and also so that any potential future
uses of x/net/http/httpproxy do not collide with maintaining this
previous desired behavior.
@golang golang locked and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
9 participants
@bradfitz @neild @fraenkel @taylanisikdemir @dmitshur @ALTree @gopherbot @tkopecek and others