OpenStack Compute (Nova)

[OSSA 2013-010] Insecure directory creation for signing

Reported by Kurt Seifried on 2013-04-30
278
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Keystone
Undecided
Unassigned
Folsom
Medium
Alan Pevec
OpenStack Compute (nova)
High
Russell Bryant
Folsom
High
Russell Bryant
Grizzly
High
Russell Bryant
OpenStack Security Advisory
Undecided
Thierry Carrez
python-keystoneclient
Medium
Dolph Mathews

Bug Description

Originally found by Grant Murphy (<email address hidden>):

The signing directory is used to store the signing certificates
and the default location for this directory is:

    signing_dir = /tmp/keystone-signing-nova

In the file:

   keystone/middleware/auth_token.py

During the initialization of the AuthMiddleware the following operations are made for the signing directory:

    IF the directory exists but cannot be written to a configuration error is raised.
    ELSE IF the directory doesn't exist, create it.
    NEXT chmod permisions(stat.S_IRWXU) to the signing_directory

AFAICT The signing certificates used in validation will only be fetched from the keystone if the cms_verify action raises an exception because the certificate file is missing from the signing directory.

This means that if an attacker populated the /tmp/keystone-signing-nova
with the appropriate files for signautre verification they could potentially
issue forged tokens which would be validated by the middleware. As:
    - The directory location deterministic. (default for glance, nova)
    - *If the directory already exists it is reused*

CVE References

Alan Pevec (apevec) wrote :

> and the default location for this directory is:
> signing_dir = /tmp/keystone-signing-nova

It is not, that happens to be default in Fedora's nova.conf
  http://pkgs.fedoraproject.org/cgit/openstack-nova.git/tree/nova.conf#n27

Default in authtoken middleware code (now located in python-keystoneclient) is ~/keystone-signing and normally homedir of the user account running the application (where authtoken m/w is inserted) should not be world-writable.

Alan Pevec (apevec) wrote :

ok, it's also in sample nova api-paste.ini

https://github.com/openstack/nova/blob/master/etc/nova/api-paste.ini#L107

That line should be removed so that default in code applies.

affects: keystone → nova
Thierry Carrez (ttx) wrote :

Ew. One would think that everyone knows that naming stuff under /tmp is bad, by now.

Adding Dolph and Russell for further discussion.

On upgrade, removing the default value will not make those who copied api-paste.ini magically secure, so I'd rather (or in addition) fix the temporary directory creation.

Thierry Carrez (ttx) wrote :

(i.e. not just check that you can write to it, but check that you actually own it)

Thierry Carrez (ttx) wrote :

FTR

cinder: signing_dir = /var/lib/cinder
quantum: signing_dir = /var/lib/quantum/keystone-signing

Everyone else doesn't seem to have that option.

Russell Bryant (russellb) wrote :

Attached a patch for the nova side of this that we can push at the same time as the authtoken middleware changes discussed here.

Dolph Mathews (dolph) wrote :

Two additional issues with the signing_dir code in keystoneclient:

- there's a race condition when a new signing dir is created with default permissions before they're set to 0700
- even if an existing directory is owned by the executing user, it may be globally writable before keystoneclient chmods it

