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