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

Bug #1324592 reported by Steven Hardy
268
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Critical
Adam Young
Havana
Fix Released
Critical
Adam Young
Icehouse
Fix Released
Critical
Adam Young
OpenStack Security Advisory
Fix Released
High
Tristan Cacqueray

Bug Description

I've been experimenting with chaining keystone trusts, and I've encountered what I think is a privilege escalation flaw, where the scope enforced by the trust when initially delegating can be circumvented by creating another trust.

I spoke about this briefly with ayoung on IRC and he seems to be in agreement that this is a bug.

Details:

1. User1 has roles admin and heat_stack_owner
2. User1 delegates to User2 via a trust, only delegating only heat_stack_owner, and enabling impersonation
3. User2 gets a trust-scoped token, impersonating User1
4. User2 creates a new trust, delegating both admin and heat_stack_owner to User3
5. This works, and so when User3 gets a trust scoped token, they can get elevated privileleges, effectively defeating the point of role-limited delegation via the trust.

I've attached a reproducer which demonstrates the problem.

CVE References

Revision history for this message
Steven Hardy (shardy) wrote :
summary: - Trust scope can be circumvented by chaining trustts
+ Trust scope can be circumvented by chaining trusts
Changed in ossa:
status: New → Incomplete
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote : Re: Trust scope can be circumvented by chaining trusts

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 ?

Dolph Mathews (dolph)
Changed in keystone:
importance: Undecided → Critical
Revision history for this message
Adam Young (ayoung) wrote :

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

Is there any existing need for impersonation?

Revision history for this message
Steven Hardy (shardy) wrote :

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

Adam Young (ayoung)
Changed in keystone:
assignee: nobody → Adam Young (ayoung)
Revision history for this message
Adam Young (ayoung) wrote :

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.

Revision history for this message
Adam Young (ayoung) wrote :

Above fails pep8.

Revision history for this message
Adam Young (ayoung) wrote :

Fixes Pep8 and test failures

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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

Thierry Carrez (ttx)
Changed in ossa:
status: Incomplete → Confirmed
importance: Undecided → High
Revision history for this message
Jeremy Stanley (fungi) wrote :

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

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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

Revision history for this message
Adam Young (ayoung) wrote :

Addresses the OAuth and Trust integration use cases.

Revision history for this message
Adam Young (ayoung) wrote :

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

Changed in keystone:
status: New → In Progress
Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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

Revision history for this message
Adam Young (ayoung) wrote :

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.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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.

Revision history for this message
Brant Knudson (blk-u) wrote :

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

Revision history for this message
Adam Young (ayoung) wrote :

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

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

The Patch in 21 looks good to me.

Revision history for this message
Brant Knudson (blk-u) wrote :

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.

Revision history for this message
Adam Young (ayoung) wrote :

Backport to Icehouse.

Revision history for this message
Adam Young (ayoung) wrote :

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

Revision history for this message
Adam Young (ayoung) wrote :

Addresses comments from blk-u

Revision history for this message
Adam Young (ayoung) wrote :

Backport for Icehouse

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

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.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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

Revision history for this message
Garth Mollett (gmollett) wrote :

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

Revision history for this message
Adam Young (ayoung) wrote :

Backport to Havana.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@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!

Revision history for this message
Dolph Mathews (dolph) wrote :

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

Revision history for this message
Dolph Mathews (dolph) wrote :

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

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

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

Revision history for this message
Jeremy Stanley (fungi) wrote :

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.

Revision history for this message
Adam Young (ayoung) wrote :

Impact description draft #2 looks accurate.

Revision history for this message
Dolph Mathews (dolph) wrote :

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.

Revision history for this message
Brant Knudson (blk-u) wrote :

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

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

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.

Revision history for this message
Dolph Mathews (dolph) wrote :

+1 for impact description

Changed in ossa:
assignee: nobody → Tristan Cacqueray (tristan-cacqueray)
Revision history for this message
Garth Mollett (gmollett) wrote :

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

Grant Murphy (gmurphy)
summary: - Trust scope can be circumvented by chaining trusts
+ Trust scope can be circumvented by chaining trusts (CVE-2014-3476)
Revision history for this message
Adam Young (ayoung) wrote : Re: Trust scope can be circumvented by chaining trusts (CVE-2014-3476)

Updated Patch for Havana.

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

Revision history for this message
Adam Young (ayoung) wrote :

Updated Havana patch triggers a unit test failure.

Revision history for this message
Adam Young (ayoung) wrote :

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

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

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

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

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.

Revision history for this message
Dolph Mathews (dolph) wrote :

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

Thierry Carrez (ttx)
Changed in ossa:
status: Confirmed → In Progress
Revision history for this message
Steven Hardy (shardy) wrote :

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

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

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

Changed in ossa:
status: In Progress → Fix Committed
information type: Private Security → Public Security
summary: - Trust scope can be circumvented by chaining trusts (CVE-2014-3476)
+ [OSSA 2014-018] Trust scope can be circumvented by chaining trusts
+ (CVE-2014-3476)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/icehouse)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/havana)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

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

Changed in keystone:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/icehouse)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/havana)

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

Changed in ossa:
status: Fix Committed → Fix Released
Changed in keystone:
milestone: none → juno-2
status: Fix Committed → Fix Released
Chuck Short (zulcss)
tags: added: in-stable-icehouse
Thierry Carrez (ttx)
Changed in keystone:
milestone: juno-2 → 2014.2
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.