Some server-side 'SSL' communication fails to check certificates (use of HTTPSConnection)

[Thierry Carrez]

Grant Murphy from Red Hat reported usage of httplib.HTTPSConnection objects. In Python 2.x those do not perform CA checks so client connections are vulnerable to MiM attacks.

"""
The following files use httplib.HTTPSConnection :
keystone/middleware/s3_token.py
keystone/middleware/ec2_token.py
keystone/common/bufferedhttp.py
vendor/python-keystoneclient-master/keystoneclient/middleware/auth_token.py

AFAICT HTTPSConnection does not validate server certificates and
should be avoided. This is fixed in Python 3, however in 2.X no
validation occurs. I suspect this is also applicable to most OpenStack
modules that make HTTPS client calls.

Similar problems were found in ovirt:
https://bugzilla.redhat.com/show_bug.cgi?id=851672 (CVE-2012-3533)

With solutions for ovirt:
http://gerrit.ovirt.org/#/c/7209/
http://gerrit.ovirt.org/#/c/7249/
"""

[Thierry Carrez (ttx)]

Instances of httplib.HTTPSConnection were also found in Cinder, keystone, nova, quantum and swift for sure.

[Thierry Carrez (ttx)]

Note: at first glance some calls seem to be used for test connections which are likely to be local and/or not contain any useful information. We need to carefully review the various cases before calling this a vulnerability.

[Thierry Carrez (ttx)]

So, for Keystone:
* keystone/common/bufferedhttp.py is apparently used nowhere ?
* keystone/middleware/{s3,ec2}_token.py seem to use HTTPSConnection for a server-to-server request (in most cases connecting to the same host)

I suspect most of the others will be in the same case (servers making HTTPS connections to other local servers) so the MiM risk is limited. To give another data point, most Swift internal server-to-server communications are not even encrypted.

[Dolph Mathews (dolph)]

Regarding keystone.common.bufferedhttp - nothing imports it AFAIK, so I opened a review unattributed to this bug to see the outcome of tests after removing the module https://review.openstack.org/#/c/31973/

[Chuck Thier (cthier)]

So for Swift:

swift.common.bufferedhttp has the capability to instantiate an https connection, but is not used directly in any of the swift code (this is likely due to the original swift client using it before it was pulled out). Internal services (for example from proxy -> object) communicate over plain http on a private network. So unless there is something else that I missed, I don't see a security vulnerability in swift.

[John Dickinson (notmyname)]

Marking as invalid in Swift since it's not used

[Thierry Carrez (ttx)]

For Cinder:

HTTPSConnection is used in Zadara driver to send commands to VPSA controller (if use_ssl = True), as well as in the Solidfire driver to communicate with SolidFire devices.

./cinder/volume/drivers/zadara.py
./cinder/volume/drivers/solidfire.py

The third instance is in unit tests where it's not so much of a problem.

[Thierry Carrez (ttx)]

For Nova:

plugins/xenserver/xenapi/etc/xapi.d/plugins/glance: XenAPI plugin uploading tarballs to Glance servers (if glance_use_ssl)
plugins/xenserver/xenapi/etc/xapi.d/plugins/pluginlib_nova.py: dead code
nova/virt/vmwareapi/read_write_util.py: VMWareAPI driver writing image files to ESX*
nova/api/ec2/__init__.py: redirection to Keystone for EC2-style auth
nova/scheduler/filters/trusted_filter.py: uses CA file through subclass HTTPSClientAuthConnection

(*) Interestingly, reading files uses urllib2.urlopen, which does not do cert verification either :)

[Thierry Carrez (ttx)]

For Quantum:

quantum/plugins/nicira/api_client/client.py: Nicira plugin NVP API client calls
quantum/plugins/nec/common/ofc_client.py: dead code
quantum/plugins/bigswitch/plugin.py: Bigswitch plugin, calls to network controller

[Thierry Carrez (ttx)]

