Open Stack

On Nov 22, 2014, at 5:01 PM, Jeremy Stanley <fungi at yuggoth.org> wrote:

> On 2014-11-22 19:45:09 +1300 (+1300), Robert Collins wrote:
>> Given the persistent risks of downgrade attacks, I think this does
>> actually qualify as a security issue: not that its breaking, but
>> that SSLv3 is advertised and accepted anywhere.
> 
> Which downgrade attacks? Outside of Web browser authors deciding it
> was a good idea to bypass the normal TLS negotiation mechanism, as
> long as both ends _support_ TLS then causing a downgrade within TLS
> version negotiation to SSLv3 or earlier should not be possible. If
> you're suggesting we strengthen against unknown future attacks,
> that's a fine idea and is something we call "security hardening"
> (not a vulnerability fix).
> 
>> The lines two lower:
>> 
>>    try:
>>        _SSL_PROTOCOLS["sslv2"] = ssl.PROTOCOL_SSLv2
>>    except AttributeError:
>>        pass
>> 
>> Are even more concerning!
> 
> _SSL_PROTOCOLS is only used in sslutils.validate_ssl_version() which
> is in turn used by a method in rpc.impl_kombu. Checking *all*
> current branches of *all* official OpenStack projects in our Gerrit,
> the only way it's called is when the kombu RPC backend is in use and
> kombu_ssl_version is set in a configuration file. It will *allow*
> explicit selection of insecure SSL versions (SSLv3 and SSLv2) by the
> administrator--this isn't a "magically uses insecure protocols
> without telling you" situation--merely providing the option to
> configure use of a specific insecure protocol. (You can also
> configure it not to use any encryption at all for that matter.)
> 
> I'm all for dropping this nonsense completely in master and also
> backporting a patch to make this not spontaneously vomit when run on
> platforms where SSLv3 is no longer available (perhaps something
> similar to the SSLv2 try/except example above), but we shouldn't
> backport a patch which suddenly breaks someone's cloud because they
> made a conscious decision to configure it to use SSLv3 for RPC
> communication. Visibly documenting (in the Security Guide or an
> OSSN) that you should configure your RPC communication to use TLS
> instead of SSLv2/3 is of course a great idea too.

It appears the option isn’t required, and that the default does what we would want as far as negotiating the best possible protocol. The only place things will be breaking is on the version of Python shipped by Debian where the constant used to set up the validation logic is no longer present in the SSL library. Let’s start by making the smallest change we can to fix that problem, and then move on.

As has been proposed, we can fix the Debian Python 2.7 issue by treating sslv3 as sslv2 is handled in the code snippet above. That’s an easy patch for us to land, and I hope Thomas will update the patch he has already submitted based on feedback on that review.

After the short-term fix is merged, we can investigate deprecating the configuration option entirely. That change will require more work because we will be removing a configuration option. Someone will have to do the archaeology needed to understand why the option was added in the first place, so we don’t unwittingly risk breaking existing deployments. Was it added for completeness (because kombu supports it)? Was it added because some combination of kombu and rabbit needed the client to specify the setting because negotiation wasn’t implemented properly? Was there some other reason?

Doug

> 
> My point is that suggesting there's a vulnerability here without
> looking at how the code is used is sort of like shouting "fire" in a
> crowded theater.
> 
>> That said, code like:
>> https://github.com/mpaladin/python-amqpclt/blob/master/amqpclt/kombu.py#L101
>> is truely egregious!
> 
> Yikes... glad I'm not on _their_ VMT instead!
> -- 
> Jeremy Stanley
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev