[OSSA 2013-015] LDAP vulnerability when checking user credentials (CVE-2013-2157)

Bug #1187305 reported by Jose Castro Leon
268
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Critical
Adam Young
Folsom
Critical
Unassigned
Grizzly
Critical
Thierry Carrez
OpenStack Security Advisory
Critical
Thierry Carrez

Bug Description

There is a security vulnerability in the LDAP module when retrieving a token while checking the credentials of a user.

If the password field is not specified, the ldap module does not do the simple_bind and it always returns a valid connection.

 curl -i https://lxbrf17b01.cern.ch:5000/v2.0/tokens -X POST -H "Content-Type: application/json" -d '{"auth": {"passwordCredentials":{"username":"jcastro"}}}' --insecure

HTTP/1.1 200 OK
Vary: X-Auth-Token
Content-Type: application/json
Content-Length: 223
Date: Tue, 04 Jun 2013 08:30:31 GMT

{"access": {"token": {"expires": "2013-06-05T08:30:31Z", "id": "cec066b8d1df485a9d90e25b555ac0ff"}, "serviceCatalog": [], "user": {"username": "jcastro", "roles_links": [], "id": "jcastro", "roles": [], "name": "jcastro"}}}

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

There is a simple fix by checking before the get_connection call if the user_id and password are not empty.

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

Jose- are you producing a fix? If so, attach the patch here rather than submitting through gerrit.

Changed in keystone:
importance: Undecided → Critical
status: New → Triaged
Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

The problem there is in the get_connection part of the ldap module, if no user or password is specified it doesn't do the simple bind. So the quickest fix is to avoid call this function with empty user or password in the authenticate method.

There is another way by forcing the simple_bind_s when doing the authentication, but there could be users that don't use authentication and store their passwords in user ldap attributes.

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

+2 on the patch.

This vulnerability has limited effect, in that it will only connect as the "Manager account" which should not have any roles or groups that are used in the rest of the Keystone system.

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

I don't think its scope is limited, a user was able to request a token of a user on a specific tenant and then delete an instance.

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

Sequence

1) Obtain a token for another user:

curl -i https://ibex-cloud-controller.cern.ch:5000/v2.0/tokens -X POST -H "Content-Type: application/json" -H "Accept: application/json" -H "User-Agent: python-novaclient" -d '{"auth": {"*tenantName*": "User Tenant", "passwordCredentials": {"username": "*USER1*"}}}' --cacert /etc/pki/tls/certs/CERN-bundle.pem

2) Delete a VM using nova REST API:

curl -i http://ibex-cloud-controller.cern.ch:8774/v2/4a7a6f88a9ae403c921cfd50c7bfb46a/servers/bd862b2d-0801-4ee9-8562-4e37346fa56b -X DELETE -H "X-Auth-Project-Id: Personal chdoming" -H "User-Agent: python-novaclient" -H "Accept: application/json" -H "X-Auth-Token:fa1733607129jh1790c7f0c18a5d786f"

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

Waiting for confirmation on the impact, but at first glance I'd say jcastro is right.

Changed in ossa:
importance: Undecided → High
status: New → Confirmed
Revision history for this message
Adam Young (ayoung) wrote :

Jose, is your server set up with Anonymous binding for the manager account? If so, I can see how it will pass through if borth userid and password are left off, but it should get a token for the manager account, which should not be a "real" Open Stack /Keystone account, should have no roles, and should not be an administrator.

"passwordCredentials": {"username": "*USER1*"}}}' Should hit the LDAP authenticate function which does:

 conn = self.user.get_connection(self.user._id_to_dn(user_id),
                                            password)

https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/core.py#L112

So it should not be passed through Blankly. Thus, it should always do the simple bind.

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

OK, so the problem is https://github.com/openstack/keystone/blob/master/keystone/common/ldap/core.py#L223

Which is there to support anonymous binding for the Administrator account. If the end user skips the password, and the system is set up to do anonymous binding, then it bypasses simple bind.

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

Discussed it with ayoung and it seems to only affect setups where you use anonymous binding (no ldap.user or ldap.password set in the keystone configuration). Jose can you confirm that it is your case ?

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

Oops, should refresh before posting

Changed in ossa:
assignee: nobody → Thierry Carrez (ttx)
importance: High → Critical
Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

We don't have anonymous binding, we were using an account that has specific permissions to create objects in the AD tree.

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

Attached patch has a unit test on it as well

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

Missed a file in the commit for the last patch

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

Note that the last patch also removes some dead code that masked the problem: there was a get_connection call in the identity backend that was supposed to provide protection against the anonymous bind, but A) was improperly implemented and B) bypassed.

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

Jose: just to make sure we cover your case... You don't do anonymous binding but you use an account with an empty password, then ? (ldap.password not set). If both ldap.user and ldap.password are set, then from our current analysis the bind with empty password should fail.

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

We don't use anonymous binding and the account to connect to LDAP has a password. When authenticated the password was empty not None. Then in the get_connection function is was not replaced by LDAP_PASSWORD and the simple_bind was also not done.

I realize that when I was trying to obtain a quick fix and instead of 'not password', I tried 'password is None' and it was not raising the exception as it should be.

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

