Bug 1566615 - Unable to use special characters in HTTPS proxy field when adding/validating container provider
Summary: Unable to use special characters in HTTPS proxy field when adding/validating ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: UI - OPS
Version: 5.9.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: GA
: 5.10.0
Assignee: Oved Ourfali
QA Contact: juwatts
URL:
Whiteboard:
Depends On:
Blocks: 1595269
TreeView+ depends on / blocked
 
Reported: 2018-04-12 15:40 UTC by Tuan
Modified: 2019-02-07 23:01 UTC (History)
16 users (show)

Fixed In Version: 5.10.0.21
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-02-07 23:01:41 UTC
Category: Bug
Cloudforms Team: Container Management
Target Upstream Version:


Attachments (Terms of Use)
Monkey patch rest-client to support percent encoded proxy user/password. (2.06 KB, patch)
2018-07-24 18:16 UTC, Joe Rafaniello
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2019:0212 None None None 2019-02-07 23:01:49 UTC

Description Tuan 2018-04-12 15:40:00 UTC
Description of problem:

Unable to use special characters. 
Using a special character fails url validation.
When using a URL encoded character, cloudforms uses the encoded characters literally when contacting the proxy instead of the encoded char. (ie - %3F instead of ?)

Version-Release number of selected component (if applicable):
5.9.0

How reproducible:
Everytime

Steps to Reproduce:
Issue can be reproduced when the proxy password has special characters.


Credential validation was not successful: bad URI(is not URI?): http://cfme:$?rhel@10.10.182.50:3128


But encode the proxy URL with 

https://www.w3schools.com/tags/ref_urlencode.asp

The result is 

http%3A%2F%2Fcfme%3A%24%3Frhel%4010.10.182.50%3A3128


If you use the result to valid it the provider it dose work.

Actual results:


Expected results:


Additional info:

Comment 3 Mooli Tayer 2018-04-17 13:33:03 UTC
According to the URI spec[1] the userinfo component must be pct-encoded.

Since pct-encoding is not idempotent, if we attempt to encode it ourselves we would be causing an error when IT IS encoded properly.

Also if it is not properly encoded, we could not even parse the URL to figure out what the userinfo component is! (password can contain '@' '//' '/' etc)


[1] https://www.ietf.org/rfc/rfc3986.txt (section 3.2.1)

Comment 6 Mooli Tayer 2018-04-18 09:17:06 UTC
Are we talking about a global proxy server for ALL of the container providers configured in EVM-> Configuration -> Advanced -> "http_proxy" -> "openshift"

Or a proxy server for an INDIVIDUAL provider configured in
"Add New Container Provider" page -> bottom of the screen(only visible after selecting "Openshift") "Settings" section -> "Proxy" tab -> Proxy Settings -> "HTTP Proxy"?

I've been working since morning on getting a test environment so I can test all scenarios since I might have misunderstood, but a clarification would help.

Sorry if I closed prematurely, I'll get to the bottom this and address accordingly, thanks.

Comment 9 Juan Hernández 2018-04-19 14:26:39 UTC
The root cause of this problem is that the `URI.parse` method does not decode the percent encoded characters that appear in the `userinfo` part of the URL. For example, the following code:

  require 'uri'
  u = URI.parse('http://myuser:%24%3fxxxx@192.168.100.10:3128')
  puts u.password

Produces the following output:

  %24%3fxxxx
But according to RFC 3986 it should decode the precent encoded characters, and produce the following output:

  $?xxxx

The `rest-client` gem, for example, extracts directly the proxy password from the URI calling this method. As a result it fails to authenticate, because it sends the encoded characters. We use that gem for the HTTP communications with the Kubernetes and OpenShift API servers.

The only simple solution that I can find to this problem is to patch the URI class, with a file `uri_patch.rb` inside the `config/initializers` directory:

---8<---
require 'uri'

class URI::HTTP
  alias_method :_password, :password

  def password
    value = _password
    value = URI.decode(value) if value
    value
  end
end
--->8---

Note however that this has to be combined with manually encoding of the URL by the user. For example, if the user name is `myuser` and the password is `$?xxxx` the user will have to put exactly this in the UI:

  http://myuser:%24%3fxxxx@192.168.100.10:3128

That patch is what I am proposing in this pull request:

  Patch URI to decode password
  https://github.com/ManageIQ/manageiq/pull/17318

However I expect objections to merge this patch, as it affects all users of the `URI.parse` method, not just the Kubernetes and OpenShift providers.

