[OSSA 2014-018] Trust scope can be circumvented by chaining trusts (CVE-2014-3476)

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

It does sounds like a practical privilege escalation, shouldn't we raise the priority ?

We originally had a check that a trust token could not be used to make a trust.

Is there any existing need for impersonation?

> Is there any existing need for impersonation?

Yes, AFAIK all of the current use-cases for trusts (heat/solum/ceilometer) require impersonation.

If we need to add a temporary check which denies creating a trust with a trust-scoped token, that sounds fine to me, but then I'd like to get a spec worked out asap for explicit chaining, which is what I tried to specify in the trusts-chained-delegation BP (which was marked implemented for Icehouse), but clearly didn't explain very well because limited-use-trusts got implemented instead :)

I'm happy to work with ayoung to define a spec which enables a limited chain of delegation, and do the work to implement it, as it is needed for Solum/Heat interoperability ref bug #1317293 (and potentially heat/ceilometer too)

This patch performs an explicit test for the trust_id. In order to perform it, we need to make sure that the context is properly populated.

Above fails pep8.

Fixes Pep8 and test failures

Minor Issue on the first pass, the patch has an untranslated string in it:

 raise exception.Forbidden(
+ 'Cannot create a trust with a trust scoped token.')

Should be:

 raise exception.Forbidden(
+ _('Cannot create a trust with a trust scoped token.'))

Will look again once I'm back online see if anything else stands out.

I think, after further analysis, this same bug extends much much further than just trusts. I will run this by ayoung and nathan kinder tomorrow morning to determine if we need a separate bug / i'm wrong / if this can all be combined into a single bug.

I just discussed this with ayoung and confirmed it is possible to perform the same type of escalation with the oauth contrib module. It would also be possible to use Oauth based tokens to create trusts that can escalate permissions.

Oauth -> Oauth

Trust -> Oauth

Oauth -> Trust

The immediate solution should be to prevent any form of chained delegation to occur from trusts or oauth (extending the new code to cover oauth scenarios).

So just to confirm, this affects only stable/icehouse and master? I assume the federated identity work for icehouse introduced the current flaw...

Looking at the code, this appears to affect Master, Icehouse, and Havana.

Addresses the OAuth and Trust integration use cases.

Typos in commit message ACKed. Waiting on feedback for actual patch before reposing.

Suggestions for the code:

It looks like the code mitigates the privilege escalation, it feels like the code should go a bit further (yes this could all be controlled via policy, but I get the feeling that most deployments would not limit in this way).

Suggestion #1:
- While it is useful to include all the OAuth information and Trust information in the credentials (common/authorization) it would be easier for both cases to set a 'is_delegated_auth' flag and we can universally check for that instead of having to check trust, and oauth, and oauth, and trust, and if anything else crops up.

Suggestion #2:
If you cannot create new trusts/OAuth request tokens (authorize in the case of Oauth), the user should not be able to delete them. Likely, the user shouldn't be able to list them either. Once we get to chaining of trusts / whatever else is out there that allows delegation of smaller scope that is current, we can allow those smaller-scope delegations to be managed by the user with delegated auth. At this point, it seems that it would be unexpected for a user to be able to delete a trust or OAuth related stuff because they authed as the via a delegated role.

This assessment comes from the sample policy allowing a lot of various things to occur as admin (all oauth related actions request token etc, require admin) - lets prevent people from screwing up other people's access if they were (for some reason) delegated admin on their project.

This patch will require minor differences for stable/icehouse:

