From 205bce153c7db8258a8a28498cf54e7374dca588 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz <tbordaz@redhat.com>

Date: Tue, 14 Apr 2015 16:24:44 +0200

Subject: [PATCH] CVE-2015-1854 389ds-base: access control bypass with modrdn

Bug Description:

	47553 fix checks the write right access only if the RDN is

	modified. This allows to rename entries even if the

	authenticated user is not allowed of that.

Fix Description:

	Roll back a wrong optimization that tested the write access

	only if RDN value was changed.

https://fedorahosted.org/389/ticket/47553

Reviewed by: ?

Platforms tested: F17 (upstream test)

Flag Day: no

Doc impact: no

(cherry picked from commit 44e5c0998bdf7dcb167e8472713ff393b776e4e3)

Conflicts:

	dirsrvtests/tickets/ticket47553_single_aci_test.py

---

 dirsrvtests/tickets/ticket47553_rdn_write_test.py  | 132 +++++++++++++++++++++

 dirsrvtests/tickets/ticket47553_single_aci_test.py |  52 ++++++--

 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c         |  32 ++---

 3 files changed, 184 insertions(+), 32 deletions(-)

 create mode 100644 dirsrvtests/tickets/ticket47553_rdn_write_test.py

diff --git a/dirsrvtests/tickets/ticket47553_rdn_write_test.py b/dirsrvtests/tickets/ticket47553_rdn_write_test.py

new file mode 100644

index 0000000..f15d9b3

--- /dev/null

+++ b/dirsrvtests/tickets/ticket47553_rdn_write_test.py

@@ -0,0 +1,132 @@

+import os

+import sys

+import time

+import ldap

+import logging

+import pytest

+from lib389 import DirSrv, Entry, tools, tasks

+from lib389.tools import DirSrvTools

+from lib389._constants import *

+from lib389.properties import *

+from lib389.tasks import *

+from lib389.utils import *

+from ldap.controls.simple import GetEffectiveRightsControl

+

+logging.getLogger(__name__).setLevel(logging.DEBUG)

+log = logging.getLogger(__name__)

+

+installation1_prefix = None

+

+SRC_ENTRY_CN = "tuser"

+EXT_RDN = "01"

+DST_ENTRY_CN = SRC_ENTRY_CN + EXT_RDN

+

+SRC_ENTRY_DN = "cn=%s,%s" % (SRC_ENTRY_CN, SUFFIX)

+DST_ENTRY_DN = "cn=%s,%s" % (DST_ENTRY_CN, SUFFIX)

+

+class TopologyStandalone(object):

+    def __init__(self, standalone):

+        standalone.open()

+        self.standalone = standalone

+

+

+#@pytest.fixture(scope="module")

+def topology(request):

+    global installation1_prefix

+

+    # Creating standalone instance ...

+    standalone = DirSrv(verbose=False)

+    if installation1_prefix:

+        args_instance[SER_DEPLOYED_DIR] = installation1_prefix

+    args_instance[SER_HOST] = HOST_STANDALONE

+    args_instance[SER_PORT] = PORT_STANDALONE

+    args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE

+    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX

+    args_standalone = args_instance.copy()

+    standalone.allocate(args_standalone)

+    instance_standalone = standalone.exists()

+    if instance_standalone:

+        standalone.delete()

+    standalone.create()

+    standalone.open()

+

+    # Clear out the tmp dir

+    standalone.clearTmpDir(__file__)

+

+    return TopologyStandalone(standalone)

+

+def test_ticket47553_rdn_write_init(topology):

+    topology.standalone.log.info("\n\n######################### Add entry tuser ######################\n")

+    topology.standalone.add_s(Entry((SRC_ENTRY_DN, {

+                                                'objectclass': "top person".split(),

+                                                'sn': SRC_ENTRY_CN,

+                                                'cn': SRC_ENTRY_CN})))

+

+def test_ticket47553_rdn_write_get_ger(topology):

