Bug 1212423 - Foreman - SSL verification is broken (cannot be turned off for https URL)
Summary: Foreman - SSL verification is broken (cannot be turned off for https URL)
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: UI - OPS
Version: 5.4.0
Hardware: Unspecified
OS: Unspecified
high
medium
Target Milestone: GA
: 5.4.0
Assignee: Keenan Brock
QA Contact: Jan Krocil
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-04-16 11:48 UTC by Jan Krocil
Modified: 2015-06-16 12:58 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-06-16 12:58:40 UTC
Category: ---
Cloudforms Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:1100 0 normal SHIPPED_LIVE CFME 5.4.0 bug fixes, and enhancement update 2015-06-16 16:28:42 UTC

Description Jan Krocil 2015-04-16 11:48:04 UTC
Description of problem:
I'm not exactly sure how it's supposed to work but I'll list a few scenarios that I consider broken; I'll be talking about the UI form for foreman/sat6 configuration manager setup (my server has selfsigned certs so it should fail verification):

URL: https://my-foreman.example.com (https scheme)
Verify SSL: OFF
Outcome: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed:
^^ BUG (verification is ON even though it shouldn't be)

URL: http://my-foreman.example.com (http scheme)
Verify SSL: ON
Outcome: Credential validation was successful
^^ BUG
(http is off by default and gets redirected to https; we have verification ON
but the setting is ignored so... as long as http is actually _really_ set up and used in the communication, its fine to ignore SSL verification, but if we get redirected, the user should either be informed that HTTP is not supported or the verification should run; just my 2 cents)

URL: my-foreman.example.com (no scheme)
Verify SSL: ON
Outcome: Credential validation was successful
^^ BUG (same as above; this should default to https and therefore fail)

URL:
Verify SSL: ON
Outcome: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed:
^^ OK (this is correct; we are using https and require SSL verification)

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

How reproducible:
Always

Steps to Reproduce:
1. All happens during configuration manager setup

Actual results:
See description.

Expected results:
See description.

Additional info:
-

Comment 1 Jan Krocil 2015-04-16 11:49:15 UTC
Apologies, the last working example should read:

URL: https://my-foreman.example.com (https scheme)
Verify SSL: ON
Outcome: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed:
^^ OK (this is correct; we are using https and require SSL verification)

Comment 3 Keenan Brock 2015-04-20 16:45:29 UTC
Most of these scenarios are at the foreman or ssl transport layer.

verify ssl is switching between SSL_VERIFY_NONE and SSL_VERIFY_PEER.
To connect to a foreman host with a self signed certificate, then we need to tell the ssl layer not to verify the peer certificate.

This flag does not make a statement around what transport is used, just gives us a provision when a user is using self signed certs vs real ones.


I'm not sure what I think about us forcing ssl or changing the default behavior of foreman.

Comment 4 Keenan Brock 2015-04-22 05:44:31 UTC
Would it be better if we changed the field label to "Verify certificate" meaning, verify it is a legit ssl certificate and not a self signed (or even man in the middle)?

Dan, do we have other places in the code that ask the user about using VERIFY_PEER vs VERIFY_NONE? (or do we just go the route of VERIFY_NONE)?

Comment 5 Keenan Brock 2015-04-28 13:48:25 UTC
The desired phrase is "Verify certificate" or "Verify peer certificate"

since the technology behind this is "VERIFY_PEER" vs "VERIFY_NONE", I'd like to put that phrase in there.

Since many users probably don't know about that, certificate is useful phrase.

Comment 7 CFME Bot 2015-04-29 01:25:57 UTC
New commit detected on manageiq/master:
https://github.com/ManageIQ/manageiq/commit/e1d68a06eb959b9e50d24d36100cd6f640ab8b78

commit e1d68a06eb959b9e50d24d36100cd6f640ab8b78
Author:     Aparna Karve <akarve>
AuthorDate: Tue Apr 28 13:33:05 2015 -0700
Commit:     Aparna Karve <akarve>
CommitDate: Tue Apr 28 13:33:21 2015 -0700

    Reword foreman provider verify SSL
    
    Previous wording was not clear
    
    Not letting it be known that this is related to VERIFY_PEER
    and VERIFY_NONE
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1212423

 vmdb/app/views/provider_foreman/_form.html.haml | 2 +-
 vmdb/config/locales/manageiq.pot                | 2 +-
 vmdb/config/model_attributes.rb                 | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comment 8 Jan Krocil 2015-05-05 15:44:06 UTC
Waiting for this to get in:
https://github.com/ManageIQ/manageiq/pull/2826

Comment 9 Jan Krocil 2015-05-07 10:55:41 UTC
The commit mentioned above (GH#2826) did not make it into 5.4.0.0.25. Waiting for the next build but I'll leave it at ON_QA.

Comment 10 Jan Krocil 2015-05-12 11:40:27 UTC
Trying to set a https://foreman.hostname with verification turned off still triggers the following flash message during creds validation:
"SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed:"

The PR mentioned above was supposed to amend this, no?

Comment 11 Keenan Brock 2015-05-13 01:26:30 UTC
Yes, the PR mentioned above was supposed to amend this.
It got the flag recorded correctly.
But looks like it was also not properly passing flag to underlying gem.

You knew there was something wrong in here. Great find.
Will mark modified once PR is merged

https://github.com/ManageIQ/manageiq/pull/2904

Comment 12 CFME Bot 2015-05-13 13:40:58 UTC
New commit detected on manageiq/master:
https://github.com/ManageIQ/manageiq/commit/0127ee62f4ad4412537ea618f0628106768b7ec5

commit 0127ee62f4ad4412537ea618f0628106768b7ec5
Author:     Keenan Brock <kbrock>
AuthorDate: Tue May 12 21:19:59 2015 -0400
Commit:     Keenan Brock <kbrock>
CommitDate: Tue May 12 21:34:56 2015 -0400

    Properly pass verify_ssl into foreman api
    
    apipie-bindings expects :verify_ssl to be in a
    second hash passed to the initializer.
    
    This means we were not able to communicate with a foreman server
    with self signed certificates. (common)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1212423

 lib/manageiq_foreman/lib/manageiq_foreman/connection.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comment 14 CFME Bot 2015-05-14 20:00:34 UTC
New commit detected on cfme/5.4.z:
https://code.engineering.redhat.com/gerrit/gitweb?p=cfme.git;a=commitdiff;h=e537166eae69d065dbc68bcb9e8934de6a973900

commit e537166eae69d065dbc68bcb9e8934de6a973900
Author:     Keenan Brock <kbrock>
AuthorDate: Tue May 12 21:19:59 2015 -0400
Commit:     Keenan Brock <kbrock>
CommitDate: Wed May 13 10:11:15 2015 -0400

    Properly pass verify_ssl into foreman api
    
    apipie-bindings expects :verify_ssl to be in a
    second hash passed to the initializer.
    
    This means we were not able to communicate with a foreman server
    with self signed certificates. (common)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1212423

 lib/manageiq_foreman/lib/manageiq_foreman/connection.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comment 15 Jan Krocil 2015-05-20 11:40:15 UTC
Current state in 5.4.0.1:

(peer certificate verification should fail when verifying in my case because I use a self-signed certificate)

a) no scheme + verify => validation successful (BUG)
b) http:// + verify   => validation successful (BUG)
c) https:// + verify  => validation fail (OK)
d) no scheme + no verify => validation successful (OK)
e) http:// + no verify   => validation successful (OK)
f) https:// + no verify  => validation successful (OK)

When the cert verification is enabled, https should be forced.

If the user specifies just the hostname "my-server.test.com" and enables cert verification, we can't expect him/her to know to include "https" in the URL for it to actually work.

Comment 16 Keenan Brock 2015-05-20 13:07:36 UTC
Well, the code works as designed.
Not saying that it works as it should.

The verify was never intended to force https. It was simply stating that when we are using https, we will verify the ssl certificate returned by the user.

At a future date, if we rework the UI, we may be able to introduce fingerprints and avoid this whole "verify peer ssl"

I'll follow up with some others not as close to the ssl issue and get their take for the meaning of this flag and what they expect

Comment 17 Jan Krocil 2015-05-28 13:41:39 UTC
Verified as fixed in 5.4.0.1.

The behavior described above is expected; works as designed. However, it is due to possibly change in the future to actually "_force_ SSL" instead of just "checking the cert only if https:// is specified". If the behavior remains as is, the UI info should change to match that. (a different BZ; will be linked here once created)

Comment 19 errata-xmlrpc 2015-06-16 12:58:40 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://rhn.redhat.com/errata/RHBA-2015-1100.html


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