nullptr:keystone morgan$ git apply --reject patch
Checking patch keystone/common/authorization.py...
Hunk #1 succeeded at 87 (offset 19 lines).
Hunk #2 succeeded at 123 (offset 19 lines).
Checking patch keystone/contrib/oauth1/controllers.py...
Checking patch keystone/tests/test_v3_auth.py...
Hunk #1 succeeded at 2777 (offset -50 lines).
Checking patch keystone/tests/test_v3_oauth1.py...
Hunk #1 succeeded at 486 (offset 10 lines).
Checking patch keystone/trust/controllers.py...
error: while searching for:
        The user creating the trust must be the trustor.

        """
        if not trust:
            raise exception.ValidationError(attribute='trust',
                                            target='request')

error: patch failed: keystone/trust/controllers.py:124
Applied patch keystone/common/authorization.py cleanly.
Applied patch keystone/contrib/oauth1/controllers.py cleanly.
Applied patch keystone/tests/test_v3_auth.py cleanly.
Applied patch keystone/tests/test_v3_oauth1.py cleanly.
Applying patch keystone/trust/controllers.py with 1 reject...
Rejected hunk #1

Where would is_delegated_auth flag live? In the token itself? It means a more invasive patch, and larger tokens. If outside of the token, it means we need a common library location for it, and none really suits the scoped of this patch.

Deleting a trust from a trust is a logically acceptable activity: it would give you a way to clean up.

Listing trusts from a trust is acceptable. Listing trusts provideds no additional access. It is an activity that may make sense withing some workflows.

Even creating a trust from a trust is logically acceptable, but it requires a lot more checks than can be done in the scope of this patch.

I'll work up an icehouse and havana version of the patch.

the "delegated" flag would be in creds in authorization.py, just a single check instead of checking oauth and trusts in any/all locations.

I also agree that logically you should be able to create trusts (etc). but based upon the limited policy / issues around it (notably not backporting a whole featureset to icehouse/havana) we should restrict these things down as much as possible.

I reviewed the patch in comment 13 and it looks good to me.

Now also blocks list_request_tokens as that could potentially be abused as well.

The Patch in 21 looks good to me.

A couple of comments on the patch in comment 21 --

1) in test_v3_oauth1.py at line 556, a file is created. The file should be deleted with self.addCleanup(os.remove, self.tmpfilename)
2) same file, at line 559 is extra-indented (8 chars rather than 4 as expected)

Other than that looks good to me.

Backport to Icehouse.

Last patch has same issues as previously reviewed by blk-u

Addresses comments from blk-u

Backport for Icehouse

Hello, thank you for such fast fixes :)

I'm not entirely sure what is the attack scenario for the OAuth case, so I leave it out of this first draft.
Can someone confirm there is a vulnerability specifics to the OAuth process? Or is it already covered by the following description ?

Here is the impact description draft #1:

Title: Keystone trust chained delegation privilege escalation
Reporter: Steven Hardy (RedHat)
Products: Keystone
Versions: up to 2013.2.3, and 2014.1

Description:
Steven Hardy from RedHat reported a vulnerability in Keystone chained delegation. By chaining delegation from a trust token, a trustee may circumvent the enforced scope, resulting in potential elevated privileges to any of the trustor's roles. All keystone setups are affected.

@Tristan,

The OAuth attack scenario is similar to trust one:

* Trustee creates an OAuth authentication delegation (with elevated privs)
* ReAuth with new OAuth based delegation.

Alternatively, someone authenticated with an OAuth delegation could:
* Create Trust with elevated privileges
* ReAuthenticate with new Trust.

The fixes Adam Young has provided eliminate both of these alternate attack vectors.

@Tristan

As this came from RedHat I can assign a CVE to this now if it saves you time?
Or I can hold off and wait for the official request if you would prefer.

Backport to Havana.

@Morgan,

Thank, that is very helpful, I'll update the impact description.
Just to be sure, the privilege elevation can only happen with out-of-scope roles from the former authentication.
I mean, it's not to any role right ?

@Garth

I would prefer having the impact description validated before requesting a CVE. Then yes, feel free to assign the CVE thanks!

@Tristan:

1. User A trusts User B to impersonate them within the scope of Project X (using User A's roles AND identity).

2a. User B abuses the identity impersonation against keystone to delegate User A's authorization (either to User B, or to a third party) in Project Y, a secondary scope in which User A never intended to delegate authorization.

2b. Alternatively, User B abuses the identity impersonation against keystone to delegate *additional* authorization (either to User B, or to a third party) held by User A in Project X, including roles which User A never intended to delegate.

Also note that you should be able to avoid exposure by completely disabling support for trusts, by setting the following in keystone.conf:

  [trust]
  enabled = false

Trust support is enabled by default since Grizzly.

I suppose it's also worth mentioning that 2a and 2b are not mutually exclusive possibilities, and can each be repeated.

@Dolph thanks for the clarification!

I put 2014.1.1 as an affected version because with the pre-OSSA process, this won't get in time for this release.

Here is the impact description draft #2:

Title: Keystone privilege escalation through trust chained delegation
Reporter: Steven Hardy (RedHat)
Products: Keystone
Versions: up to 2013.2.3, and 2014.1.1

Description:
Steven Hardy from RedHat reported a vulnerability in Keystone chained delegation. By creating a delegation from a trust or OAuth token, a trustee may abuse the identity impersonation against keystone and circumvent the enforced scope, resulting in potential elevated privileges to any of the trustor's projects and or roles. Note that trust support is enabled by default since Grizzly and can only be manually disabled through keystone configuration. All keystone setups are affected.

Versions: up to 2013.2.3, and 2014.1 to 2014.1.1

(because 2014.1 is also affected)

Otherwise Tristan's impact description proposed in comment #35 looks great.

Impact description draft #2 looks accurate.

Tristan: I feel like "All keystone setups are affected" conflicts with the previous sentence... maybe combine the two statements? Something like:

  All Keystone deployments configured to enable trusts are affected, which has been the default since Grizzly.

Reviewed the patch in comment 26, looks good to me.

Reviewed the icehouse backport in comment 27, looks good to me.

Reviewed the havana backport in comment 31
 - there's an unrelated change in keystone/exception.py that should be reverted.
   This change causes a unit test failure on my system.
 - there's commented-out code in test_v3_oauth1.py at line 529 that should be removed

Thanks folks, I've updated the description with your comments,
Here is the impact description draft #3:

Title: Keystone privilege escalation through trust chained delegation
Reporter: Steven Hardy (Red Hat)
Products: Keystone
Versions: up to 2013.2.3, and 2014.1 to 2014.1.1

Description:
Steven Hardy from Red Hat reported a vulnerability in Keystone chained delegation. By creating a delegation from a trust or OAuth token, a trustee may abuse the identity impersonation against keystone and circumvent the enforced scope, resulting in potential elevated privileges to any of the trustor's projects and or roles. All Keystone deployments configured to enable trusts are affected, which has been the default since Grizzly.

+1 for impact description

Ok. hope I'm not jumping in too early here: Please use CVE-2014-3476 for this issue.

Updated Patch for Havana.

Removes the change to exception and the commented out code are referred to by blk-u

Updated Havana patch triggers a unit test failure.

Unit test failures were result of artifacts from previous runs in the git repository. Unit tests run clean.

Can keystone-coresec please have a look at proposed patch before we proceed to stakeholders pre-OSSA:

stable/havana is in comment #43
stable/icehouse in comment #27
master is in comment #26

Thanks in advance

Patches in #43, #27, and #26 apply cleanly to the respective branches.

Tests appear to pass for all patches.

The changeset in #26 looks good to me against master
The changeset in #27 looks good to me against icehouse
The changeset in #43 looks good to me against havana

+1 on the patches and impact statement, I would like to see a second core review the patches before they are merged though.

The fix is actually over-restrictive. It's not delegated auth in general that should cause these operations to be denied, but specifically just impersonation. Regardless, the fix is certainly effective at closing the vulnerability.

+1 for master patch in #26
+1 for stable/icehouse patch in #27
+1 for stable/havana patch in #43

I have a few small nits on the request context stuff, but I'll save those for a subsequent patch to master :)

+1 for master patch in #26 - I've pulled that and tested it, and can confirm it resolves the reported issue.

I've not tested the stable backports.

The pre-OSSA has been sent, the proposed disclosure date/time is:
2014-06-12, 1500UTC

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

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/99700

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/99703

Reviewed: https://review.openstack.org/99687
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ebfc2da158d15fe2f36787bcbf7407a0fa8f551d
Submitter: Jenkins
Branch: master

commit ebfc2da158d15fe2f36787bcbf7407a0fa8f551d
Author: Adam Young <email address hidden>
Date: Thu May 29 13:56:17 2014 -0400

    Block delegation escalation of privilege

    Forbids doing the following with either a trust
      or oauth based token:
      creating a trust
      approving a request_token
      listing request tokens

    Change-Id: I1528f9dd003f5e03cbc50b78e1b32dbbf85ffcc2
    Closes-Bug: 1324592

Reviewed: https://review.openstack.org/99700
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=73785122eefe523bb57c819e085c7f6ec97d779c
Submitter: Jenkins
Branch: stable/icehouse

commit 73785122eefe523bb57c819e085c7f6ec97d779c
Author: Adam Young <email address hidden>
Date: Thu May 29 13:56:17 2014 -0400

    Block delegation escalation of privilege

    Forbids doing the following with either a trust
      or oauth based token:
      creating a trust
      approving a request_token
      listing request tokens

    Change-Id: I1528f9dd003f5e03cbc50b78e1b32dbbf85ffcc2
    Closes-Bug: 1324592

Reviewed: https://review.openstack.org/99703
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=9162837329665b4316afc2270a602dc8ae11f6d2
Submitter: Jenkins
Branch: stable/havana

commit 9162837329665b4316afc2270a602dc8ae11f6d2
Author: Adam Young <email address hidden>
Date: Thu May 29 13:56:17 2014 -0400

    Block delegation escalation of privilege

    Forbids doing the following with either a trust
      or oauth based token:
      creating a trust
      approving a request_token
      listing request tokens

    Change-Id: I1528f9dd003f5e03cbc50b78e1b32dbbf85ffcc2
    Closes-Bug: 1324592