+    ANONYMOUS_DN = ""

+    topology.standalone.log.info("\n\n######################### GER rights for anonymous ######################\n")

+    request_ctrl = GetEffectiveRightsControl(criticality=True, authzId="dn:" + ANONYMOUS_DN)

+    msg_id = topology.standalone.search_ext(SUFFIX, ldap.SCOPE_SUBTREE, "objectclass=*", serverctrls=[request_ctrl])

+    rtype, rdata, rmsgid, response_ctrl = topology.standalone.result3(msg_id)

+    value = ''

+    for dn, attrs in rdata:

+        topology.standalone.log.info("dn: %s" % dn)

+        for value in attrs['entryLevelRights']:

+            topology.standalone.log.info("###############  entryLevelRights: %r" % value)

+            assert 'n' not in value

+

+def test_ticket47553_rdn_write_modrdn_anonymous(topology):

+    ANONYMOUS_DN = ""

+    topology.standalone.close()

+    topology.standalone.binddn = ANONYMOUS_DN

+    topology.standalone.open()

+    msg_id = topology.standalone.search_ext("", ldap.SCOPE_BASE, "objectclass=*")

+    rtype, rdata, rmsgid, response_ctrl = topology.standalone.result3(msg_id)

+    value = ''

+    for dn, attrs in rdata:

+        topology.standalone.log.info("dn: %s" % dn)

+        for attr in attrs:

+            topology.standalone.log.info("###############  %r: %r" % (attr, attrs[attr]))

+

+

+    try:

+        topology.standalone.rename_s(SRC_ENTRY_DN, "cn=%s" % DST_ENTRY_CN, delold=True)

+    except Exception as e:

+        topology.standalone.log.info("Exception (expected): %s" % type(e).__name__)

+        isinstance(e, ldap.INSUFFICIENT_ACCESS)

+

+    try:

+        topology.standalone.getEntry(DST_ENTRY_DN, ldap.SCOPE_BASE, "objectclass=*")

+        assert False

+    except Exception as e:

+        topology.standalone.log.info("The entry was not renamed (expected)")

+        isinstance(e, ldap.NO_SUCH_OBJECT)

+

+def test_ticket47553_rdn_write(topology):

+    '''

+    Write your testcase here...

+    '''

+

+    log.info('Test complete')

+

+

+def test_ticket47553_rdn_write_final(topology):

+    topology.standalone.delete()

+    log.info('Testcase PASSED')

+

+

+def run_isolated():

+    global installation1_prefix

+    installation1_prefix = '/home/tbordaz/install_master'

+

+    topo = topology(True)

+    test_ticket47553_rdn_write_init(topo)

+    test_ticket47553_rdn_write_get_ger(topo)

+    test_ticket47553_rdn_write(topo)

+    test_ticket47553_rdn_write_modrdn_anonymous(topo)

+    test_ticket47553_rdn_write_final(topo)

+

+

+if __name__ == '__main__':

+    run_isolated()

+

diff --git a/dirsrvtests/tickets/ticket47553_single_aci_test.py b/dirsrvtests/tickets/ticket47553_single_aci_test.py

index 4be2470..0c8d7e9 100644

--- a/dirsrvtests/tickets/ticket47553_single_aci_test.py

+++ b/dirsrvtests/tickets/ticket47553_single_aci_test.py

