[OSSA 2014-024] nova metadata does not use a constant time compare for validating an HMAC token (CVE-2014-3517)

[Alex Gaynor]

Here:

https://github.com/openstack/nova/blob/HEAD/nova/api/metadata/handler.py#L173

a constant time comparison should be used, more information on this type of attack here: http://codahale.com/a-lesson-in-timing-attacks/

An example constant time comparison in Python can be found here: https://github.com/django/django/blob/master/django/utils/crypto.py#L80 or via the PyCA cryptography library: https://cryptography.io/en/latest/hazmat/primitives/constant-time/

[Tristan Cacqueray (tristan-cacqueray)]

Thank you for the report.

The OSSA task is incomplete pending additional details from security reviewers, nova-coresec are now subscribed to the bug.

[Jeremy Stanley (fungi)]

Since the nova security reviewers have been too busy to weigh in on this yet, I've added a few reviewers from the OSSG to help evaluate risk and exploitability.

If this demonstrably makes it possible to guess an HMAC token gaining unintended access/privileges in a supported release of nova and can be fixed in a stable backport, then we likely need to keep it embargoed while patches are assembled. Otherwise, I suspect this is a security hardening improvement we can discuss and develop far more efficiently in the open.

[Alex Gaynor (alex-gaynor)]

This particularly one hasn't been demonstrated to be exploitable, however this precise pattern has: http://rdist.root.org/2009/05/28/timing-attack-in-google-keyczar-library/

If someone more familiar with this bit of code and the implications can weigh in and says it's fine, I'm happy to just send up a CR for this.

[Andrew Laski (alaski)]

I'm basing the following comments on just having gone through the code paths before this comparison takes place so confirmation from someone more familiar with this flow would be nice.

But from what I'm seeing there is no user input that makes it into the token generation, depending on how networking access is structured for the nova metadata service. This token comparison is based on CONF.service_neutron_metadata_proxy being set which means the deployer is expecting the Neutron namespace proxy to be setup. The namespace proxy does a lookup based on the ip address of the instance making a query to it, and populates the X-Instance-ID header based on that result and then forwards to the Nova metadata service. I would hope that the metadata service would not be directly accessible from instances.

If the metadata service is accessible from instances and CONF.service_neutron_metadata_proxy is set to True then this would be theoretically vulnerable to a timing attack. The result of which would be gaining access to information about another instance. It should be noted that this bug includes Neutron as well since it performs the signing so any change would have to be coordinated across both services.

[Russell Bryant (russellb)]

The fix should be something like this ... though not tested

[Alex Gaynor (alex-gaynor)]

Looks reasonable to me, we should probably add a constant time comparison to oslo so it can be shared across multiple projects.

[Jeremy Stanley (fungi)]

So it seems there's some consensus that this is not generally exploitable and can instead be fixed in public as a hardening measure?

As for adding a constant time comparison to oslo, are there not already existing Python modules which can provide that so we don't reinvent the wheel (the age old adage of not rolling one's own crypto primitives seems applicable here, even if this isn't strictly a cryptographic matter)?

[Alex Gaynor (alex-gaynor)]

PyCA cryptography (cryptography.io) has one, as does Django, and the stdlib has one in 2.7.8+/3.4+ IIRC, probably a bunch of other projects have them too (I know for a fact both keystoneclient and swift do).

[Andrew Laski (alaski)]

I would be ok with opening this up to be a public bug. The exploit is theoretical at this point, and even the included links contain theoretical attacks with no proof of concept code. And in general this token compare should not be receiving user input, though that is dependent on the deployment setup.

[Thierry Carrez (ttx)]

Yes, I think we can open this one and fix it as a hardening measure. Scream soon if you disagree... otherwise we'll open it in a couple of days.

[Robert Clark (robert-clark)]

History is littered with attack vectors that were only theoretical - until they weren't.

Are there reasonable configuration steps that can be taken to lessen the risk?

[Jeremy Stanley (fungi)]

Also, is the proposed patch something we would consider backporting to stable release branches? It does look trivial, introducing no new dependencies nor config options, so I'm assuming yes.

[Thierry Carrez (ttx)]

Agreed it's easy to backport, let's do an OSSA then.

[Grant Murphy (gmurphy)]

Ceilometer also appears to get this wrong here.

https://github.com/openstack/ceilometer/blob/master/ceilometer/publisher/utils.py#L71

And swift? (Or is this test code?)

https://github.com/openstack/swift/blob/master/swift/common/middleware/tempauth.py#L285

[Grant Murphy (gmurphy)]

So if we are going to issue an advisory for this maybe we could do something like this for the impact description:

Title: Use of non-constant time comparison operation
Reporter: Alex Gaynor (Rackspace)
Products: Nova
Versions: Up to 2013.2.3, and 2014.1 to 2014.1.1

Alex Gaynor of Rackspace reported a timing attack vulnerability in Nova.
By analyzing response times to requests for instance metadata, an attacker
may be able to guess a valid instance ID signature. This could allow access
to important configuration details of another instance. Only setups
configured to proxy metadata requests via Neturon are affected.

--

[Grant Murphy (gmurphy)]

Created bug #1332390 for ceilometer issue.

[Thierry Carrez (ttx)]

nova coresec: please review proposed patch

[Grant Murphy (gmurphy)]

CVE assigned using the following impact description:

Title: Use of non-constant time comparison operation
Reporter: Alex Gaynor (Rackspace)
Products: Nova
Versions: Up to 2013.2.3, and 2014.1 to 2014.1.1

Alex Gaynor from Rackspace reported a timing attack vulnerability in Nova.
By analyzing response times to requests for instance metadata, an attacker
may be able to guess a valid instance ID signature. This could allow access
to important configuration details of another instance. Only setups
configured to proxy metadata requests via Neutron are affected.

Disclosure day/date schedule TBD. In preparation can we please get a definitive +1 / +2 on the patch from nova-coresec ?

[John Garbutt (johngarbutt)]

I am +2 on russell's patch (assuming it passes gate tests, etc), as it seems to do the right thing, as far as I can tell from that article.

I would love to get someone to have time to do a manual test of that patch, but I think it makes sense.

[Grant Murphy (gmurphy)]

FWIW Keystone uses a slightly more comprehensive approach than the proposed patch -

https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/memcache_crypt.py#L88

We should probably at least fall back to hmac.compare_digest if it is available.

[Grant Murphy (gmurphy)]

The original patch supplied no longer applies cleanly to master. I've tweaked it to the keystone version and back-ported to supported security levels.

Can we get these patches reviewed and approved? I would like to send the pre-OSSA out soon with a view to have a disclosure date of mid next week.

[Tristan Cacqueray (tristan-cacqueray)]

./run_tests.sh reported no errors, all tests passed with all three patches on local Ubuntu (12.04) based devstacks.

@nova-coresec: please have a look at those too!

[Grant Murphy (gmurphy)]

Proposed public disclosure date/time:
2014-07-16, 1500UTC

[Grant Murphy (gmurphy)]

Reviews -

https://review.openstack.org/107396
https://review.openstack.org/107397
https://review.openstack.org/107398

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/107396
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=4a60c6a655006b2882331844664fac5cf67c5f34
Submitter: Jenkins
Branch: master

commit 4a60c6a655006b2882331844664fac5cf67c5f34
Author: Grant Murphy <email address hidden>
Date: Tue Jul 8 03:35:40 2014 +0000

    Avoid possible timing attack in metadata api

    Introduce a constant time comparison function to
    nova utils for comparing authentication tokens.

    Change-Id: I7374f2edc6f03c7da59cf73ae91a87147e53d0de
    Closes-bug: #1325128

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/107398
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1dd97d1335f6ec028d0e4440250f80802a2f1d18
Submitter: Jenkins
Branch: stable/havana

commit 1dd97d1335f6ec028d0e4440250f80802a2f1d18
Author: Grant Murphy <email address hidden>
Date: Tue Jul 8 03:35:40 2014 +0000

    Avoid possible timing attack in metadata api

    Introduce a constant time comparison function to
    nova utils for comparing authentication tokens.

    Conflicts:
     nova/tests/test_utils.py
     nova/utils.py

    Closes-bug: #1325128
    Change-Id: I7374f2edc6f03c7da59cf73ae91a87147e53d0de
    (cherry picked from commit 9f59ca751f1a392ef24d8ab73a7bf5ce9655017e)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/107397
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9f59ca751f1a392ef24d8ab73a7bf5ce9655017e
Submitter: Jenkins
Branch: stable/icehouse

commit 9f59ca751f1a392ef24d8ab73a7bf5ce9655017e
Author: Grant Murphy <email address hidden>
Date: Tue Jul 8 03:35:40 2014 +0000

    Avoid possible timing attack in metadata api

    Introduce a constant time comparison function to
    nova utils for comparing authentication tokens.

    Conflicts:
     nova/utils.py

    Closes-bug: #1325128
    Change-Id: I7374f2edc6f03c7da59cf73ae91a87147e53d0de
    (cherry picked from commit 4a60c6a655006b2882331844664fac5cf67c5f34)

[Scott Moser (smoser)]

I've opened bug 1361357 to address the very significant performance regression caused by this fix.

[Scott Moser (smoser)]

Please ignore the above comment. bug 1361357's performance change is not a result of this bug.