So all the occurences seem to be for serverside node-to-node communication that could be assumed to happen on private networks. That said, all those "use_ssl" give a false sense of security (about the same as communicating unencrypted). The inability to specify a ca_file should serve as a hint that it's not really safe, but that subtlety may be lost on most.

I'm not 100% certain to classify this as an exploitable vulnerability that warrants embargoed disclosure -- we could consider this as missing proper internal encryption features between internal nodes and remind people (through OSSN) to deploy over secure private management networks (as Swift already does). The only thing which makes this a potential vulnerability is that the "use_ssl" parameters where available may induce people into thinking they are safe while they are not...

Thoughts ?

[Jeremy Stanley (fungi)]

SSL/TLS capability likely still needs to remain available for compatibility with third-party systems, where administrators may be deploying canned configurations with all plaintext protocols disabled. I agree the short term fix is to disable any default unauthenticated encryption across the board, surround the config knobs for it with shouty disclaimers and possibly also log warning/info level messages when services are started running in this manner.

Long-term fix is of course to pathologically assume compromised/hostile internal networks, encrypt everywhere and perform peer validation using some pluggable mechanism (Kerberos, private CA, DNSSec+SSHFP, whatever) but I expect that's a very long way off.

[Thierry Carrez (ttx)]

Indeed... The SSL support in there is more to support the case where that other service is configured to only accept SSL client connections... not to really encrypt internal traffic.

I'm leaning towards issuing an OSSN about this, and fix it over time with additional features. Adding Rob Clark to the discussion.

[Dolph Mathews (dolph)]

keystoneclient.middleware.auth_token can be configured to use httplib.HTTPSConnection to communicate back to keystone for token validation, etc.

[Thierry Carrez (ttx)]

@Dolph: now /that/ is a clear vulnerability. Client to server connections should always be properly encrypted. Could you open a separate (private security) bug about that ?

[Dolph Mathews (dolph)]

Well, generally keystoneclient.middleware.auth_token will be deployed on an internal network, so I don't see it as being different from any other service-service communication identified here.

[Thierry Carrez (ttx)]

@Dolph: sorry I misread your comment (client vs. middleware). It's fine to keep on this bug.

[Robert Clark (robert-clark)]

We need to move away from this pervasive assumption that private networks are secure. You only have to look at the 6 different hypervisor vulnerabilities released in the last 2ish months to understand that people are starting to come after virt tech in a big way and a full IaaS stack has a hell of an attack surface.

NSA, HP and a bunch of others all design their networks assuming some hostile actor is already inside and use various methods to try and contain an attacker. If an option to enable SSL is made available to deployers then it should provide most of the protections that you'd expect. Now we all know that SSL is actually quite the pain to configure correctly and perhaps there are areas we can help make life easier for deployers there too.

I vote for keeping this embargoed. OpenStack as-is offers SSL on some connections. Deployers who've turned on SSL for x,y,z connections will have done so to meet assurance requirements they've decided upon for their deployment. If this issue is disclosed without a fix it leaves deployers in a difficult position.

If you believe there's warrant for a wider OSSN on this issue I'd be happy to arrange for one to be drafted.

[Thierry Carrez (ttx)]

> We need to move away from this pervasive assumption that private networks are secure.

I can certainly relate to that. The question is, when does a missing important security feature become a vulnerability ?

Advertising encryption between components while not properly implementing it is definitely a vulnerability in my book (we issued OSSAs for that in the past, and have another in the pipe coming up). In the precise case of this bug it's slightly more blurry: "use_ssl" parameters are more to allow servers to connect to other services that are configured to only accept SSL, than to "encrypt internal communications". I agree that it can still be seen as "advertising encryption while not properly encrypting" though... so I'm OK with embargo/OSSA on this.

The remaining question is how to fix this in stable branches ? We are not supposed to introduce new configuration parameters in stable branches. We also need to not break anyone on upgrade, and there is no good default value for ca_file. Could we piggyback on a Python library that uses the local system cert store ?

