V3 api authentication method chaining

Bug #1299012 reported by Abu Shohel Ahmed
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Guang Yee
OpenStack Security Advisory
Undecided
Unassigned

Bug Description

When using authentication method chaining for token creation (POST) in Keystone V3 api , it is possible to use authentication credentials for two different users . For example, if i have an existing token for a Demo user, say 6bb934a0120f097a32b5d3cc71f83beb ( created earlier for demo tenant) and i have a user say 'test131' in admin tenant

Now i can make an authentication call using auth method chaining

{
   "auth":{
      "identity":{
         "methods":[
            "password",
            "token"
         ],
        "token":{
            "id":"6bb934a0120f097a32b5d3cc71f83beb"
         },
        "password":{
            "user":{
               "domain":{
                  "id":"default"
               },
               "name":"test131",
               "password":"test131"
            }
         }
      }
   }
}

The call will succeed even though two different users authentication credentials are used. The generated token will get properties of test131 user although the expirary date is set by demo user token. If we change the methods sequence, the generated token will get all properties from demo users token.

This is an undesired security behaviour - token should not be allowed to generate using credentials from two different users.

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

Definitely weird and undesirable! This doesn't seem to be a viable attack vector, though; can this be fixed publicly?

Thierry Carrez (ttx)
Changed in ossa:
status: New → Incomplete
Revision history for this message
Guang Yee (guang-yee) wrote :

Obviously the caller already have the credentials of the two users so we are not compromising anything. Keystone basically pick and choose the user based on the order in this case. Keystone's auth protocol was designed to handle multi-factor and multi-roundtrip authentications and there was no expectation that all auth methods will yield a user_id. It is up to the plugins themselves to agree on a user_id.

Revision history for this message
Abu Shohel Ahmed (shohel-csdu) wrote :

i can think of an attack scenario in which an organisation decided that all authentication must go through a chain e.g., external auth (say one time code) and user/passwd. This provides two factor authentication . Now, with the current setup, any attacker with the possession of one factor can assume the role of the victim user. For example, an attacker gets the possession of one time code of the target victim by some means, can create an own account with username/password and combine the both to assume the role of the victim user. This effectively reduce the authentication strength to one factor i.e., knowing the one time code.

 The attack, most commonly, can be performed by one tenant user against another tenant user.

Revision history for this message
Thierry Carrez (ttx) wrote :

I think the attack scenario is a bit far-stretched. Chaining tokens was never documented as a way to securely perform 2-factor auth (and IMHO should never be used for it, this bug being fixed or not). This should definitely be fixed but I don't think we should consider it an exploitable vulnerability.

Unless someone complains we shall open this bug and fix it quickly in public.

Revision history for this message
Guang Yee (guang-yee) wrote :

Neither password or token auth plugins are considered 2-factor. There are a number of ways to implement multi-factor authentication and the V3 auth methods does not dictate one way or the other. For example, you can implement 2-factor auth in a single auth method payload where the password is the combination of one time passcode and a static pin. It is up to the plug-in implementations to reconcile the attributes of an identity (or user_context). The identity attributes are conveyed in the user_context, which is shared among the plugins.

Having said that, I agree we need to fix the password and token auth plugins to agree on the user_id. But I don't think they pose a security problem right now.

Revision history for this message
Guang Yee (guang-yee) wrote :

patch attached

Revision history for this message
Thierry Carrez (ttx) wrote :

OK, time is up. Making this public so we can issue a quick fix.

Changed in ossa:
status: Incomplete → Invalid
information type: Private Security → Public
tags: added: security
Revision history for this message
Dolph Mathews (dolph) wrote :

Guang: can you propose your patch against master via gerrit now?

Changed in keystone:
milestone: none → icehouse-rc2
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/84945

Changed in keystone:
assignee: nobody → Guang Yee (guang-yee)
status: New → In Progress
Thierry Carrez (ttx)
Changed in keystone:
importance: Undecided → Medium
Thierry Carrez (ttx)
Changed in keystone:
milestone: icehouse-rc2 → none
tags: added: icehouse-rc-potential
Thierry Carrez (ttx)
tags: added: icehouse-backport-potential
removed: icehouse-rc-potential
Changed in keystone:
assignee: Guang Yee (guang-yee) → Brant Knudson (blk-u)
Brant Knudson (blk-u)
Changed in keystone:
assignee: Brant Knudson (blk-u) → Guang Yee (guang-yee)
Revision history for this message
Robert Clark (robert-clark) wrote :

Is there any possibility to leverage this to offset accountability for certain actions etc?

I don't fully understand the impact because I've never looked at chaining before.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to keystone (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/97852

Revision history for this message
Guang Yee (guang-yee) wrote :

@Robert Clark, not sure if I understand your question. Comment #3 suggested if one does not properly setup their authorization policy, this could be problematic. For example,

"some_api": "auth_method: token and auth_method: password and user_id:%(user_id)s",

where token and password methods are obtained using the combined credentials. But this is not something we support in OpenStack right now. We don't populate the auth method in auth_context which used for policy check.

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

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

commit 33fd4cf8b308fd078e632eed9f398b91ed77b35b
Author: guang-yee <email address hidden>
Date: Wed Apr 2 21:41:11 2014 -0700

    Make sure all the auth plugins agree on the shared identity attributes.

    Note: this patch also corrected some of the external auth tests where an
    auth request consists of two methods with two different identities.

    Closes-Bug: #1299012

    Change-Id: I5d7dd42d373879322823b16b215f11a015b734f8

Changed in keystone:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in keystone:
milestone: none → juno-1
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to keystone (master)

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

commit 7f5e120ab8e73fc77360a4adb2f73f2516bae8a0
Author: Morgan Fainberg <email address hidden>
Date: Wed Jun 4 09:37:12 2014 -0700

    Use translation hints

    Use the _LI() function instead of _() for new LOG.info messages.

    Change-Id: I6af25a33018e76b321488cacea65885fa597abbf
    Related-Bug: #1299012

Thierry Carrez (ttx)
Changed in keystone:
milestone: juno-1 → 2014.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers