[gentoo-commits] gentoo-x86 commit in sys-auth/keystone/files: 2014.1-CVE-2014-0204.patch

[Matt Thode (prometheanfire)]

prometheanfire    14/06/01 02:19:16

  Added:                2014.1-CVE-2014-0204.patch
  Log:
  fix for CVE-2014-0204 bug 511000
  
  (Portage version: 2.2.8-r1/cvs/Linux x86_64, signed Manifest commit with key 0x2471eb3e40ac5ac3)

Revision  Changes    Path
1.1                  sys-auth/keystone/files/2014.1-CVE-2014-0204.patch

file : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-auth/keystone/files/2014.1-CVE-2014-0204.patch?rev=1.1&view=markup
plain: http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-auth/keystone/files/2014.1-CVE-2014-0204.patch?rev=1.1&content-type=text/plain

Index: 2014.1-CVE-2014-0204.patch
===================================================================
From 786af9829c5329a982e3451f77afebbfb21850bd Mon Sep 17 00:00:00 2001
From: Brant Knudson <bknudson@××××××.com>
Date: Fri, 18 Apr 2014 11:18:42 -0500
Subject: [PATCH] SQL and LDAP fixes for get_roles_for_user_and_project
 user=group ID

When there was a role assigned to a group with the same ID as a user,
the SQL and LDAP assignment backends would incorrectly return the
assignment to the group when requesting roles for the user via the
get_roles_for_user_and_project method.

With this change, assignments to a group with the same ID are not
returned for the user when calling get_roles_for_user_and_project.

Functions were added to compare DNs more accurately based on the
LDAP RFCs.

The fakeldap code was changed to normalize the values when
comparing values for checking if the values match the filter.

Co-Authored By: Nathan Kinder <nkinder@××××××.com>
Co-Authored By: Adam Young <ayoung@××××××.com>

Change-Id: Id3d6f66c995e65e37d909359420d71ecdde86b69
Closes-Bug: #1309228
---
 keystone/assignment/backends/ldap.py    |  11 +--
 keystone/assignment/backends/sql.py     |  15 +++
 keystone/common/ldap/core.py            | 110 +++++++++++++++++++++
 keystone/tests/fakeldap.py              |  17 +++-
 keystone/tests/test_backend.py          |  37 +++++++
 keystone/tests/test_backend_ldap.py     |  28 ++++++
 keystone/tests/unit/common/test_ldap.py | 169 ++++++++++++++++++++++++++++++++
 7 files changed, 378 insertions(+), 9 deletions(-)
 create mode 100644 keystone/tests/unit/common/test_ldap.py

diff --git a/keystone/assignment/backends/ldap.py b/keystone/assignment/backends/ldap.py
index 2afd339..09b0f01 100644
--- a/keystone/assignment/backends/ldap.py
+++ b/keystone/assignment/backends/ldap.py
@@ -88,24 +88,19 @@ def _get_metadata(self, user_id=None, tenant_id=None,
 
         def _get_roles_for_just_user_and_project(user_id, tenant_id):
             self.get_project(tenant_id)
+            user_dn = self.user._id_to_dn(user_id)
             return [self.role._dn_to_id(a.role_dn)
                     for a in self.role.get_role_assignments
                     (self.project._id_to_dn(tenant_id))
-                    if self.user._dn_to_id(a.user_dn) == user_id]
+                    if common_ldap.is_dn_equal(a.user_dn, user_dn)]
 
         def _get_roles_for_group_and_project(group_id, project_id):
             self.get_project(project_id)
             group_dn = self.group._id_to_dn(group_id)
-            # NOTE(marcos-fermin-lobo): In Active Directory, for functions
-            # such as "self.role.get_role_assignments", it returns
-            # the key "CN" or "OU" in uppercase.
-            # The group_dn var has "CN" and "OU" in lowercase.
-            # For this reason, it is necessary to use the "upper()"
-            # function so both are consistent.
             return [self.role._dn_to_id(a.role_dn)
                     for a in self.role.get_role_assignments
                     (self.project._id_to_dn(project_id))
-                    if a.user_dn.upper() == group_dn.upper()]
+                    if common_ldap.is_dn_equal(a.user_dn, group_dn)]
 
         if domain_id is not None:
             msg = _('Domain metadata not supported by LDAP')
diff --git a/keystone/assignment/backends/sql.py b/keystone/assignment/backends/sql.py
index 1d8c78f..b546a42 100644
--- a/keystone/assignment/backends/sql.py
+++ b/keystone/assignment/backends/sql.py
@@ -86,6 +86,21 @@ def _get_metadata(self, user_id=None, tenant_id=None,
             session = sql.get_session()
 
         q = session.query(RoleAssignment)
+
+        def _calc_assignment_type():
+            # Figure out the assignment type we're checking for from the args.
+            if user_id:
+                if tenant_id:
+                    return AssignmentType.USER_PROJECT
+                else:
+                    return AssignmentType.USER_DOMAIN
+            else:
+                if tenant_id:
+                    return AssignmentType.GROUP_PROJECT
+                else:
+                    return AssignmentType.GROUP_DOMAIN
+
+        q = q.filter_by(type=_calc_assignment_type())
         q = q.filter_by(actor_id=user_id or group_id)
         q = q.filter_by(target_id=tenant_id or domain_id)
         refs = q.all()
diff --git a/keystone/common/ldap/core.py b/keystone/common/ldap/core.py
index e8d1dc0..9561650 100644
--- a/keystone/common/ldap/core.py
+++ b/keystone/common/ldap/core.py
@@ -13,6 +13,7 @@
 # under the License.
 
 import os.path
+import re
 
 import ldap
 import ldap.filter
@@ -101,6 +102,115 @@ def ldap_scope(scope):
                 'options': ', '.join(LDAP_SCOPES.keys())})
 
 
+def prep_case_insensitive(value):
+    """Prepare a string for case-insensitive comparison.
+
+    This is defined in RFC4518. For simplicity, all this function does is
+    lowercase all the characters, strip leading and trailing whitespace,
+    and compress sequences of spaces to a single space.
+    """
+    value = re.sub(r'\s+', ' ', value.strip().lower())
+    return value
+
+
+def is_ava_value_equal(attribute_type, val1, val2):
+    """Returns True if and only if the AVAs are equal.
+
+    When comparing AVAs, the equality matching rule for the attribute type
+    should be taken into consideration. For simplicity, this implementation
+    does a case-insensitive comparison.
+
+    Note that this function uses prep_case_insenstive so the limitations of
+    that function apply here.
+
+    """
+
+    return prep_case_insensitive(val1) == prep_case_insensitive(val2)
+
+
+def is_rdn_equal(rdn1, rdn2):
+    """Returns True if and only if the RDNs are equal.
+
+    * RDNs must have the same number of AVAs.
+    * Each AVA of the RDNs must be the equal for the same attribute type. The
+      order isn't significant. Note that an attribute type will only be in one
+      AVA in an RDN, otherwise the DN wouldn't be valid.
+    * Attribute types aren't case sensitive. Note that attribute type
+      comparison is more complicated than implemented. This function only
+      compares case-insentive. The code should handle multiple names for an
+      attribute type (e.g., cn, commonName, and 2.5.4.3 are the same).
+
+    Note that this function uses is_ava_value_equal to compare AVAs so the
+    limitations of that function apply here.
+
+    """
+
+    if len(rdn1) != len(rdn2):
+        return False
+
+    for attr_type_1, val1, dummy in rdn1:
+        found = False
+        for attr_type_2, val2, dummy in rdn2:
+            if attr_type_1.lower() != attr_type_2.lower():
+                continue
+
+            found = True
+            if not is_ava_value_equal(attr_type_1, val1, val2):
+                return False
+            break
+        if not found:
+            return False
+
+    return True
+
+
+def is_dn_equal(dn1, dn2):
+    """Returns True if and only if the DNs are equal.
+
+    Two DNs are equal if they've got the same number of RDNs and if the RDNs
+    are the same at each position. See RFC4517.
+
+    Note that this function uses is_rdn_equal to compare RDNs so the
+    limitations of that function apply here.
+
+    :param dn1: Either a string DN or a DN parsed by ldap.dn.str2dn.
+    :param dn2: Either a string DN or a DN parsed by ldap.dn.str2dn.
+
+    """
+
+    if not isinstance(dn1, list):
+        dn1 = ldap.dn.str2dn(dn1)
+    if not isinstance(dn2, list):
+        dn2 = ldap.dn.str2dn(dn2)
+
+    if len(dn1) != len(dn2):
+        return False
+
+    for rdn1, rdn2 in zip(dn1, dn2):
+        if not is_rdn_equal(rdn1, rdn2):
+            return False
+    return True
+
+
+def dn_startswith(descendant_dn, dn):
+    """Returns True if and only if the descendant_dn is under the dn.
+
+    :param descendant_dn: Either a string DN or a DN parsed by ldap.dn.str2dn.
+    :param dn: Either a string DN or a DN parsed by ldap.dn.str2dn.
+
+    """
+
+    if not isinstance(descendant_dn, list):
+        descendant_dn = ldap.dn.str2dn(descendant_dn)
+    if not isinstance(dn, list):
+        dn = ldap.dn.str2dn(dn)
+
+    if len(descendant_dn) <= len(dn):
+        return False
+
+    return is_dn_equal(descendant_dn[len(dn):], dn)
+
+
 _HANDLERS = {}
 
 