[Robert Clark (robert-clark)]

Python libs in general seem pretty dire at using the default store, I think perhaps M2Crypto does, but that's really just wrapping OpenSSL anyway...

[Jeremy Stanley (fungi)]

It's a perennial debate in the Python community, the most recent thread on the python-dev ML being http://mail.python.org/pipermail/python-dev/2013-June/126671.html . There's also an unassigned bug open for the past couple years ( http://bugs.python.org/issue13655 ).

The python-nss module is LGPL and could be leveraged to access the builtin default NSS certdata... the real questions are around maturity and cross-platform support if you're wanting something which can leverage system-level cert databases and platform-specific trustdb management tools.

But by turning on certificate verification in existing deployments you're going to instabreak people who enabled HTTPS to talk to some random third-party device which has an expired, self-signed, CN-mismatched or otherwise nonconformant certificate on it because they saw there was a crypto option and decided to turn the knob all the way up without understanding the benefits (or lack thereof in this case).

[Thierry Carrez (ttx)]

So... It looks like something we can't "fix" as a simple security patch (as it would add a new feature and break on upgrade), but that we should definitely:

* implement as a new feature in an upcoming version (the sooner the better)
* document as unsafe in previous versions (OSSN ?)

Does that work for everyone ?

[Jeremy Stanley (fungi)]

I agree that sounds like the most pragmatic approach. I don't see a reasonable way to "fix" this in stable releases without introducing new dependencies, adding config options, and potentially breaking existing production deployments. An OSSN and documentation improvements describing the actual intent for internal HTTPS support in these components seems like the only way to avoid having this embargoed more or less indefinitely.

Also, once the bug is an open/public hardening feature request it becomes much easier to pool the developer community to collectively solve instances of this much more quickly.

[Thierry Carrez (ttx)]

Kurt, Robert, would that work for you ?

[Robert Clark (robert-clark)]

Works for me, throw it on the OSSN queue.

[Thierry Carrez (ttx)]

Ok, so we should:
* implement as a new feature in an upcoming version (the sooner the better)
* document as unsafe in previous versions (OSSN)

Any taker to work on that ?

[Jamie Lennox (jamielennox)]

I've got a patch or two related to this waiting already

[Robert Clark (robert-clark)]

Some SSL-Enabled connections fail to perform basic certificate checks
----

### Summary ###
In many places OpenStack components use Python 2.x HTTPSConnection to establish an SSL connection between endpoints. This does not provide many of the assurances one would expect when using SSL and leaves connections open to potential man-in-the-middle attacks

### Affected Services / Software ###
keystone/middleware/s3_token.py
keystone/middleware/ec2_token.py
keystone/common/bufferedhttp.py
vendor/python-keystoneclient-master/keystoneclient/middleware/auth_token.py
<<<<OTHERS NEED TO BE ADDED HERE>>>>>

### Discussion ###
A secure SSL session relies on validation of a X.509 certificate. Basic checks include:
* Is the certificate signed by a CA I recognize
* Has the CA revoked this certificate
* Does the common name on the certificate match the server I'm trying to reach

The HTTPSConnection class is used in a large number of locations and fails to check that certificates are signed by a valid authority. Without that check in place, the following checks (some highlighted above) are largely invalid.

The result is that an attacker who has access to the network traffic between two endpoints relying on HTTPSConnection can trivially create a certificate that will be accepted by HTTPSConnection as valid - allowing the attacker to intercept, read and modify traffic that should be encrypted by SSL.

### Recommended Actions ###
<<<< MORE INVESTIGATION REQUIRED here on short-long term options >>>>

### Contacts / References ###
This OSSN : https://bugs.launchpad.net/ossn/+bug/1188189
OpenStack Security ML : openstack-security at lists.openstack.org
OpenStack Security Group : https://launchpad.net/~openstack-ossg

[Dolph Mathews (dolph)]

Jamie- can you provide a link to your patch/review here?

[Jamie Lennox (jamielennox)]

It appears that the new Partial-Bug syntax doesn't do an automatic link anymore.

python-keystoneclient: Replace HttpConnection in auth_token with Requests
https://review.openstack.org/#/c/34161/

[Jeremy Stanley (fungi)]

It only adds the review link on patchset #1 (to avoid spamming bug subscribers with comments). You didn't mention this bug in your commit message until patchset #3 of that review.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/34161
Committed: http://github.com/openstack/python-keystoneclient/commit/20e166fd8a943ee3f91ba362a47e9c14c7cc5f4c
Submitter: Jenkins
Branch: master

commit 20e166fd8a943ee3f91ba362a47e9c14c7cc5f4c
Author: Jamie Lennox <email address hidden>
Date: Mon Aug 12 13:12:27 2013 +1000

    Replace HttpConnection in auth_token with Requests

    Requests is becoming the standard way of doing http communication, it
    also vastly simplifies adding other authentication mechanisms. Use it in
    the auth_token middleware.

    This adds the ability to specify a CA file that will be used to verify a
    HTTPS connections or insecure to specifically ignore HTTPS validation.

    SecurityImpact
    DocImpact
    Partial-Bug: #1188189
    Change-Id: Iae94329e7abd105bf95224d28f39f4b746b9eb70

[Robert Clark (robert-clark)]

Some SSL-Enabled connections fail to perform basic certificate checks
----

### Summary ###
In many places OpenStack components use Python 2.x HTTPSConnection to establish an SSL connection between endpoints. This does not provide many of the assurances one would expect when using SSL and leaves connections open to potential man-in-the-middle attacks

### Affected Services / Software ###
keystone/middleware/s3_token.py
keystone/middleware/ec2_token.py
keystone/common/bufferedhttp.py
vendor/python-keystoneclient-master/keystoneclient/middleware/auth_token.py

### Discussion ###
A secure SSL session relies on validation of a X.509 certificate. Basic checks include:
* Is the certificate signed by a CA I recognize
* Has the CA revoked this certificate
* Does the common name on the certificate match the server I'm trying to reach

The HTTPSConnection class is used in a large number of locations and fails to check that certificates are signed by a valid authority. Without that check in place, the following checks (some highlighted above) are largely invalid.

The result is that an attacker who has access to the network traffic between two endpoints relying on HTTPSConnection can trivially create a certificate that will be accepted by HTTPSConnection as valid - allowing the attacker to intercept, read and modify traffic that should be encrypted by SSL.

### Recommended Actions ###
Consider using an up to date version of the keystone client http://github.com/openstack/python-keystoneclient/commit/20e166fd8a943ee3f91ba362a47e9c14c7cc5f4c

### Contacts / References ###
This OSSN : https://bugs.launchpad.net/ossn/+bug/1188189
OpenStack Security ML : openstack-security at lists.openstack.org
OpenStack Security Group : https://launchpad.net/~openstack-ossg

[Robert Clark (robert-clark)]

Some SSL-Enabled connections fail to perform basic certificate checks
----

### Summary ###
In many places OpenStack components use Python 2.x HTTPSConnection to establish an SSL connection between endpoints. This does not provide many of the assurances one would expect when using SSL and leaves connections open to potential man-in-the-middle attacks

### Affected Services / Software ###
keystone/middleware/s3_token.py
keystone/middleware/ec2_token.py
keystone/common/bufferedhttp.py
vendor/python-keystoneclient-master/keystoneclient/middleware/auth_token.py

### Discussion ###
A secure SSL session relies on validation of a X.509 certificate. Basic checks include:
* Is the certificate signed by a CA I recognize
* Has the CA revoked this certificate
* Does the common name on the certificate match the server I'm trying to reach

The HTTPSConnection class is used in a large number of locations and fails to check that certificates are signed by a valid authority. Without that check in place, the following checks (some highlighted above) are largely invalid.

The result is that an attacker who has access to the network traffic between two endpoints relying on HTTPSConnection can trivially create a certificate that will be accepted by HTTPSConnection as valid - allowing the attacker to intercept, read and modify traffic that should be encrypted by SSL.

### Recommended Actions ###
Some projects have updated their code to be more secure, others have not. The OSSG suggest cloud deployers check the status of bug https://bugs.launchpad.net/ossn/+bug/1188189 to see if the projects they require have updated.

### Contacts / References ###
This OSSN : https://bugs.launchpad.net/ossn/+bug/1188189
OpenStack Security ML : openstack-security at lists.openstack.org
OpenStack Security Group : https://launchpad.net/~openstack-ossg

[Robert Clark (robert-clark)]

Posted to OpenStack ML 19-9-13

[Daniel Gollub (d-gollub)]

cinder: Replace HTTPSConnection in solidfire driver
https://review.openstack.org/#/c/75647/

[Daniel Gollub (d-gollub)]

cinder: Replace httplib.HTTPSConnection in unittests
https://review.openstack.org/#/c/75667/

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/76476

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/76987

[Daniel Gollub (d-gollub)]

For the record:

 * quantum/plugins/bigswitch/plugin.py: Bigswitch plugin, calls to network controller

this one has an ongoing blueprint bsn-certificate-enforcement

https://review.openstack.org/#/c/70906/

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/77346

[John Griffith (john-griffith)]

So Daniel I have to say AWESOME job picking this up and running with it. I'm working on figuring out how to do this without breaking existing setups; although maybe somebody already has an idea off the top of their head.

I did want to clarify though, I think we need to make sure that everywhere this is proposed it doesn't break existing setups or upgrades. I'm not sure I was clear about that in the one version but it's going to be a critical piece of this. If we can automate the switch (which I think and hope we can) then at the very least we're going to need a very BOLD and EXPLICIT warning and instructions in the Release Notes and in the documents on how to keep from losing your cloud.

[Daniel Gollub (d-gollub)]

Random idea:
Maybe this is something which should be discussed/handled by the individual OpenStack distributors/vendors?
They could handle this inside their packages of cinder (or other components)? With pre-/post-scripts while upgrading the package?

All other automatic approaches were it disables SSL verification - or tolerates failing verification would let us end-up with the same security issue again - I guess.

I do not have a better idea either. But maybe we could use the Icehouse release and stress this fundamental change in Upgrade notes/instruction. And switch to SSL verification by default across all the identified OpenStack components.

Introducing some logic like: if the configured HTTPS URL holds an IP address instead of a DNS name, "very likely" SSL verification is not going to succeed so the code falls back to ssl_insecure=True .... or something like that - I would not recommend to do. Since it would silently hide the fact that the current setup is not as secure as one would think.

Not quite sure if just logging that SSL verification failed and continuing with ssl_insecure=True would "solve" it either in a correct fashion.

Maybe security- and release folks should decide to go once through this burden with the Icehouse release or not. And try to warn the admins ahead in Release Notes and Upgrade Instruction to verify if their setup would pass SSL verification - or not.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/77414

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/75667
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=0f9652d92e175a1f7dc3c2a37ab444b8f189375a
Submitter: Jenkins
Branch: master

commit 0f9652d92e175a1f7dc3c2a37ab444b8f189375a
Author: Daniel Gollub <email address hidden>
Date: Sun Feb 23 09:30:00 2014 +0100

    Replace httplib.HTTPSConnection in unittests

    SSL Verification is from now on enabled by default for the
    TestOpenStackClient. So far no unittests was making actively use of
    httplib.HTTPSConnection.

    Intention is to reduce noise of audits/scanners which look for Python 2.x
    httplib.HTTPSConnection missing SSL verification. By completely abandoning the use
    of httplib.HTTPSConnection.

    Change-Id: Ic0352cf453d5c41f09084a6d68b3393b8ddda84a
    Partial-Bug: 1188189

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/70906
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=7255e056092f034daaeb4246a812900645d46911
Submitter: Jenkins
Branch: master

commit 7255e056092f034daaeb4246a812900645d46911
Author: Kevin Benton <email address hidden>
Date: Sun Feb 2 20:46:18 2014 -0800

    BigSwitch: Add SSL Certificate Validation

    This patch adds the option to use SSL certificate
    validation on the backend controller using SSH-style
    sticky authentication, individual trusted
    certificates, and/or certificate authorities.
    Also adds caching of connections to deal with
    increased overhead of TLS/SSL handshake.

    Default is now sticky-style enforcement.

    Partial-Bug: 1188189
    Implements: blueprint bsn-certificate-enforcement
    Change-Id: If0bab196495c4944a53e0e394c956cca36269883

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/76476
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=5bd4c2984d329625a2a8442b316fa235dbb88a3d
Submitter: Jenkins
Branch: master

commit 5bd4c2984d329625a2a8442b316fa235dbb88a3d
Author: Daniel Gollub <email address hidden>
Date: Wed Feb 26 06:56:13 2014 +0100

    Replace httplib.HTTPSConnection in ec2_token

    httplib.HTTPSConnection is known to not verify SSL certificates in Python 2.x.
    Implementation got adapted to make use of the requests module instead.

    SSL Verification is from now on enabled by default.

    Can be disabled via an additional introduced configuration option:

    `keystone_ec2_insecure=True`

    SecurityImpact
    DocImpact
    Partial-Bug: 1188189

    Change-Id: Ie6a6620685995add56f38dc34c9a0a733558146a

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/77414
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=264b4a2523c165640f17aa4837f87ddfd0b49640
Submitter: Jenkins
Branch: master

commit 264b4a2523c165640f17aa4837f87ddfd0b49640
Author: Daniel Gollub <email address hidden>
Date: Sun Mar 2 09:33:38 2014 +0100

    Replace HTTPSConnection in NEC plugin

    Replace HTTPSConnection in NEC plugin PFC driver with Requests.

    SSL Verification is from now on enabled by default.

    This changes the default behaviour and is the primary intention of this
    change: verify SSL certificates.

    This might break existing configuration/setups where the SSL certificate
    used by the NEC PFC driver would not pass the verification.

    SecurityImpact
    DocImpact
    Partial-Bug: 1188189

    Change-Id: I1e5fdc9c2ed5b812aa6509d1639bd499acc5c337

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: milestone-proposed
Review: https://review.openstack.org/85470

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/85470
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=8fd51240983f4c2ff2a73d907ccb1c597bd5d835
Submitter: Jenkins
Branch: milestone-proposed

commit 8fd51240983f4c2ff2a73d907ccb1c597bd5d835
Author: Daniel Gollub <email address hidden>
Date: Sun Mar 2 09:33:38 2014 +0100

    Replace HTTPSConnection in NEC plugin

    Replace HTTPSConnection in NEC plugin PFC driver with Requests.

    SSL Verification is from now on enabled by default.

    This changes the default behaviour and is the primary intention of this
    change: verify SSL certificates.

    This might break existing configuration/setups where the SSL certificate
    used by the NEC PFC driver would not pass the verification.

    SecurityImpact
    DocImpact
    Partial-Bug: 1188189

    Change-Id: I1e5fdc9c2ed5b812aa6509d1639bd499acc5c337
    (cherry picked from commit 264b4a2523c165640f17aa4837f87ddfd0b49640)

[OpenStack Infra (hudson-openstack)]

Change abandoned by Duncan Thomas (<email address hidden>) on branch: master
Review: https://review.openstack.org/77346
Reason: Abandoning change: No update for more than 14 days

[Sean Dague (sdague)]

This is definitely still in nova in a few places. I kind of thing this should be an RC bug to actually clean up the rest of this.

[Nathanael Burton (mathrock)]

Sean, I agree. Here's one that's almost through gerrit after *only* seven months of review for a tiny change.

https://bugs.launchpad.net/nova/+bug/1279381

https://review.openstack.org/#/c/72974/

[Michael Still (mikal)]

So, for this to remain RC for nova I need a list of patches that need review. If we don't have that basically now, then we're going to need to drop this from rc1.

(That doesn't mean its important, just that I don't think we can hold rc1 waiting for it).

[Sean Dague (sdague)]

The generic version of this isn't getting fixed in Nova, so openning the following bugs:

https://bugs.launchpad.net/nova/+bug/1373992
https://bugs.launchpad.net/nova/+bug/1373993
https://bugs.launchpad.net/nova/+bug/1374000
https://bugs.launchpad.net/nova/+bug/1374001

For the specific instances where we need to fix this

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/124104

[Sean Dague (sdague)]

Nova bugs will be tracked in the separate bugs listed above, so removing Nova from this bug.

[haruka tanizawa (h-tanizawa)]

Is there any in progress work for neutron ?

[Jeremy Stanley (fungi)]

Now that the solidfire driver has switched from httplib to requests in the current development cycle (see https://review.openstack.org/127399 for details) would it be possible to switch certificate validation on for it prior to the Kilo release, or do we need to wait for Liberty?

[John Griffith (john-griffith)]

@Jeremy
I'll look at making the switch, frankly the overwhelming response I had received from the field was that is was a "don't care", that being said it's certainly a supported capability that I can adjust. Even if I make it an option that for now defaults to False I think it at least addresses the heart of the matter here.

[Robert Clark (robert-clark)]

It's sad that we can't have something more secure by default.

[Jeremy Stanley (fungi)]

While I agree that secure defaults are preferable, even if only to provide a good example to operators and prove that we as a project are doing all we can to keep them and their users safe, from what I've seen in the field most will be disinterested in running a CA or purchasing and tracking certificates for the various bits of hardware to which these drivers are communicating.

The additional complexity and liability of spontaneous breakage from certificate expiration often outweighs concerns over attacks by malicious entities infiltrating an isolated management network in ways which internal server-to-server and driver-to-hardware HTTPS might mitigate.

I appreciate the degree to which our developers already make security a priority in our software, and recognize that sometimes user demands for other fixes and features pragmatically outweigh some particular security improvements in the absence of specific developers focused on driving and implementing the latter.

[Sean McGinnis (sean-mcginnis)]

No longer any instances of this in cinder.

[Xing Yang (xing-yang)]

In Cinder, the following drivers are using six.moves.http_client, which on python 2.7 I believe calls httplib:

   Location: cinder/cinder/volume/drivers/blockbridge.py:149
   Location: cinder/cinder/volume/drivers/cloudbyte/cloudbyte.py:116
   Location: cinder/cinder/volume/drivers/prophetstor/dplcommon.py:102
   Location: cinder/cinder/volume/drivers/prophetstor/dplcommon.py:124
   Location: cinder/cinder/volume/drivers/qnap.py:814
   Location: cinder/cinder/volume/drivers/zadara.py:238

See reference of six.moves.http_client calling httplib here: https://pythonhosted.org/six/

Re-opening the bug.

[Chris Suttles (killface007)]

Current status:

find cinder/cinder/volume/drivers -name '*.py' | while read file ; do echo "$file" ; nl $file | grep http_client ; done

cinder/cinder/volume/drivers/blockbridge.py
    24 from six.moves import http_client
   128 connection = http_client.HTTPSConnection(cfg['host'], cfg['port'])
cinder/cinder/volume/drivers/dell_emc/vmax/https.py
    28 from six.moves import http_client
    63 Supplies an additional 'makefile' method which http_client requires
    74 class HTTPSConnection(http_client.HTTPSConnection):
    86 http_client.HTTPSConnection.__init__(self, host, port,
   222 response in XML. Uses Python's build-in http_client. x509 may be a
   282 except http_client.BadStatusLine as arg:
cinder/cinder/volume/drivers/falconstor/rest_proxy.py
    23 from six.moves import http_client
   821 connection = http_client.HTTPConnection(self.hostip, 80, timeout=60)
cinder/cinder/volume/drivers/prophetstor/dplcommon.py
    31 from six.moves import http_client
    90 connection = http_client.HTTPSConnection(self.ip,
   108 except http_client.CannotSendRequest as e:
   111 connection = http_client.HTTPSConnection(self.ip,
   131 if response.status == http_client.SERVICE_UNAVAILABLE:
   140 except http_client.ResponseNotReady as e:
   151 and response.status == http_client.NOT_FOUND):
   158 'response': http_client.responses[response.status],
   160 if response.status == http_client.UNAUTHORIZED:
   164 elif retcode == 0 and response.status is http_client.NOT_FOUND:
   166 elif retcode == 0 and response.status is http_client.ACCEPTED:
   180 response.status in [http_client.OK, http_client.CREATED] and
   181 http_client.NO_CONTENT not in expected_status):
   211 [http_client.OK, http_client.ACCEPTED])
   233 [http_client.OK, http_client.ACCEPTED,
   234 http_client.CREATED])
   253 [http_client.OK, http_client.ACCEPTED,
   254 http_client.CREATED])
   264 [http_client.OK, http_client.ACCEPTED,
   265 http_client.NOT_FOUND, http_client.NO_CONTENT])
   290 [http_client.OK, http_client.ACCEPTED,
   291 http_client.CREATED])
   307 [http_client.OK, http_client.ACCEPTED,
   308 http_client.CREATED])
   312 return self._execute(method, url, None, [http_client.OK])
   317 [http_client.OK, http_client.ACCEPTED])
   341 [http_client.OK, http_client.CREATED,
   342 http_client.ACCEPTED])
   361 [http_client.OK, http_client.CREATED,
   362 http_client.ACCEPTED])
   368 [http_client.OK, http_client.ACCEPTED,
   369 ...

[Chris Suttles (killface007)]

Eek, I didn't realize that would be such a big wall of text.

It looks like this has been stagnant for a while and the requirements are pretty simple, so I am going to reassign this to myself and start working through the drivers that are using six.moves.http_client and migrating them to requests instead.

[Chris Suttles (killface007)]

I'm sorry to bail on this after grabbing it @d-gollub. I spent some time on this, but got stuck on updating tests. I reached out in a couple related tickets but got no replies. I'd hand this back but I don't have permission to do so.

[Ibad Khan (ik-ibadkhan)]

It's been time since @killface007 had updated this.

Right now only QNAP Storage drivers use six.moves.http_client's HTTPSConnection.

cinder/volume/drivers/qnap.py
    34 from six.moves import http_client
   979 connection = http_client.HTTPSConnection(management_ip,
   983 connection = http_client.HTTPSConnection(management_ip,
   987 http_client.HTTPConnection(management_ip, management_port))
  1006 connection = http_client.HTTPSConnection(nas_ip,
  1010 connection = http_client.HTTPSConnection(
  1013 connection = http_client.HTTPConnection(nas_ip, self.port)

Rest of the files refer only HTTP codes, which I presume is safe.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/538237

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/538237
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=431b4284bf12dfd8f97af95a9b96356105e08404
Submitter: Zuul
Branch: master

commit 431b4284bf12dfd8f97af95a9b96356105e08404
Author: Ibadulla Khan <email address hidden>
Date: Fri Jan 26 19:08:35 2018 +0530

    QNAP Drivers - Move from httplib to requests

    Use driver_ssl_cert_verify under backend section to
    enable or disable SSL verfication.

    NOTE: IPv6 isn't supported by QNAP driver.

    Change-Id: Iba886fd0bd401052a444eb7a4427607e693d7c81
    Closes-Bug: 1658766
    Partial-Bug: 1188189