Skip to content

Commit

Permalink
Decode user and password from env configured proxy
Browse files Browse the repository at this point in the history
If someone sets an env variable defining a http_proxy, containing a
username / password with percent-encoded characters, then the resulting
base64 encoded auth header will be wrong.

For example, suppose a username is `Y\X` and the password is `R%S] ?X`.
Properly URL encoded the proxy url would be:

    http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000

The resulting proxy auth header should be: `WVxYOlIlU10gP1g=`, but the
getters defined by ruby StdLib `URI` return a username `Y%5CX` and
password `R%25S%5D%20%3FX`, resulting in `WSU1Q1g6UiUyNVMlNUQlMjAlM0ZY`.
As a result the proxy will deny the request.

Please note that this is my first contribution to the ruby ecosystem, to
standard lib especially and I am not a ruby developer.

Sorry for that and a happy and healthy 2021!

References:

- https://gitlab.com/gitlab-org/gitlab/-/issues/289836
- https://bugs.ruby-lang.org/projects/ruby-master/repository/trunk/revisions/58461
- https://bugs.ruby-lang.org/issues/17542
  • Loading branch information
leipert committed Feb 9, 2021
1 parent d9ba437 commit 5d9ee8e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/net/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,9 @@ def proxy_port
# The username of the proxy server, if one is configured.
def proxy_user
if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env
proxy_uri&.user
require 'cgi/util'
user = proxy_uri&.user
CGI.unescape(user) if user
else
@proxy_user
end
Expand All @@ -1189,7 +1191,9 @@ def proxy_user
# The password of the proxy server, if one is configured.
def proxy_pass
if ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE && @proxy_from_env
proxy_uri&.password
require 'cgi/util'
pass = proxy_uri&.password
CGI.unescape(pass) if pass
else
@proxy_pass
end
Expand Down
17 changes: 17 additions & 0 deletions test/net/http/test_http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,23 @@ def test_proxy_eh_ENV_with_user
end
end

def test_proxy_eh_ENV_with_urlencoded_user
TestNetHTTPUtils.clean_http_proxy_env do
ENV['http_proxy'] = 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000'

http = Net::HTTP.new 'hostname.example'

assert_equal true, http.proxy?
if Net::HTTP::ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE
assert_equal "Y\\X", http.proxy_user
assert_equal "R%S] ?X", http.proxy_pass
else
assert_nil http.proxy_user
assert_nil http.proxy_pass
end
end
end

def test_proxy_eh_ENV_none_set
TestNetHTTPUtils.clean_http_proxy_env do
assert_equal false, Net::HTTP.new('hostname.example').proxy?
Expand Down

0 comments on commit 5d9ee8e

Please sign in to comment.