Bug 1436221

Summary: Add provider screen: No error message when trying to add a provider with a custom ssl certificate that does not match
Product: Red Hat CloudForms Management Engine Reporter: Pavel Zagalsky <pzagalsk>
Component: ProvidersAssignee: Beni Paskin-Cherniavsky <cben>
Status: CLOSED CURRENTRELEASE QA Contact: Einat Pacifici <epacific>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 5.8.0CC: cben, cpelland, epacific, fsimonce, jfrey, jhardy, mtayer, obarenbo, simaishi
Target Milestone: GAKeywords: TestOnly
Target Release: 5.9.0Flags: epacific: automate_bug+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: container
Fixed In Version: 5.9.0.1 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 1441670 (view as bug list) Environment:
Last Closed: 2018-03-06 15:46:37 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: Container Management Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1441670    
Attachments:
Description Flags
custom_cert_failure_evm.log
none
custom_cert_failure_production.log
none
custom_ssl_error_screenshot.png
none
certificate.log none

Description Pavel Zagalsky 2017-03-27 13:13:00 UTC
Created attachment 1266645 [details]
custom_cert_failure_evm.log

Description of problem:
If using the SSL trusting custom CA as an option for Sec Protocol the provider addition will fail

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

How reproducible:
Always

Steps to Reproduce:
1. Go to Compute --> Containers --> Providers
2. Select Configuration --> Add Existing Provider
3. Add provider's name/hostname/port/token and select SSL trusting custom CA as the option for Sec Protocol
4. Try to validate and add the provider

Actual results:
When attempting to validate the token there's no alert wether validation worked or failed. After pressing the Add button there will be an error screen telling:
hostname "xxxx.redhat.com" does not match the server certificate [ems_container/create]

Expected results:
There should be an alert to tell if the validation succeeded or failed.
The provider addition should work

Additional info:
Added evm.log and production.log with the details

Comment 2 Pavel Zagalsky 2017-03-27 13:13:30 UTC
Created attachment 1266648 [details]
custom_cert_failure_production.log

Comment 3 Pavel Zagalsky 2017-03-27 13:21:56 UTC
Created attachment 1266655 [details]
custom_ssl_error_screenshot.png

Comment 4 Pavel Zagalsky 2017-03-27 13:32:31 UTC
Created attachment 1266658 [details]
certificate.log

