MITM vulnerability with EMC VMAX driver

Bug #1372635 reported by Matthew Edmonds
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
High
Xing Yang
Icehouse
Won't Fix
Undecided
Unassigned
Juno
Won't Fix
Undecided
Unassigned
OpenStack Security Advisory
Won't Fix
Undecided
Unassigned

Bug Description

The EMC VMAX driver in Juno appears to blindly trust whatever certificate it gets back from the device without any validation (it does not specify the ca_certs parameter, etc. on WBEMConnection.__init__). This would leave it open to a MITM attack.

Changed in ossa:
status: New → Incomplete
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Thank you for the report! The OSSA task is set to incomplete pending for additional review.

Just to be sure, only Juno is affected ?

In stable/icehouse, cinder/volume/drivers/emc/emc_smis_common.py is also using WBEMConnection without ca_certs parameters...

Revision history for this message
Matthew Edmonds (edmondsw) wrote :

The driver I was looking at (emc_vmax_common.py) is new for Juno, but it looks like it was also an issue in the one you mentioned in icehouse.

Revision history for this message
Matthew Edmonds (edmondsw) wrote :

I've looked into the code some more. WBEMConnection.__init__()'s ca_certs and no_verification parameters are not specified by emc_vmax_common.py. The defaults for these parameters are None and False, respectively, which should mean that verification will be done if the "default system path" for certificates is present. But pywbem skips validation if none of these paths exist:

                '/etc/pki/ca-trust/extracted/openssl/ca-bundle.trust.crt',
                '/etc/ssl/certs',
                '/etc/ssl/certificates'

That seems to be what I've been hitting. That's really more of a pywbem bug than anything... when told to verify certificates they should fail verification if they can't find a store to verify against.

The emc_vmax_common.py driver should be enhanced to allow the OpenStack deployer to specify whether they want to verify certificates (no_verification) and where they are going to store certificates (ca_certs). And it can prevent the above-mentioned pywbem problem from coming into play by checking for the existence of those 3 directories itself if ca_certs is None and no_verification=False and failing if none of those directories exist.

Jay Bryant (jsbryant)
Changed in cinder:
assignee: nobody → Xing Yang (xing-yang)
milestone: none → juno-rc1
Revision history for this message
John Griffith (john-griffith) wrote :

We've had the question of cert checking raised in the past, particularly with respect to httplib and requests and at that time made the decision that these were items of concern however didn't warrant security advisements.

There are a number of drivers that don't use SSL certs (in fact their web services don't implement them IIRC) and only use login/password. I'm not sure what EMC's stance is on this, but I still am unsure if I believe this should be a Security Issue or not.

Revision history for this message
Xing Yang (xing-yang) wrote :

We've done some investigation and found that we are using non-HTTPS mode currently to communicate with ECOM. There's an option that allows us to use HTTPS mode. We want to support this for client side validation.

We are still investigating on the server side validation. We'd like to fix it once a solution is found.

Revision history for this message
Matthew Edmonds (edmondsw) wrote :

I can't imagine how failing to use a secure protocol or properly validate certificates could not be considered security issues... Thanks for fixing, xing-yang.

Revision history for this message
Jay Bryant (jsbryant) wrote :

Given John's response this is not a blocker for Juno release. Removing the RC1 target.

Changed in cinder:
milestone: juno-rc1 → next
Revision history for this message
John Griffith (john-griffith) wrote :

sure, it can easily be considered a security issue, and depending on who you ask so is using iSCSI in the first place, and so is having unencrypted passwords in conf files. All I was getting at was that some devices may not even be supporting certs. I'm mostly opposed to blocking Juno for this which I should've been more clear about.

I'm also not exactly sure what category this should fall under.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Much of OpenStack's internal server-side communication needs securing, and while the VMT has previously deemed it not an explicit vulnerability for which we would issue an advisory, any efforts toward securing internal communications in a consistent fashion are wholeheartedly encouraged.

I propose this bug be switched to public, tagged as security (hardening), and possibly marked as a duplicate of bug 1188189.

Revision history for this message
John Griffith (john-griffith) wrote :

Thanks Jeremy for putting in to coherent and well stated text what I was trying to get at :)

+1

Revision history for this message
Matthew Edmonds (edmondsw) wrote :

