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

[Jose Castro Leon]

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"}}}

[Jose Castro Leon (jose-castro-leon)]

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

[Dolph Mathews (dolph)]

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

[Jose Castro Leon (jose-castro-leon)]

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.

[Adam Young (ayoung)]

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

[Jose Castro Leon (jose-castro-leon)]

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.

[Jose Castro Leon (jose-castro-leon)]

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"

[Thierry Carrez (ttx)]

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

[Adam Young (ayoung)]

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.

[Adam Young (ayoung)]

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.

[Thierry Carrez (ttx)]

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 ?

[Thierry Carrez (ttx)]

Oops, should refresh before posting

[Jose Castro Leon (jose-castro-leon)]

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

[Adam Young (ayoung)]

Attached patch has a unit test on it as well

[Adam Young (ayoung)]

Missed a file in the commit for the last patch

[Adam Young (ayoung)]

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.

[Thierry Carrez (ttx)]

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.

[Jose Castro Leon (jose-castro-leon)]

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.

[Jose Castro Leon (jose-castro-leon)]

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

[Thierry Carrez (ttx)]

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

[Adam Young (ayoung)]

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)

[Dolph Mathews (dolph)]

+1 for latest patch and impact description.

[Dolph Mathews (dolph)]

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

[Jose Castro Leon (jose-castro-leon)]

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

[Jeremy Stanley (fungi)]

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

[Thierry Carrez (ttx)]

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

[Thierry Carrez (ttx)]

CVE requested

[Dolph Mathews (dolph)]

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

[Adam Young (ayoung)]

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

[Adam Young (ayoung)]

+1 for impact description

[Brant Knudson (blk-u)]

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.

[Brant Knudson (blk-u)]

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

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

[Thierry Carrez (ttx)]

Oops

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

[Thierry Carrez (ttx)]

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'

[Thierry Carrez (ttx)]

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.

[Thierry Carrez (ttx)]

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

[Thierry Carrez (ttx)]

New patch with correct unit tests for master branch

[Thierry Carrez (ttx)]

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

[Thierry Carrez (ttx)]

master: https://review.openstack.org/32896
grizzly: https://review.openstack.org/#/c/32895/
folsom: https://review.openstack.org/#/c/32894/

[Thierry Carrez (ttx)]

OSSA 2013-015

[Jamie Strandboge (jdstrand)]

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.