BTW: The patch solves the issue and cover our case :)

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

First try at an impact description, please make sure I got that right

=========================
Title: Authentication bypass when using LDAP backend
Reporter: Jose Castro Leon (CERN)
Products: Keystone
Affects: Folsom, Grizzly

Description:
Jose Castro Leon from CERN reported a vulnerability in the way the Keystone LDAP backend authenticates users. When provided with an empty password, the backend would perform an anonymous LDAP bind that would result in successfully authenticating the user. An attacker could therefore easily impersonate and get valid tokens for any user. Only Keystone setups using LDAP authentication backend are affected.
==========================

Keystone-core, Please also +2 the proposed patch at comment 15.
Adam, if i'm not mistaken we'll need Grizzly and Folsom backports for this one (unless the dead code path was still being used at that time ?)

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

Please confirm: in addition, the lines in keystone/common/ldap/core.py

        if user is None:
            user = self.LDAP_USER

        if password is None:
            password = self.LDAP_PASSWORD

should be

        if not user:
            user = self.LDAP_USER

        if not password:
            password = self.LDAP_PASSWORD

to match the logic of the check for bypassing the bind?

        if user and password:
            conn.simple_bind_s(user, password)

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

+1 for latest patch and impact description.

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

Adam's patch also applies cleanly to stable/grizzly and the new test passes.

Revision history for this message
Jose Castro Leon (jose-castro-leon) wrote :

+1 for last patch it also covers empty user and password...

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

The proposed impact description in comment #20 looks good to me.

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

Adam: could you backport it to Folsom ?
Could we get another keystone-core +2 on that patch ?

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

CVE requested

Changed in ossa:
status: Triaged → In Progress
Revision history for this message
Dolph Mathews (dolph) wrote :

Backport to folsom attached.

The fix is the same, but the test had to be revised for folsom (most importantly, removed domain_id and user's tenancy).

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

+1 for backport of patch. Tenant assignment is not necessary, as authentication is denied before the tenant is ever checked.

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

+1 for impact description

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

Reviewed patch in #15, keystone-ayoung-0005-1-Force-simple-Bind-for-authentication.patch. Looks good to me, +2

When I git apply the patch in #28, bug-1187305-folsom-v1.patch, to stable/folsom,
I only get identity/backends/ldap/core.py and the change to test doesn't apply for some reason.

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

I was able to apply the patch in #28, bug-1187305-folsom-v1.patch to stable/folsom, and that one looks good to me. +2.

Thierry Carrez (ttx)
summary: - LDAP vulnerability when checking user credentials
+ LDAP vulnerability when checking user credentials (CVE-2013-2157)
Revision history for this message
Thierry Carrez (ttx) wrote : Re: LDAP vulnerability when checking user credentials (CVE-2013-2157)

Proposed public disclosure date/time: Thursday, May 13, 1500UTC.

Changed in ossa:
status: In Progress → Fix Committed
Revision history for this message
Thierry Carrez (ttx) wrote :

Oops

Proposed public disclosure date/time: Thursday, June 13, 1500UTC.

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

Got the following errors running unit tests:

======================================================================
ERROR: test_authenticate_requires_simple_bind (test_backend_ldap.LDAPIdentity)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jenkins/workspace-cache/openstack/keystone/tests/test_backend_ldap.py", line 614, in test_authenticate_requires_simple_bind
    self.identity_api.authenticate,
AttributeError: 'Identity' object has no attribute 'authenticate'

======================================================================
ERROR: test_authenticate_requires_simple_bind (test_backend_ldap.LDAPIdentityEnabledEmulation)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jenkins/workspace-cache/openstack/keystone/tests/test_backend_ldap.py", line 614, in test_authenticate_requires_simple_bind
    self.identity_api.authenticate,
AttributeError: 'Identity' object has no attribute 'authenticate'

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

Looks like self.identity_api.authenticate should be self.identity_api.authenticate_user...

Running tests at least once is actually a good idea before +2ing a security patch.

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

Only affects the master patch. The grizzly and folsom branches still have Identity.authenticate().

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

New patch with correct unit tests for master branch

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

Ran devstack-gate-full on master and stable/folsom successfully.
Ran py27 unit tests on master and stable/folsom successfully.

Revision history for this message
Thierry Carrez (ttx) wrote :
information type: Private Security → Public Security
Changed in keystone:
status: Triaged → In Progress
Revision history for this message
Thierry Carrez (ttx) wrote :

OSSA 2013-015

summary: - LDAP vulnerability when checking user credentials (CVE-2013-2157)
+ [OSSA 2013-015] LDAP vulnerability when checking user credentials
+ (CVE-2013-2157)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

FYI, this doesn't seem to affect Essex since Essex lacks 0d32a417c811ce37b1b7ea1fbbc0a8376b9b3723 ("don't assume that the LDAP server require authentication") - ie, anonymous binds fail without that patch.

Thierry Carrez (ttx)
Changed in keystone:
status: In Progress → Fix Committed
Changed in ossa:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
milestone: none → havana-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
milestone: havana-2 → 2013.2
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers