[OSSA 2013-010] Insecure directory creation for signing

[Kurt Seifried]

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*

[Alan Pevec (apevec)]

> 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)]

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.

[Thierry Carrez (ttx)]

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)]

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

[Thierry Carrez (ttx)]

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)]

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)]

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)]

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

[Thierry Carrez (ttx)]

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)]

Adding Rob Clark for suggestions

[Robert Clark (robert-clark)]

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)]

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).

[Thierry Carrez (ttx)]

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)]

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.

[Thierry Carrez (ttx)]

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 (mikal)]

nova patch looks good to me.

[Thierry Carrez (ttx)]

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 (mikal)]

Looks good to me.

[Kurt Seifried (kseifried)]

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

[Dolph Mathews (dolph)]

description lgtm

[Thierry Carrez (ttx)]

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)]

@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)]

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)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/grizzly
Review: https://review.openstack.org/28569

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Thierry Carrez (ttx)]

Will push the OSSA tomorrow

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[Thomas Goirand (thomas-goirand)]

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)]

This does NOT affect Essex.

[Thierry Carrez (ttx)]

OSSA-2013-010

[OpenStack Infra (hudson-openstack)]

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)

[Bernhard M. Wiedemann (ubuntubmw)]

note regression introduced by the nova change: bug 1181157