The right solution would probably be, as suggested by David Luong, to add to the UI two new fields for the user name and the password. But that would require changes to the UI, to the backend, to the `kubeclient` gem, and possible to the `rest-client` gem (which we don't maintain).

My suggestion is to see if dropping that patch file in the `config/initializers` directory solves the problem for the user. In parallel, but with less urgency, we can consider what it takes to do the right solution.

Comment 17 Juan Hernández 2018-04-20 08:08:31 UTC
A less intrusive way to solve this is to add a new `proxy` URI schema that does the percent decoding automatically. I have updated the pull request accordingly:

  Add proxy URI schema
  https://github.com/ManageIQ/manageiq/pull/17318

To use it drop a `proxy_uri.rb` file with the following content into the `config/initializers` directory:

---8<---
require 'cgi'
require 'uri'

module URI
  class PROXY < HTTP
    def password
      CGI.unescape(super())
    end
  end

  @@schemes['PROXY'] = PROXY
end
--->8---

Then, when adding or editing the provider, use a proxy URI like this:

  proxy://myuser:%24%3fxxxx@192.168.122.40:3128

Note the use of `proxy` instead of `http` and the percent encoding of the characters.

Comment 18 Juan Hernández 2018-04-20 08:19:55 UTC
A simple command to generate the percent encoded password:

  $ ruby -e "require 'cgi'; puts CGI.escape(ARGV[0])" '$?xxxx'
  %24%3Fxxxx

Comment 41 Joe Rafaniello 2018-07-24 18:16:58 UTC
Created attachment 1470341 [details]
Monkey patch rest-client to support percent encoded proxy user/password.

Based on changes not yet merged in rest-client:
https://github.com/rest-client/rest-client/pull/665

Comment 42 Joe Rafaniello 2018-07-24 18:24:33 UTC
I have the proposed attached monkey patch to rest-client.  Note, it needs to load parts of rest-client in order to monkey patch it.  This could make normal processes that never load it consume more memory and ultimately changes how rest-client is loaded.

If we really want to ship this fix, I can open a PR to manageiq with this change.

Here is how I verified it worked in rails console:

Before:

irb(main):004:0> r = RestClient::Request.new(method: :get, url: 'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1')
=> <RestClient::Request @method="get", @url="http://127.0.0.1/api">=> <RestClient::Request @method="get", @url="https://127.0.0.1/api">
irb(main):002:0> r.net_http_object('host', 80).proxy_user
=> "%24myuser"
irb(main):003:0> r.net_http_object('host', 80).proxy_pass
=> "%24%3Fxxxx"

After:
irb(main):004:0> r = RestClient::Request.new(method: :get, url: 'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1')
=> <RestClient::Request @method="get", @url="http://127.0.0.1/api">=> <RestClient::Request @method="get", @url="https://127.0.0.1/api">
irb(main):002:0> r.net_http_object('host', 80).proxy_user
=> "$myuser"
irb(main):003:0> r.net_http_object('host', 80).proxy_pass
=> "$?xxxx"

Comment 43 bascar 2018-07-24 19:20:06 UTC
I changed the target release to 5.10 so am not looking to do anything in 5.9.x anymore, have concerns like you on monkey patching. Maybe we have this same conversation in a couple months if nothing has happened on that PR in rest-client project.

Comment 45 bascar 2018-10-17 13:56:48 UTC
Looks like we are now at that point where we need to carry the patch ourselves. Joe R lets move ahead as you describe in comment #42.

Comment 47 Joe Rafaniello 2018-10-17 18:18:27 UTC
Opened PR to manageiq to monkey patch the rest client library since it's still not merged and released upstream, see comment #46.

Comment 50 CFME Bot 2018-10-19 14:20:43 UTC
New commit detected on ManageIQ/manageiq/master:

https://github.com/ManageIQ/manageiq/commit/d2b0e320ac475e6a16a8cd52c30de49b3665695f
commit d2b0e320ac475e6a16a8cd52c30de49b3665695f
Author:     Joe Rafaniello <jrafanie@redhat.com>
AuthorDate: Wed Oct 17 13:56:48 2018 -0400
Commit:     Joe Rafaniello <jrafanie@redhat.com>
CommitDate: Wed Oct 17 13:56:48 2018 -0400

    RestClient: Support percent encoded proxy user/pass

    https://bugzilla.redhat.com/show_bug.cgi?id=1566615

    Monkey patch RestClient for patch releases of rest-client 2.0.0.
    The underlying method, net_http_object, has not been modified since 2015
    so it should be relatively safe to monkey patch this until the upstream
    PR gets merged/released.

    Upstream PR: https://github.com/rest-client/rest-client/pull/665

    To verify this works, run bin/rails console:

    Before:

    ```
    irb(main):004:0> r = RestClient::Request.new(method: :get, url:
    'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1')
    => <RestClient::Request @method="get", @url="http://127.0.0.1/api">=>
    <RestClient::Request @method="get", @url="https://127.0.0.1/api">
    irb(main):002:0> r.net_http_object('host', 80).proxy_user
    => "%24myuser"
    irb(main):003:0> r.net_http_object('host', 80).proxy_pass
    => "%24%3Fxxxx"
    ```

    After:

    ```
    irb(main):004:0> r = RestClient::Request.new(method: :get, url:
    'http://127.0.0.1/api', proxy: 'http://%24myuser:%24%3Fxxxx@127.0.0.1')
    => <RestClient::Request @method="get", @url="http://127.0.0.1/api">=>
    <RestClient::Request @method="get", @url="https://127.0.0.1/api">
    irb(main):002:0> r.net_http_object('host', 80).proxy_user
    => "$myuser"
    irb(main):003:0> r.net_http_object('host', 80).proxy_pass
    => "$?xxxx"
    ```

 lib/patches/rest_client_patch.rb | 29 +
 lib/vmdb_helper.rb | 1 +
 2 files changed, 30 insertions(+)

Comment 51 CFME Bot 2018-10-19 20:05:44 UTC
New commit detected on ManageIQ/manageiq/hammer:

https://github.com/ManageIQ/manageiq/commit/81bec89af724da633bfa316f40e04cca8f55e2de
commit 81bec89af724da633bfa316f40e04cca8f55e2de
Author:     Brandon Dunne <brandondunne@hotmail.com>
AuthorDate: Fri Oct 19 10:19:26 2018 -0400
Commit:     Brandon Dunne <brandondunne@hotmail.com>
CommitDate: Fri Oct 19 10:19:26 2018 -0400

    Merge pull request #18105 from jrafanie/rest_client_monkey_patch4percent_encoded_proxy_user_password

    RestClient: Support percent encoded proxy user/pass
    (cherry picked from commit 5a784c39bfc87d4d281259061b7578ce140d0283)

    https://bugzilla.redhat.com/show_bug.cgi?id=1566615

 lib/patches/rest_client_patch.rb | 29 +
 lib/vmdb_helper.rb | 1 +
 2 files changed, 30 insertions(+)

Comment 53 juwatts 2018-11-15 16:25:21 UTC
Verified in 5.10.0.24.20181113213923_03b81fd

1) Setup proxy server with htpasswd authentication:

first request is the correct username and password, cfme:$?rhel, and the second is a bad username:

juwatts@localhost /tmp $ export http_proxy=http://cfme:%24%3Frhel@10.8.198.140:3128
juwatts@localhost /tmp $ wget www.google.com
--2018-11-14 16:09:22--  http://www.google.com/
Connecting to 10.8.198.140:3128... connected.
Proxy request sent, awaiting response... 200 OK
Length: unspecified [text/html]
Saving to: ‘index.html.10’

index.html.10                                                           [ <=>                                                                                                                                                                ]  11.98K  --.-KB/s    in 0.01s  

2018-11-14 16:09:22 (1.19 MB/s) - ‘index.html.10’ saved [12271]

juwatts@localhost /tmp $ export http_proxy=http://cfm2:%24%3Frhel@10.8.198.140:3128
juwatts@localhost /tmp $ wget www.google.com
--2018-11-14 16:09:30--  http://www.google.com/
Connecting to 10.8.198.140:3128... connected.
Proxy request sent, awaiting response... 407 Proxy Authentication Required
2018-11-14 16:09:30 ERROR 407: Proxy Authentication Required.

juwatts@localhost /tmp $ 

2) Added an openshift provider and in the proxy setting, added the following URI:

http://cfme:%24%3Frhel@10.8.198.140:3128

Successfully saw access in squid proxy

1542296585.051     18 10.8.198.119 TCP_TUNNEL/200 11070 CONNECT epacific-ocp-master-v3.cmqe.lab.eng.rdu2.redhat.com:8443 cfme HIER_DIRECT/10.8.218.81 -
1542296585.068      9 10.8.198.119 TCP_TUNNEL/200 2855 CONNECT epacific-ocp-master-v3.cmqe.lab.eng.rdu2.redhat.com:8443 cfme HIER_DIRECT/10.8.218.81 -

Comment 55 errata-xmlrpc 2019-02-07 23:01:41 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:0212


Note You need to log in before you can comment on or make changes to this bug.