diff --git a/keystone/tests/fakeldap.py b/keystone/tests/fakeldap.py
index 8347d68..21e1bd3 100644
--- a/keystone/tests/fakeldap.py
+++ b/keystone/tests/fakeldap.py
@@ -51,6 +51,19 @@ def _process_attr(attr_name, value_or_values):
 
     def normalize_dn(dn):
         # Capitalize the attribute names as an LDAP server might.
+
+        # NOTE(blk-u): Special case for this tested value, used with
+        # test_user_id_comma. The call to str2dn here isn't always correct
+        # here, because `dn` is escaped for an LDAP filter. str2dn() normally
+        # works only because there's no special characters in `dn`.
+        if dn == 'cn=Doe\\5c, John,ou=Users,cn=example,cn=com':
+            return 'CN=Doe\\, John,OU=Users,CN=example,CN=com'
+
+        # NOTE(blk-u): Another special case for this tested value. When a
+        # roleOccupant has an escaped comma, it gets converted to \2C.
+        if dn == 'cn=Doe\\, John,ou=Users,cn=example,cn=com':
+            return 'CN=Doe\\2C John,OU=Users,CN=example,CN=com'
+
         dn = ldap.dn.str2dn(dn)
         norm = []
         for part in dn:
@@ -118,7 +131,9 @@ def _match(key, value, attrs):
         str_sids = [str(x) for x in attrs[key]]
         return str(value) in str_sids
     if key != 'objectclass':
-        return _process_attr(key, value)[0] in attrs[key]
+        check_value = _process_attr(key, value)[0]
+        norm_values = list(_process_attr(key, x)[0] for x in attrs[key])
+        return check_value in norm_values
     # it is an objectclass check, so check subclasses
     values = _subs(value)
     for v in values:
diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py
index c8d7341..b42b209 100644
--- a/keystone/tests/test_backend.py
+++ b/keystone/tests/test_backend.py
@@ -1377,6 +1377,43 @@ def test_multi_group_grants_on_project_domain(self):
         self.assertIn(role_list[1]['id'], combined_role_list)
         self.assertIn(role_list[2]['id'], combined_role_list)
 
+    def test_get_roles_for_user_and_project_user_group_same_id(self):
+        """When a user has the same ID as a group,
+        get_roles_for_user_and_project returns only the roles for the user and
+        not the group.
+
+        """
+
+        # Setup: create user, group with same ID, role, and project;
+        # assign the group the role on the project.
+
+        user_group_id = uuid.uuid4().hex
+
+        user1 = {'id': user_group_id, 'name': uuid.uuid4().hex,
+                 'domain_id': DEFAULT_DOMAIN_ID, }
+        self.identity_api.create_user(user_group_id, user1)
+
+        group1 = {'id': user_group_id, 'name': uuid.uuid4().hex,
+                  'domain_id': DEFAULT_DOMAIN_ID, }
+        self.identity_api.create_group(user_group_id, group1)
+
+        role1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
+        self.assignment_api.create_role(role1['id'], role1)
+
+        project1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
+                    'domain_id': DEFAULT_DOMAIN_ID, }
+        self.assignment_api.create_project(project1['id'], project1)
+
+        self.assignment_api.create_grant(role1['id'],
+                                         group_id=user_group_id,
+                                         project_id=project1['id'])
+
+        # Check the roles, shouldn't be any since the user wasn't granted any.
+        roles = self.assignment_api.get_roles_for_user_and_project(
+            user_group_id, project1['id'])
+
+        self.assertEqual([], roles, 'role for group is %s' % role1['id'])
+
     def test_delete_role_with_user_and_group_grants(self):
         role1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
         self.assignment_api.create_role(role1['id'], role1)
diff --git a/keystone/tests/test_backend_ldap.py b/keystone/tests/test_backend_ldap.py
index 310fbbc..9964527 100644
--- a/keystone/tests/test_backend_ldap.py
+++ b/keystone/tests/test_backend_ldap.py
@@ -546,6 +546,34 @@ def test_new_arbitrary_attributes_are_returned_from_update_user(self):
     def test_updated_arbitrary_attributes_are_returned_from_update_user(self):
         self.skipTest("Using arbitrary attributes doesn't work under LDAP")
 
