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

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

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

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.

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

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.

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"

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

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.

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.

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 ?

Oops, should refresh before posting

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

Attached patch has a unit test on it as well

Missed a file in the commit for the last patch

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.

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.

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.

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

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

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)

+1 for latest patch and impact description.

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

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

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

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

CVE requested

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

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

+1 for impact description

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.

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.

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

Oops

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

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'

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.

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

New patch with correct unit tests for master branch

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

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

OSSA 2013-015

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.