From 750d247862e35e1df4e38e199945d05a1f40bed4 Mon Sep 17 00:00:00 2001 From: Justin Sherrill Date: Tue, 12 Feb 2019 14:01:32 -0500 Subject: [PATCH] Fixes #16909 - properly escape user and pass in proxy auth rest-client alongside URIs parse method introduces a bug where by usernames or passwords cannot have symbols in them. More Information: https://github.com/rest-client/rest-client/issues/661 https://github.com/ManageIQ/manageiq/pull/17318 --- app/lib/katello/util/http_proxy.rb | 19 +++++++-- app/lib/katello/util/proxy_uri.rb | 64 ++++++++++++++++++++++++++++++ test/lib/util/http_proxy_test.rb | 31 +++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 app/lib/katello/util/proxy_uri.rb create mode 100644 test/lib/util/http_proxy_test.rb diff --git a/app/lib/katello/util/http_proxy.rb b/app/lib/katello/util/http_proxy.rb index d7a14039d62..32deebab955 100644 --- a/app/lib/katello/util/http_proxy.rb +++ b/app/lib/katello/util/http_proxy.rb @@ -1,12 +1,23 @@ +require 'katello/util/proxy_uri' + module Katello module Util module HttpProxy def proxy_uri - URI("#{proxy_scheme}://#{proxy_user_info}@#{proxy_host}:#{proxy_port}").to_s if proxy_host - end + #Reset the scheme to proxy(s) based on http or https to handle cgi unescaping in rest-client + # this relies on katello/util/proxy_uri + if proxy_host + scheme = 'proxy' if proxy_scheme == 'http' + scheme = 'proxys' if proxy_scheme == 'https' + + uri = URI("#{scheme}://#{proxy_host}:#{proxy_port}") + if proxy_config && proxy_config[:user] + uri.user = CGI.escape(proxy_config[:user]) + uri.password = CGI.escape(proxy_config[:password]) + end - def proxy_user_info - "#{proxy_config[:user]}:#{proxy_config[:password]}" if proxy_config && proxy_config[:user] + uri.to_s + end end def proxy_config diff --git a/app/lib/katello/util/proxy_uri.rb b/app/lib/katello/util/proxy_uri.rb new file mode 100644 index 00000000000..cde70196d6f --- /dev/null +++ b/app/lib/katello/util/proxy_uri.rb @@ -0,0 +1,64 @@ +# +# The Ruby URI parser doesn't decode the percent encoded characters in the URI, in particular it +# doesn't decode the password which is frequently used when specifying proxy addresses and +# authentication. For example, the following code: +# +# require 'uri' +# proxy = URI.parse('http://myuser:%24%3fxxxx@192.168.100.10:3128') +# puts proxy.password +# +# Produces the following output: +# +# %24%3fxxxx +# +# But some gems, in particular `rest-client` and `kubeclient`, expect it to decode those characters, +# as they use the value returned by the `password` method directly, and thus they fail to +# authenticate against the proxy server when the password contains percent encoded characters. +# +# To address this issue this file adds a new `proxy` URI schema that almost identical to the `http` +# schema, but that decodes the password before returning it. Users can use this schema instead of +# `http` when they need to use percent encoded characters in the password. For example, the user +# can type in the UI the following proxy URL: +# +# proxy://myuser:%24%3fxxxx@192.168.100.10:3128 +# +# And the new schema will automatically translate `%24%3fxxxx` into `$?xxxx`. +# This fix is derived from: https://github.com/ManageIQ/manageiq/pull/17318 +require 'cgi' +require 'uri' + +module URI + class ProxyUri < HTTP + def password + value = super + value = CGI.unescape(value) if value + value + end + + def user + value = super + value = CGI.unescape(value) if value + value + end + end + + @@schemes['PROXY'] = ProxyUri +end + +module URI + class ProxysUri < HTTP + def password + value = super + value = CGI.unescape(value) if value + value + end + + def user + value = super + value = CGI.unescape(value) if value + value + end + end + + @@schemes['PROXYS'] = ProxysUri +end diff --git a/test/lib/util/http_proxy_test.rb b/test/lib/util/http_proxy_test.rb new file mode 100644 index 00000000000..c85b9e4caaa --- /dev/null +++ b/test/lib/util/http_proxy_test.rb @@ -0,0 +1,31 @@ +require 'katello_test_helper' + +module Katello + module Util + class HttpProxyTest < ActiveSupport::TestCase + include Katello::Util::HttpProxy + + def test_handles_no_username_test + SETTINGS[:katello][:cdn_proxy] = { + host: 'http://foobar.com', + username: nil, + password: nil + } + assert_equal 'proxy://foobar.com', proxy_uri + end + + def test_properly_escapes_username + SETTINGS[:katello][:cdn_proxy] = { + host: 'http://foobar.com', + user: 'red!hat', + password: 'red@hat' + } + assert_equal 'proxy://red%21hat:red%40hat@foobar.com', proxy_uri + + uri = URI.parse(proxy_uri) + assert_equal 'red!hat', uri.user + assert_equal 'red@hat', uri.password + end + end + end +end