This response concerns me. Not encrypting network communication is a good way to get fired, or get your product kicked out of a datacenter, but we're going to say that even supporting encrypted communication is hardening?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Well, we've already said it, quite some time ago. The options are to stop shipping OpenStack or to convince developers to help fix it. Whether or not we issue security advisories is orthogonal, but it boils down to being a fairly obvious issue which needs addressing in many, many, many places. If we were to decide to issue advisories for all of them, 1. I seriously doubt it would improve the current situation at all, and 2. we'd be up to our eyeballs in repetitive advisories. Declaring this a vulnerability doesn't magically make developers decide to fix it. In fact, keeping it under a secret embargo instead of making it a public bug means there are far fewer opportunities for developers to see and fix this.

Revision history for this message
Matthew Edmonds (edmondsw) wrote :

If the decision has been made that this particular kind of issue is hardening, and made publicly by the looks of bug 1188189, then it's been made.

I'm really less concerned with whether this gets marked public or has a security advisory issued, as with making sure this is fixed before juno does go out the door. The EMC VMAX driver is new in Juno. Shouldn't we put extra emphasis on not adding to the problem new security issues that didn't exist in the previous release? If we don't start clamping down, the problem is just going to get worse.

Obviously none of us thinks we should stop shipping OpenStack but neither can we let things go... If there are things we can't address in Juno at this point, we need to put plans in place to make sure they're addressed in Kilo. As you say, we need to convince developers to help fix it / stop breaking it. Maybe this needs to be a focus item in Paris. How do we get the various projects more security-focused? How can we get more developers fixing things without making everything public? Etc. If we can figure out how to share the load better...

Revision history for this message
Jay Bryant (jsbryant) wrote :

Matt, let me make a suggestion here. We are all aware that there are weaknesses in security here, as is mentioned above. Unfortunately there is often a great focus on putting new features in place and fixing obvious bugs in functionality over addressing these weaknesses. As a result, it can be harder to get resource to focus on these weaknesses.

You clearly have a passion for these sorts of issues, you are finding them and wish to see them fixed. In Kilo we are going to be focusing, in Cinder, on cleaning up and improving base Cinder. This would be a good opportunity to start fixing these issues. If you want to start building up a list of areas that security could be improved in Cinder we would more than welcome any patches you wish to submit to address those issues.

Revision history for this message
Jeremy Stanley (fungi) wrote :

It's worth elaborating on how the VMT drew its fuzzy line on what constitutes a security vulnerability for lack of encryption or lack of host key/certificate verification:

1. OpenStack is currently assumed to be deployed with a trusted and externally-secured management network for control-plane communication between its server components. The existing documentation strongly recommends this deployment model. As such, failure to secure this internal communication is necessarily a security vulnerability for an OpenStack deployment which follows those recommendations. This is somewhere we, as a project, can do a better job. If in the (hopefully near!) future we solve these shortcomings, any other discovered bugs in this vein would probably begin to be treated as vulnerabilities and would get security advisories, but only after documentation asserted that it was safe to connect internal OpenStack components over untrusted networks.

2. Any client-to-server communication (administrator access, end user access) which is not secured by encrypted connections and validating its endpoints is absolutely considered a security vulnerability and we have issued advisories for these bugs in the past.

3. Anything in between those other two is sort of a gray area, and we make a human judgment call as to how to handle it.

Hopefully that helps explain the landscape a little more clearly.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Gah, I should proofread a little better. #1 was supposed to say "...failure to secure this internal communication is NOT necessarily a security vulnerability..."

Revision history for this message
Jeremy Stanley (fungi) wrote :

Having seen no other objection/counterpoint to switching this bug to security hardening, I propose we do so on Thursday.

Thierry Carrez (ttx)
information type: Private Security → Public
tags: added: security
Changed in ossa:
status: Incomplete → Won't Fix
Revision history for this message
Robert Clark (robert-clark) wrote :