The attached patch fixes these issues (including ttx's suggestion) but fails unit testing because they use a signing_dir w/ 0755. Feedback/review on the fix itself appreciated in the mean time.

I also think this should move to oslo.fileutils eventually.

Kurt Seifried (kseifried) wrote :

So no patch yet, do we have an ETA on one?

Thierry Carrez (ttx) wrote :

Kurt: I am not sure we nailed the correct way to fix that yet.
Dolph: I see two issues with the proposed patch

(1) it can break existing setups on upgrade (for people using an open directory)
(2) there is still a TOCTOU race that make it still potentially exploitable (even if unlikely)

The trick with creating directories in code in areas specified by the user and wanting to enforce permissions on that location is that it's actually quite difficult to do :) Alternatives include:

* use a subdirectory of a directory that is assumed safe (/var/lib/nova)
* rely on the packaging / deployer to create the directory they specify, with the right permissions (you can still issue a warning if they got the permissions wrong)
* pure (but unpredictable) /tmp directory atomic creation using Python tempfile module

Kurt: any suggestion on how to do this right ?

Thierry Carrez (ttx) wrote :

Adding Rob Clark for suggestions

Robert Clark (robert-clark) wrote :

I think ttx describes the the issue with the fix quite well, TOCTOU is tricky.

I'd be inclined to go with either the first option, subdir in a known-safe location - where known safe really seems to mean: 'if this is screwed, everything screwed anyway'.

Or go with the second option, putting this in the hands of deployers. If the second option was preferable I'd be happy to cut an OSSN for the issue.

Dolph Mathews (dolph) wrote :

With Russel's patch to nova and defaulting signing_dir to ~/keystone-signing in keystoneclient, I think ttx's first suggestion is covered.

Changing the keystoneclient patch to simply log warnings if the signing_dir's owner or permissions have unexpected values satisfies the second suggestion (see attached, which no longer needs any update to the test framework).

Changed in python-keystoneclient:
status: New → Confirmed
Thierry Carrez (ttx) wrote :

I think that's the right trade-off. Changing the default will fix it for new users, printing the warning in the log will alert existing vulnerable users but won't break them on upgrade, and we suppose that the parameter given is a correctly-owned directory.

Even the TOCTOU race seems closed since os.makedirs fails if the directory exists.

Thierry Carrez (ttx) wrote :

Adding Keystone core and Nova core...

Please review proposed patches (comment #8 for Nova and comment #14 for python-keystoneclient) and +1 them on the bug if you're happy with them.

Changed in python-keystoneclient:
importance: Undecided → Medium
Changed in nova:
status: New → Confirmed
importance: Undecided → High
Thierry Carrez (ttx) wrote :

I'd tend to consider this is a vulnerability in Nova, and the python-keystoneclient fix is more a strengthening thing (warn when the configuration is insecure)

Michael Still (mikalstill) wrote :

nova patch looks good to me.

Thierry Carrez (ttx) wrote :

Please review proposed impact description:

========================
Title: Nova uses insecure keystone middleware tmpdir by default
Reporter: Grant Murphy (Red Hat)
Products: Nova
Affects: All versions

Description:
Grant Murphy from Red Hat reported a vulnerability in Nova default location for the Keystone middleware signing directory (signing_dir). By previously setting up a malicious directory structure, an attacker with local shell access on the Nova node could potentially issue forged tokens that would be accepted by the middleware. Only setups that use the default value for signing_dir are affected. Note that future versions of the Keystone middleware will issue a warning if an insecure signing directory is used.
=========================

Michael Still (mikalstill) wrote :

Looks good to me.

Kurt Seifried (kseifried) wrote :

What do we want to set the embargo to on this one? Say May 13, 2013, 15:00 UTC?

Dolph Mathews (dolph) wrote :

description lgtm

Thierry Carrez (ttx) wrote :

Hmm, looks like it was independently reported publicly at bug 1177681

I propose we open this bug, push the patches publicly, and issue the OSSA as soon as possible. I'll send an early notice to downstream stakeholders today. Let me know if that worls for you all.

Thierry Carrez (ttx) wrote :

@glance-acc: Do you want to be credited for co-discovery ? Which name (and optionally company) would you want us to add to the advisory (see draft in comment #19)

glance (glance-acc) wrote :

My name is Anton Lundin and this work was done independently. Credits are always nice.

Sorry for not clicking the "Security" button when reporting. I thought it was quite minor as access to nova hosts ain't that common for non-admin users anyway.

Thierry Carrez (ttx) wrote :

OK, will add credit. FTR the issue was introduced in the Folsom cycle, so this doesn't affect Essex

Thierry Carrez (ttx) on 2013-05-08
information type: Private Security → Public Security

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

Changed in nova:
assignee: nobody → Russell Bryant (russellb)
status: Confirmed → In Progress

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/28570

Changed in python-keystoneclient:
assignee: nobody → Dolph Mathews (dolph)
status: Confirmed → In Progress

Will push the OSSA tomorrow

Alan Pevec (apevec) on 2013-05-08
Changed in keystone:
status: New → Invalid

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/28577

Changed in python-keystoneclient:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/28572
Committed: http://github.com/openstack/python-keystoneclient/commit/1736e2ffb12f70eeebed019448bc14def48aa036
Submitter: Jenkins
Branch: master

commit 1736e2ffb12f70eeebed019448bc14def48aa036
Author: Dolph Mathews <email address hidden>
Date: Wed May 8 10:49:20 2013 -0500

    Securely create signing_dir (bug 1174608)

    Also verifies the security of an existing signing_dir.

    Change-Id: I0685b4274a94ad3974a2b2a7ab3f45830d3934bb

Reviewed: https://review.openstack.org/28570
Committed: http://github.com/openstack/nova/commit/74aa04e2ca7942cb1e1a86dcbaffeb72d260ccd7
Submitter: Jenkins
Branch: stable/folsom

commit 74aa04e2ca7942cb1e1a86dcbaffeb72d260ccd7
Author: Russell Bryant <email address hidden>
Date: Wed May 1 09:41:57 2013 -0400

    Remove insecure default for signing_dir option.

    The sample api-paste.ini file included an insecure value for the
    signing_dir option for the keystone authtoken middleware. Comment out
    the option so that we just rely on the default behavior by default.

    Fix bug 1174608.

    Conflicts:
     etc/nova/api-paste.ini

    Change-Id: I6189788953d789c34456bbe150b8ed6ce6f68403
    (cherry picked from commit 58d6879b1caaa750c39c8e452a0634c24ffef2ce)

Reviewed: https://review.openstack.org/28568
Committed: http://github.com/openstack/nova/commit/58d6879b1caaa750c39c8e452a0634c24ffef2ce
Submitter: Jenkins
Branch: master

commit 58d6879b1caaa750c39c8e452a0634c24ffef2ce
Author: Russell Bryant <email address hidden>
Date: Wed May 1 09:41:57 2013 -0400

    Remove insecure default for signing_dir option.

    The sample api-paste.ini file included an insecure value for the
    signing_dir option for the keystone authtoken middleware. Comment out
    the option so that we just rely on the default behavior by default.

    Fix bug 1174608.

    Change-Id: I6189788953d789c34456bbe150b8ed6ce6f68403

Changed in nova:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/28569
Committed: http://github.com/openstack/nova/commit/7bf3e8d3e254d817ff5ae7ef1f2884b10410ca60
Submitter: Jenkins
Branch: stable/grizzly

commit 7bf3e8d3e254d817ff5ae7ef1f2884b10410ca60
Author: Russell Bryant <email address hidden>
Date: Wed May 1 09:41:57 2013 -0400

    Remove insecure default for signing_dir option.

    The sample api-paste.ini file included an insecure value for the
    signing_dir option for the keystone authtoken middleware. Comment out
    the option so that we just rely on the default behavior by default.

    Fix bug 1174608.

    Change-Id: I6189788953d789c34456bbe150b8ed6ce6f68403
    (cherry picked from commit 58d6879b1caaa750c39c8e452a0634c24ffef2ce)

Hi,

Is there a backport for Essex prepared by the Ubuntu security team? Or should I work on the backport myself?

Thomas

Thierry Carrez (ttx) wrote :

This does NOT affect Essex.

Thierry Carrez (ttx) wrote :

OSSA-2013-010

Reviewed: https://review.openstack.org/28577
Committed: http://github.com/openstack/keystone/commit/24c25b38ed6fc95ed919ab34463cdb10bdcc57fd
Submitter: Jenkins
Branch: stable/folsom

commit 24c25b38ed6fc95ed919ab34463cdb10bdcc57fd
Author: Dolph Mathews <email address hidden>
Date: Wed May 8 10:49:20 2013 -0500

    Securely create signing_dir (bug 1174608)

    Also verifies the security of an existing signing_dir.

    Change-Id: I0685b4274a94ad3974a2b2a7ab3f45830d3934bb
    (cherry picked from python-keystoneclient 1736e2ffb12f70eeebed019448bc14def48aa036)

note regression introduced by the nova change: bug 1181157

Thierry Carrez (ttx) on 2013-05-24
summary: - Insecure directory creation for signing
+ [OSSA 2013-010] Insecure directory creation for signing
Changed in ossa:
assignee: nobody → Thierry Carrez (ttx)
status: New → Fix Released
Thierry Carrez (ttx) on 2013-05-29
Changed in nova:
milestone: none → havana-1
status: Fix Committed → Fix Released
Dolph Mathews (dolph) on 2013-05-29
Changed in python-keystoneclient:
milestone: none → 0.2.4
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in nova:
milestone: havana-1 → 2013.2
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Duplicates of this bug

Other bug subscribers