+    def test_user_id_comma_grants(self):
+        """Even if the user has a , in their ID, can get user and group grants.
+        """
+
+        # Create a user with a , in their ID
+        # NOTE(blk-u): the DN for this user is hard-coded in fakeldap!
+        user_id = u'Doe, John'
+        user = {
+            'id': user_id,
+            'name': self.getUniqueString(),
+            'password': self.getUniqueString(),
+            'domain_id': CONF.identity.default_domain_id,
+        }
+        self.identity_api.create_user(user_id, user)
+
+        # Grant the user a role on a project.
+
+        role_id = 'member'
+        project_id = self.tenant_baz['id']
+
+        self.assignment_api.create_grant(role_id, user_id=user_id,
+                                         project_id=project_id)
+
+        role_ref = self.assignment_api.get_grant(role_id, user_id=user_id,
+                                                 project_id=project_id)
+
+        self.assertEqual(role_id, role_ref['id'])
+
 
 class LDAPIdentity(BaseLDAPIdentity, tests.TestCase):
     def setUp(self):
diff --git a/keystone/tests/unit/common/test_ldap.py b/keystone/tests/unit/common/test_ldap.py
new file mode 100644
index 0000000..220bf1a
--- /dev/null
+++ b/keystone/tests/unit/common/test_ldap.py
@@ -0,0 +1,169 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import ldap.dn
+
+from keystone.common import ldap as ks_ldap
+from keystone import tests
+
+
+class DnCompareTest(tests.BaseTestCase):
+    """Tests for the DN comparison functions in keystone.common.ldap.core."""
+
+    def test_prep(self):
+        # prep_case_insensitive returns the string with spaces at the front and
+        # end if it's already lowercase and no insignificant characters.
+        value = 'lowercase value'
+        self.assertEqual(value, ks_ldap.prep_case_insensitive(value))
+
+    def test_prep_lowercase(self):
+        # prep_case_insensitive returns the string with spaces at the front and
+        # end and lowercases the value.
+        value = 'UPPERCASE VALUE'
+        exp_value = value.lower()
+        self.assertEqual(exp_value, ks_ldap.prep_case_insensitive(value))
+
+    def test_prep_insignificant(self):
+        # prep_case_insensitive remove insignificant spaces.
+        value = 'before   after'
+        exp_value = 'before after'
+        self.assertEqual(exp_value, ks_ldap.prep_case_insensitive(value))
+
+    def test_prep_insignificant_pre_post(self):
+        # prep_case_insensitive remove insignificant spaces.
+        value = '   value   '
+        exp_value = 'value'
+        self.assertEqual(exp_value, ks_ldap.prep_case_insensitive(value))
+
+    def test_ava_equal_same(self):
+        # is_ava_value_equal returns True if the two values are the same.
+        value = 'val1'
+        self.assertTrue(ks_ldap.is_ava_value_equal('cn', value, value))
+
+    def test_ava_equal_complex(self):
+        # is_ava_value_equal returns True if the two values are the same using
+        # a value that's got different capitalization and insignificant chars.
+        val1 = 'before   after'
+        val2 = '  BEFORE  afTer '
+        self.assertTrue(ks_ldap.is_ava_value_equal('cn', val1, val2))
+
+    def test_ava_different(self):
+        # is_ava_value_equal returns False if the values aren't the same.
+        self.assertFalse(ks_ldap.is_ava_value_equal('cn', 'val1', 'val2'))
+
+    def test_rdn_same(self):
+        # is_rdn_equal returns True if the two values are the same.
+        rdn = ldap.dn.str2dn('cn=val1')[0]
+        self.assertTrue(ks_ldap.is_rdn_equal(rdn, rdn))
+
+    def test_rdn_diff_length(self):
+        # is_rdn_equal returns False if the RDNs have a different number of
+        # AVAs.
+        rdn1 = ldap.dn.str2dn('cn=cn1')[0]
+        rdn2 = ldap.dn.str2dn('cn=cn1+ou=ou1')[0]
+        self.assertFalse(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_rdn_multi_ava_same_order(self):
+        # is_rdn_equal returns True if the RDNs have the same number of AVAs
+        # and the values are the same.
+        rdn1 = ldap.dn.str2dn('cn=cn1+ou=ou1')[0]
+        rdn2 = ldap.dn.str2dn('cn=CN1+ou=OU1')[0]
+        self.assertTrue(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_rdn_multi_ava_diff_order(self):
+        # is_rdn_equal returns True if the RDNs have the same number of AVAs
+        # and the values are the same, even if in a different order
+        rdn1 = ldap.dn.str2dn('cn=cn1+ou=ou1')[0]
+        rdn2 = ldap.dn.str2dn('ou=OU1+cn=CN1')[0]
+        self.assertTrue(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_rdn_multi_ava_diff_type(self):
+        # is_rdn_equal returns False if the RDNs have the same number of AVAs
+        # and the attribute types are different.
+        rdn1 = ldap.dn.str2dn('cn=cn1+ou=ou1')[0]
+        rdn2 = ldap.dn.str2dn('cn=cn1+sn=sn1')[0]
+        self.assertFalse(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_rdn_attr_type_case_diff(self):
+        # is_rdn_equal returns True for same RDNs even when attr type case is
+        # different.
+        rdn1 = ldap.dn.str2dn('cn=cn1')[0]
+        rdn2 = ldap.dn.str2dn('CN=cn1')[0]
+        self.assertTrue(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_rdn_attr_type_alias(self):
+        # is_rdn_equal returns False for same RDNs even when attr type alias is
+        # used. Note that this is a limitation since an LDAP server should
+        # consider them equal.
+        rdn1 = ldap.dn.str2dn('cn=cn1')[0]
+        rdn2 = ldap.dn.str2dn('2.5.4.3=cn1')[0]
+        self.assertFalse(ks_ldap.is_rdn_equal(rdn1, rdn2))
+
+    def test_dn_same(self):
+        # is_dn_equal returns True if the DNs are the same.
+        dn = 'cn=Babs Jansen,ou=OpenStack'
+        self.assertTrue(ks_ldap.is_dn_equal(dn, dn))
+
+    def test_dn_diff_length(self):
+        # is_dn_equal returns False if the DNs don't have the same number of
+        # RDNs
+        dn1 = 'cn=Babs Jansen,ou=OpenStack'
+        dn2 = 'cn=Babs Jansen,ou=OpenStack,dc=example.com'
+        self.assertFalse(ks_ldap.is_dn_equal(dn1, dn2))
+
+    def test_dn_equal_rdns(self):
+        # is_dn_equal returns True if the DNs have the same number of RDNs
+        # and each RDN is the same.
+        dn1 = 'cn=Babs Jansen,ou=OpenStack+cn=OpenSource'
+        dn2 = 'CN=Babs Jansen,cn=OpenSource+ou=OpenStack'
+        self.assertTrue(ks_ldap.is_dn_equal(dn1, dn2))
+
+    def test_dn_parsed_dns(self):
+        # is_dn_equal can also accept parsed DNs.
+        dn_str1 = ldap.dn.str2dn('cn=Babs Jansen,ou=OpenStack+cn=OpenSource')
+        dn_str2 = ldap.dn.str2dn('CN=Babs Jansen,cn=OpenSource+ou=OpenStack')
+        self.assertTrue(ks_ldap.is_dn_equal(dn_str1, dn_str2))
+
+    def test_startswith_under_child(self):
+        # dn_startswith returns True if descendant_dn is a child of dn.
+        child = 'cn=Babs Jansen,ou=OpenStack'
+        parent = 'ou=OpenStack'
+        self.assertTrue(ks_ldap.dn_startswith(child, parent))
+
+    def test_startswith_parent(self):
+        # dn_startswith returns False if descendant_dn is a parent of dn.
+        child = 'cn=Babs Jansen,ou=OpenStack'
+        parent = 'ou=OpenStack'
+        self.assertFalse(ks_ldap.dn_startswith(parent, child))
+
+    def test_startswith_same(self):
+        # dn_startswith returns False if DNs are the same.
+        dn = 'cn=Babs Jansen,ou=OpenStack'
+        self.assertFalse(ks_ldap.dn_startswith(dn, dn))
+
+    def test_startswith_not_parent(self):
+        # dn_startswith returns False if descendant_dn is not under the dn
+        child = 'cn=Babs Jansen,ou=OpenStack'
+        parent = 'dc=example.com'
+        self.assertFalse(ks_ldap.dn_startswith(child, parent))
+
+    def test_startswith_descendant(self):
+        # dn_startswith returns True if descendant_dn is a descendant of dn.
+        descendant = 'cn=Babs Jansen,ou=Keystone,ou=OpenStack,dc=example.com'
+        dn = 'ou=OpenStack,dc=example.com'
+        self.assertTrue(ks_ldap.dn_startswith(descendant, dn))
+
+    def test_startswith_parsed_dns(self):
+        # dn_startswith also accepts parsed DNs.
+        descendant = ldap.dn.str2dn('cn=Babs Jansen,ou=OpenStack')
+        dn = ldap.dn.str2dn('ou=OpenStack')
+        self.assertTrue(ks_ldap.dn_startswith(descendant, dn))
-- 
1.9.3