@@ -276,7 +276,27 @@ def _moddn_aci_deny_tree(topology, mod_type=None, target_from=STAGING_DN, target

     #topology.master1.modify_s(SUFFIX, mod)

     topology.master1.log.info("Add a DENY aci under %s " % PROD_EXCEPT_DN)

     topology.master1.modify_s(PROD_EXCEPT_DN, mod)

-    

+

+def _write_aci_staging(topology, mod_type=None):

+    assert mod_type is not None

+

+    ACI_TARGET = "(targetattr= \"cn\")(target=\"ldap:///cn=*,%s\")" % STAGING_DN

+    ACI_ALLOW        = "(version 3.0; acl \"write staging entries\"; allow (write)"

+    ACI_SUBJECT      = " userdn = \"ldap:///%s\";)" % BIND_DN

+    ACI_BODY         = ACI_TARGET + ACI_ALLOW + ACI_SUBJECT

+    mod = [(mod_type, 'aci', ACI_BODY)]

+    topology.master1.modify_s(SUFFIX, mod)

+

+def _write_aci_production(topology, mod_type=None):

+    assert mod_type is not None

+

+    ACI_TARGET = "(targetattr= \"cn\")(target=\"ldap:///cn=*,%s\")" % PRODUCTION_DN

+    ACI_ALLOW        = "(version 3.0; acl \"write production entries\"; allow (write)"

+    ACI_SUBJECT      = " userdn = \"ldap:///%s\";)" % BIND_DN

+    ACI_BODY         = ACI_TARGET + ACI_ALLOW + ACI_SUBJECT

+    mod = [(mod_type, 'aci', ACI_BODY)]

+    topology.master1.modify_s(SUFFIX, mod)

+

 def _moddn_aci_staging_to_production(topology, mod_type=None, target_from=STAGING_DN, target_to=PRODUCTION_DN):

     assert mod_type != None

@@ -293,6 +313,8 @@ def _moddn_aci_staging_to_production(topology, mod_type=None, target_from=STAGIN

     ACI_BODY         = ACI_TARGET_FROM + ACI_TARGET_TO + ACI_ALLOW + ACI_SUBJECT

     mod = [(mod_type, 'aci', ACI_BODY)]

     topology.master1.modify_s(SUFFIX, mod)

+    

+    _write_aci_staging(topology, mod_type=mod_type)

 def _moddn_aci_from_production_to_staging(topology, mod_type=None):

     assert mod_type != None

@@ -303,6 +325,8 @@ def _moddn_aci_from_production_to_staging(topology, mod_type=None):

     ACI_BODY         = ACI_TARGET + ACI_ALLOW + ACI_SUBJECT

     mod = [(mod_type, 'aci', ACI_BODY)]

     topology.master1.modify_s(SUFFIX, mod)

+    

+    _write_aci_production(topology, mod_type=mod_type)

 def test_ticket47553_init(topology):

@@ -347,12 +371,9 @@ def test_ticket47553_init(topology):

                                             'description': "production except DIT"})))

     # enable acl error logging

-    #mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', '128')]

-    #topology.master1.modify_s(DN_CONFIG, mod)

-    #topology.master2.modify_s(DN_CONFIG, mod)

-    

-

-    

+    mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', str(128+262144))]

+    topology.master1.modify_s(DN_CONFIG, mod)

+    topology.master2.modify_s(DN_CONFIG, mod)

     # add dummy entries in the staging DIT

@@ -883,6 +904,7 @@ def test_ticket47553_moddn_staging_prod_9(topology):

     _bind_manager(topology)

     mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)]

     topology.master1.modify_s(PRODUCTION_DN, mod)

+    _write_aci_staging(topology, mod_type=ldap.MOD_ADD)

     _bind_normal(topology)

     topology.master1.log.info("Try to MODDN %s -> %s,%s" % (old_dn, new_rdn, new_superior))

@@ -891,6 +913,7 @@ def test_ticket47553_moddn_staging_prod_9(topology):

     _bind_manager(topology)

     mod = [(ldap.MOD_DELETE, 'aci', ACI_BODY)]

     topology.master1.modify_s(PRODUCTION_DN, mod)

+    _write_aci_staging(topology, mod_type=ldap.MOD_DELETE)

     _bind_normal(topology)

@@ -934,6 +957,7 @@ def test_ticket47553_moddn_staging_prod_9(topology):

     _bind_manager(topology)

     mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)]

     topology.master1.modify_s(PRODUCTION_DN, mod)

+    _write_aci_staging(topology, mod_type=ldap.MOD_ADD)

     _bind_normal(topology)

     try:

@@ -949,6 +973,7 @@ def test_ticket47553_moddn_staging_prod_9(topology):

     _bind_manager(topology)

     mod = [(ldap.MOD_DELETE, 'aci', ACI_BODY)]

     topology.master1.modify_s(PRODUCTION_DN, mod)

+    _write_aci_staging(topology, mod_type=ldap.MOD_DELETE)

     _bind_normal(topology)

     # Add the moddn aci that will be evaluated because of the config flag

@@ -1009,7 +1034,12 @@ def test_ticket47553_moddn_prod_staging(topology):

     old_dn  = "%s,%s" % (old_rdn, PRODUCTION_DN)

     new_rdn = old_rdn

     new_superior = STAGING_DN

-    

+

+    # add the write right because we want to check the moddn

+    _bind_manager(topology)

+    _write_aci_production(topology, mod_type=ldap.MOD_ADD)

+    _bind_normal(topology)

+

     try:

         topology.master1.log.info("Try to move back MODDN %s -> %s,%s" % (old_dn, new_rdn, new_superior))

         topology.master1.rename_s(old_dn, new_rdn, newsuperior=new_superior)

@@ -1019,7 +1049,11 @@ def test_ticket47553_moddn_prod_staging(topology):

     except Exception as e:

         topology.master1.log.info("Exception (expected): %s" % type(e).__name__)

         assert isinstance(e, ldap.INSUFFICIENT_ACCESS)

-    

+

+    _bind_manager(topology)

+    _write_aci_production(topology, mod_type=ldap.MOD_DELETE)

+    _bind_normal(topology)

+

     # successfull MOD with the both ACI

     _bind_manager(topology)

     _moddn_aci_staging_to_production(topology, mod_type=ldap.MOD_DELETE, target_from=STAGING_DN, target_to=PRODUCTION_DN)

diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

index 6a4982c..4129318 100644

--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

@@ -677,31 +677,17 @@ ldbm_back_modrdn( Slapi_PBlock *pb )

             /* JCMACL - Should be performed before the child check. */

             /* JCMACL - Why is the check performed against the copy, rather than the existing entry? */

+            /* This check must be performed even if the entry is renamed with its own name 

+             * No optimization here we need to check we have the write access to the target entry

+             */

+            ldap_result_code = plugin_call_acl_plugin(pb, ec->ep_entry,

+                    NULL /*attr*/, NULL /*value*/, SLAPI_ACL_WRITE,

+                    ACLPLUGIN_ACCESS_MODRDN, &errbuf);

+            if (ldap_result_code != LDAP_SUCCESS)

             {

-                Slapi_RDN *new_rdn;

-                Slapi_RDN *old_rdn;

-

-                /* Taken from the entry */

-                old_rdn = slapi_entry_get_srdn(ec->ep_entry);

-

-                /* Taken from the request */

-                new_rdn = slapi_rdn_new();

-                slapi_sdn_get_rdn(&dn_newrdn, new_rdn);

-

-                /* Only if we change the RDN value, we need the write access to the entry */

-                if (slapi_rdn_compare(old_rdn, new_rdn)) {

-                        ldap_result_code = plugin_call_acl_plugin(pb, ec->ep_entry,

-                                NULL /*attr*/, NULL /*value*/, SLAPI_ACL_WRITE,

-                                ACLPLUGIN_ACCESS_MODRDN, &errbuf);

-                }

-

-                slapi_rdn_free(&new_rdn);

+                goto error_return;

+            }

-                if (ldap_result_code != LDAP_SUCCESS) {

-                        goto error_return;

-                }

-            } 

-        

             /* Set the new dn to the copy of the entry */

             slapi_entry_set_sdn( ec->ep_entry, &dn_newdn );

             if (entryrdn_get_switch()) { /* subtree-rename: on */

-- 

1.9.3