Comment 5 Beni Paskin-Cherniavsky 2017-03-27 21:04:37 UTC
Can reproduce on master (with slightly different symptom, I don't even get the error screen but I see it sent to the browser, perhaps be dev/prod difference?)

[----] F, [2017-03-27T23:50:58.487188 #2693:2ab1edd61f54] FATAL -- : Error caught: [OpenSSL::SSL::SSLError] hostname "ep-ose32-master.scl.lab.tlv.redhat.com" does not match the server certificate
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/2.3.0/openssl/ssl.rb:315:in `post_connection_check'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/2.3.0/net/http.rb:944:in `connect'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/2.3.0/net/http.rb:863:in `do_start'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/2.3.0/net/http.rb:852:in `start'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rest-client-2.0.1/lib/restclient/request.rb:715:in `transmit'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rest-client-2.0.1/lib/restclient/request.rb:145:in `execute'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rest-client-2.0.1/lib/restclient/request.rb:52:in `execute'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/rest-client-2.0.1/lib/restclient/resource.rb:51:in `get'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:437:in `block in fetch_entities'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:99:in `handle_exception'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:437:in `fetch_entities'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:427:in `load_entities'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:111:in `discover'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/kubeclient-2.3.0/lib/kubeclient/common.rb:78:in `method_missing'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/manageiq-ui-classic-31bd63966d71/app/controllers/mixins/ems_common_angular.rb:477:in `get_hostname_from_routes'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/manageiq-ui-classic-31bd63966d71/app/controllers/mixins/ems_common_angular.rb:434:in `set_ems_record_vars'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/manageiq-ui-classic-31bd63966d71/app/controllers/mixins/ems_common_angular.rb:56:in `update_ems_button_validate'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/bundler/gems/manageiq-ui-classic-31bd63966d71/app/controllers/mixins/ems_common_angular.rb:105:in `create_ems_button_validate'

We may need to extend https://github.com/ManageIQ/manageiq-ui-classic/pull/314 to rescue all exceptions.

I'm surprised SSLError is escaping out of kubeclient without becoming KubeException.  There is specific test for that:
https://github.com/abonas/kubeclient/blob/f489a35027a7/test/test_kubeclient.rb#L95-L105
Actually `handle_exception` only catches RestClient::Exception, so I'm surprised that test works.

Comment 6 Beni Paskin-Cherniavsky 2017-03-27 21:34:20 UTC
Figured it out.  Solution probably belongs in kubeclient gem. https://github.com/abonas/kubeclient/issues/240
But rescue-all in get_hostname_from_routes might also be prudent.

Comment 7 Mooli Tayer 2017-03-28 12:49:15 UTC
According to Pavel, he found the positive flow failing: he added the correct CA Certificates and got the failure he described (although the error seems to say the certificate does not match)

Beni please work with him to identify the full range of the error:
Is the positive flow (correct certificate) working? is he using the correct certificate etc.

Comment 8 Beni Paskin-Cherniavsky 2017-03-28 14:18:13 UTC
Your server's certificate doesn't cover the hostname ep-ose32-master.scl.lab.tlv.redhat.com.  It covers the IPs.
You can see it by running:

$ openssl s_client -connect ep-ose32-master.scl.lab.tlv.redhat.com:8443 -showcerts < /dev/null | openssl x509 -text 
...
            X509v3 Subject Alternative Name: 
                DNS:kubernetes, DNS:kubernetes.default, DNS:kubernetes.default.svc, DNS:kubernetes.default.svc.cluster.local, DNS:openshift, DNS:openshift.default, DNS:openshift.default.svc, DNS:openshift.default.svc.cluster.local, DNS:10.35.160.9, DNS:172.30.0.1, IP Address:10.35.160.9, IP Address:172.30.0.1
...


Using 10.35.160.9 in add provider dialog, I don't get the error, and was able to add it.

For Hawkular I believe using an IP can not work (router relies on sent hostname to know which pods you want to talk to), and you'll have to install a working  cert for the right hostname.

Comment 9 Beni Paskin-Cherniavsky 2017-03-28 14:19:06 UTC
The bug with crashing instead of nice error flash still needs to be fixed of course.  Let me know if this unblocks you?

Comment 10 Federico Simoncelli 2017-03-28 20:58:35 UTC
(In reply to Beni Paskin-Cherniavsky from comment #8)
> Using 10.35.160.9 in add provider dialog, I don't get the error, and was
> able to add it.

Then this seems a deployment issue. Should we close this as NOTABUG?

Comment 11 Federico Simoncelli 2017-03-28 21:01:38 UTC
(In reply to Beni Paskin-Cherniavsky from comment #9)
> The bug with crashing instead of nice error flash still needs to be fixed of
> course.  Let me know if this unblocks you?

OK we can re-purpose this BZ to handle that (instead of closing) but you need to update the title, and make clear what we're going to fix here.
Or you can close this and open a specific BZ.

Comment 13 Mooli Tayer 2017-04-09 12:08:29 UTC
Re appropriating this per comment 11

Comment 14 Beni Paskin-Cherniavsky 2017-04-09 12:43:58 UTC
OK, exact bug here is that any SSL error except "certificate verify failed"
crashes the UI on pressing [Validate].
Fix: https://github.com/ManageIQ/manageiq-ui-classic/pull/972

To verify the fix you'd actually need a provider with misconfigured SSL :-)
such as ep-ose32-master whose cert does not cover it's hostname.

Comment 15 CFME Bot 2017-04-09 18:13:27 UTC
New commit detected on ManageIQ/manageiq-ui-classic/master:
https://github.com/ManageIQ/manageiq-ui-classic/commit/7e2bc528a132efd5a6858947964d572ab53d6bb1

commit 7e2bc528a132efd5a6858947964d572ab53d6bb1
Author:     Beni Cherniavsky-Paskin <cben@redhat.com>
AuthorDate: Sun Apr 9 13:59:22 2017 +0300
Commit:     Beni Cherniavsky-Paskin <cben@redhat.com>
CommitDate: Sun Apr 9 13:59:22 2017 +0300

    Catch SSLError too when adding a provider
    
    Expands #314.
    KubeException already covers the common "certificate verify failed" SSL
    errors but not rarer ones like "does not match the server certificate".
    This might be resolved in Kubeclient or RestClient one day
    (https://github.com/abonas/kubeclient/issues/240) but it's blocked
    on backward compatibility concerns so let's catch it here for now.
    
    get_hostname_from_routes is best-effort, should never crash UI.
    Any SSL error will probably fail both get_hostname_from_routes and the
    main validation code; the error from validation will then be displayed
    in a red flash.
    https://bugzilla.redhat.com/show_bug.cgi?id=1436221

 app/controllers/mixins/ems_common_angular.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment 16 Federico Simoncelli 2017-04-11 08:22:19 UTC
Beni, the PR you mentioned in comment 14 is merged, is there anything else to handle here or can we move this to POST?

Comment 18 Beni Paskin-Cherniavsky 2017-04-20 12:56:25 UTC
This is still broken in some cases because I caught the wrong exception :-(
Thanks Pavel for finding.

Repro: select "SSL trusting custom CA" mode, write garbage e.g. "x" into Trusted CA Certificates, press Validate.

[----] F, [2017-04-20T15:37:39.951184 #29642:2ac09a5c10f4] FATAL -- : Error caught: [OpenSSL::X509::CertificateError] header too long
/home/bpaskinc/miq/master/app/models/endpoint.rb:58:in `initialize'
/home/bpaskinc/miq/master/app/models/endpoint.rb:58:in `new'
/home/bpaskinc/miq/master/app/models/endpoint.rb:58:in `block in parse_certificate_authority'
/home/bpaskinc/miq/master/app/models/endpoint.rb:57:in `collect'
/home/bpaskinc/miq/master/app/models/endpoint.rb:57:in `parse_certificate_authority'
/home/bpaskinc/miq/master/app/models/endpoint.rb:23:in `ssl_cert_store'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.2/lib/active_support/core_ext/object/try.rb:17:in `public_send'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.2/lib/active_support/core_ext/object/try.rb:17:in `try!'
/home/bpaskinc/myenv/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/activesupport-5.0.2/lib/active_support/core_ext/object/try.rb:6:in `try'
/home/bpaskinc/miq/master/app/models/manageiq/providers/kubernetes/container_manager_mixin.rb:87:in `ssl_cert_store'
/home/bpaskinc/miq/PLUGINS/manageiq-ui-classic/app/controllers/mixins/ems_common_angular.rb:473:in `get_hostname_from_routes'
/home/bpaskinc/miq/PLUGINS/manageiq-ui-classic/app/controllers/mixins/ems_common_angular.rb:434:in `set_ems_record_vars'
/home/bpaskinc/miq/PLUGINS/manageiq-ui-classic/app/controllers/mixins/ems_common_angular.rb:56:in `update_ems_button_validate'
/home/bpaskinc/miq/PLUGINS/manageiq-ui-classic/app/controllers/mixins/ems_common_angular.rb:105:in `create_ems_button_validate'
/home/bpaskinc/miq/PLUGINS/manageiq-ui-classic/app/controllers/mixins/ems_common_angular.rb:76:in `create'

Comment 20 Beni Paskin-Cherniavsky 2017-04-24 17:33:43 UTC
PR 1120 superceded by https://github.com/ManageIQ/manageiq-ui-classic/pull/1126, merged, waiting for backport.

Comment 21 Pavel Zagalsky 2017-10-29 16:34:33 UTC
Verified