ldap driver returns hashed passwords

Bug #1178032 reported by Brant Knudson
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
High
Adam Young

Bug Description

If I'm using the LDAP identity backend, listing group users includes the users' passwords (sha-encoded, but that probably depends on LDAP server configuration).

Keystone shouldn't be handing out users' passwords.

The fix is probably to just remove the password attribute. If Keystone is just returning all attributes, then it should be changed to only return the attributes that are known to be safe.

Steps to recreate:

1) start with devstack configured to use LDAP
# set LDAP options in localrc
./stack.sh ...

2) add the default domain since it doesn't exist by default for some reason.

$ ldapadd -x -D dc=Manager,dc=openstack,dc=org -w ldapadminpwd
dn: cn=default,ou=Domains,dc=openstack,dc=org
objectclass: groupOfNames
member: cn=dummy

3) Create a couple users

(set environment variables so you're admin)

$ keystone user-create --name user1 --pass user1pwd
(example id is 1db4a4d16ba1458aae139db0f43b0904)
$ keystone user-create --name user2 --pass user2pwd
(example id is 4091d11924f5498c8008b655bcf94b9d)

4) Create a group

$ ldapadd -x -D dc=Manager,dc=openstack,dc=org -w ldapadminpwd

dn: ou=UserGroups,dc=openstack,dc=org
objectclass: organizationalUnit

dn: cn=group1,ou=UserGroups,dc=openstack,dc=org
objectclass: groupOfNames
member: cn=1db4a4d16ba1458aae139db0f43b0904,ou=Users,dc=openstack,dc=org
member: cn=4091d11924f5498c8008b655bcf94b9d,ou=Users,dc=openstack,dc=org

5) List the group members:

$ curl -H "X-Auth-Token: admintoken" http://localhost:35357/v3/groups/group1/users | python -m json.tool

{
    "links": {
        "next": null,
        "previous": null,
        "self": "http://localhost:5000/v3/groups/group1/users"
    },
    "users": [
        {
            "domain_id": "default",
            "id": "1db4a4d16ba1458aae139db0f43b0904",
            "links": {
                "self": "http://localhost:5000/v3/users/1db4a4d16ba1458aae139db0f43b0904"
            },
            "name": "user1",
            "password": "{SSHA}eQnQSd6SS6tioL/uN4M7odr/cf2SsjbG"
        },
        {
            "domain_id": "default",
            "id": "4091d11924f5498c8008b655bcf94b9d",
            "links": {
                "self": "http://localhost:5000/v3/users/4091d11924f5498c8008b655bcf94b9d"
            },
            "name": "user2",
            "password": "{SSHA}HDtgM7HcrlXnLM7N85htpz1kKYL2npMS"
        }
    ]
}

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

Adding keystone-core for opinion...

Not totally convinced this is a Keystone vulnerability. Should the attributes be filtered on Keystone side, or rather not be handed out by the LDAP server itself ? Who can list those users ? Doesn't that role already involve modifying the group members password ? I agree that this should be fixed, but I'm not sure there is an exploitable attack scenario here.

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

THe attribute is filtered already:

For example, authenticate ,which returns the user object calls:

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

identity.filter_user(user_ref)

implemented at
https://github.com/openstack/keystone/blob/master/keystone/identity/core.py#L31

which does
 user_ref.pop('password', None)

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

It doesn't look like the LDAP driver filters passwords on GET /users either.

Changed in keystone:
importance: Undecided → High
status: New → Confirmed
Revision history for this message
Dolph Mathews (dolph) wrote :

get_project_users() is not filtered either. This filtering should be done by the identity Manager rather than the individual drivers.

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

Taking a stab at Thierry's comments in #1:

"Should the attributes be filtered on Keystone side, or rather not be handed out by the LDAP server itself ?"

I think that if they Keystone server were configured to bind to the LDAP server as a non-administrator then the LDAP server would not include passwords in the response. So part of this is a configuration issue.

"Who can list those users ?"

Keystone's policy can be configured so that anybody can list group members. The default is admin only:
"identity:list_users_in_group": [["rule:admin_required"]],

"Doesn't that role already involve modifying the group members password ?"

Yes, this is probably the case. If the Keystone server is configured to bind to LDAP as admin, then the Keystone admin has the LDAP admin password.

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

So... my view on this is that it's a valid strengthening measure and should definitely be fixed, but I would not consider it a vulnerability, unless random users access that information about other users by default... which doesn't seem to be the case ?

If we agree on that we can open this bug and fix it publicly... But would the fix be backportable to stable branches, or is the behavior change unacceptable there ?

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

Password should neve be visible anywhere in the system. I think we can remove it from the LDAP backend everywhere. It will break the "change password" code, but that could be special cased.

The SQL backend makes use of the password to compare. The LDAP code does not need to do this, as it does a "simple bind" to authenticate the user.

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

Thierry - It doesn't look to me that random users can get passwords by default.

I also checked and getting users requires admin by default too.
    "identity:list_users": [["rule:admin_required"]],

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

This seems more like a hardening improvement we should make (whether we globally filter these, remove access to them in the backend by default, or both), but I think we should be safe to open this bug and only issue a security note unless other access vectors come to light during public analysis.

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

Will open publicly in a few, unless someone violently complains.

Thierry Carrez (ttx)
information type: Private Security → Public
tags: added: security
Adam Young (ayoung)
Changed in keystone:
assignee: nobody → Adam Young (ayoung)
Revision history for this message
Bryan D. Payne (bdpayne) wrote :

In all of the examples above, the admin is grabbing the passwords over HTTP... so anyone on that network can see them. Perhaps this is an orthogonal issue. But I think that it goes towards how bad it is to be moving SHA-1 password hashes around in the system. There will be unintended consequences because admins won't expect this behavior and/or won't understand how to properly protect themselves.

A good policy here should be to not deliver SHA-1 password hashes by default *ever*. And then to only make it an option for properly privileged users if there is a valid use case for why they would need them. At the moment, I'm not seeing that use case here. It seems to me that the connection to the LDAP server should be setup to not send this information back to Keystone, if at all possible.

Dolph Mathews (dolph)
summary: - ldap list members returns passwords
+ ldap driver returns hashed passwords
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/39406

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

Reviewed: https://review.openstack.org/39406
Committed: http://github.com/openstack/keystone/commit/cda7d1637c7276902ab8dc789590166347f742b3
Submitter: Jenkins
Branch: master

commit cda7d1637c7276902ab8dc789590166347f742b3
Author: Adam Young <email address hidden>
Date: Tue Jul 30 23:07:41 2013 -0400

    Remove passwords from LDAP queries

    Bug 1178032

    Change-Id: Idca895b1d4d2e611fe834f49b436864a73f4006c

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

Other bug subscribers

Remote bug watches

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