This is how software projects get a bad reputation. I agree with everything that Matt has said (which won't be a surprise to the VMT). Deviation from best practice / security failures should be a blocker to new features/drivers being accepted into the project.

Revision history for this message
Jeremy Stanley (fungi) wrote :

I don't necessarily disagree with this stance, but from a pragmatic perspective the VMT lacks any real authority to put all OpenStack development on lockdown and hold developers hostage so that they're forced to redesign internal communication between components with more secure mechanisms rather than work on their various pet features. As much as I wish we could...

Revision history for this message
Robert Clark (robert-clark) wrote :

Is this concern something that could be bubbled up to the technical committee?

Revision history for this message
Jeremy Stanley (fungi) wrote :

It's more likely something that needs to be bubbled out to the PTLs managing the projects in question. While the TC has some sway over cross-project matters, design choices are generally the purview of individual programs. (TC members: is that an accurate statement?)

Revision history for this message
Thierry Carrez (ttx) wrote :

@Rob: unfortunately we singled that design flaw a long time ago (you were still on the VMT then) and nothing was done to fix it. In some cases it's oversight (like in this driver), in some others it's an architectural choice (like in Swift using unencrypted rsync on the management network). Unless a group takes on the task to fix it throughout OpenStack (and folows up with patches and hangs there until they get approved and merged), I don't see any progress coming.

Do you think the OSSG could form a workgroup around that ? We already have one proposed to eradicate XSS from Horizon... we really need one taking on absence of proper encryption on the management network side.

Revision history for this message
Monty Taylor (mordred) wrote :

I support fixing management network encryption.

Revision history for this message
Jay Bryant (jsbryant) wrote :

Xing,

Any update on this? It would be nice to get this fixed in master so we can at least cherry-pick it back to icehouse and juno. Let me know what you think and if you have any better idea when you might have a fix.

Thanks!

Changed in cinder:
status: New → Triaged
importance: Undecided → High
milestone: next → kilo-1
tags: added: drivers emc
Revision history for this message
Xing Yang (xing-yang) wrote :

Jay,

The code changes required in the driver is minimum:

We just need to use the two new parameters "ca_cert" and "no_verification" in the new Connection API:
    conn = pywbem.WBEMConnection(url,
                                creds,
                                default_namespace=namespace,
                                x509=None,
                                verify_callback=None,
                                ca_certs=’/etc/cinder/ca_certs/dsib2202.lss.emc.com.crt’,
                                no_verification=False)

However, we encountered problems when testing this. The pywbem library packaged with Ubuntu 12.04 and 14.04 is 0.7.0, which was released in 12/12/2008. It doesn't support "ca_certs" and "no_verification". Version 0.8.0 is still under development.

There are newer RPM packages that have these parameters and we tried to convert the RPM package but couldn't get it to work on Ubuntu. Until Ubuntu has the newer version of the pywbem library, we can't make these changes that don't work on Ubuntu.

Let me know if you have other suggestions. Thanks.

Revision history for this message
Jay Bryant (jsbryant) wrote :

Thanks for the update Xing.

Revision history for this message
ANish (anish2good) wrote :

should we proceed with the communication, if there is any verify error, in this scenario

Revision history for this message
Xing Yang (xing-yang) wrote :

Sorry, but we can't provide a fix that doesn't work on Ubuntu.

Revision history for this message
Matt Riedemann (mriedem) wrote :

Per comment 25, has anyone reported a bug against Ubuntu for packaging of pywbem?

Otherwise, we could put some conditional logic in the code that checks the signature of the pywbem library and if the args are available use them, if not don't - or try to use them and catch an exception and handle it gracefully with a note about why we have to do the conditional checks.

Revision history for this message
Xing Yang (xing-yang) wrote :
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Triaged → In Progress
Revision history for this message
Matt Riedemann (mriedem) wrote :

Why hasn't anyone reported a bug against pywbem yet?

http://sourceforge.net/p/pywbem/bugs/?source=navbar

Why are we even supporting this library, it's not even global-requirements:

https://github.com/openstack/requirements/blob/master/global-requirements.txt

Revision history for this message
John Griffith (john-griffith) wrote :

@Matt
Excellent questions...

Revision history for this message
Mark Brown (mstevenbrown) wrote :

Hmm. I may be delinquent in noting this here, that's on me; but an attempt to address this is being made in https://bugs.launchpad.net/ubuntu/+source/pywbem/+bug/1385469

Revision history for this message
Mark Brown (mstevenbrown) wrote :

Closed that too early. I think the reason there is no bug agains pywbem is that this is already fixed in their dev branch, but not released yet as a stable version.

Mike Perez (thingee)
Changed in cinder:
milestone: kilo-1 → kilo-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by Mike Perez (<email address hidden>) on branch: master
Review: https://review.openstack.org/130923
Reason: No activity.

Mike Perez (thingee)
Changed in cinder:
milestone: kilo-2 → kilo-3
Revision history for this message
Xing Yang (xing-yang) wrote :

Fixed as part of Kilo driver update: https://review.openstack.org/#/c/141729/

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in cinder:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: kilo-3 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.