Blame SOURCES/0089-Ticket-49652-DENY-aci-s-are-not-handled-properly.patch

3b7e51
From 5b7d67bdef7810c661ae4ba1fdfa620c86985661 Mon Sep 17 00:00:00 2001
3b7e51
From: Mark Reynolds <mreynolds@redhat.com>
3b7e51
Date: Fri, 27 Apr 2018 08:34:51 -0400
3b7e51
Subject: [PATCH] Ticket 49652 - DENY aci's are not handled properly
3b7e51
3b7e51
Bug Description:  There are really two issues here.  One, when a resource
3b7e51
                  is denied by a DENY aci the cached results for that resource
3b7e51
                  are not proprely set, and on the same connection if the same
3b7e51
                  operation repeated it will be allowed instead of denied because
3b7e51
                  the cache result was not proprely updated.
3b7e51
3b7e51
                  Two, if there are no ALLOW aci's on a resource, then we don't
3b7e51
                  check the deny rules, and resources that are restricted are
3b7e51
                  returned to the client.
3b7e51
3b7e51
Fix Description:  For issue one, when an entry is denied access reset all the
3b7e51
                  attributes' cache results to DENIED as it's possible previously
3b7e51
                  evaluated aci's granted access to some of these attributes which
3b7e51
                  are still present in the acl result cache.
3b7e51
3b7e51
                  For issue two, if there are no ALLOW aci's on a resource but
3b7e51
                  there are DENY aci's, then set the aclpb state flags to
3b7e51
                  process DENY aci's
3b7e51
3b7e51
https://pagure.io/389-ds-base/issue/49652
3b7e51
3b7e51
Reviewed by: tbordaz & lkrispenz(Thanks!!)
3b7e51
3b7e51
(cherry picked from commit d77c7f0754f67022b42784c05be8a493a00f2ec5)
3b7e51
---
3b7e51
 dirsrvtests/tests/suites/acl/acl_deny_test.py | 198 ++++++++++++++++++
3b7e51
 ldap/servers/plugins/acl/acl.c                |  24 ++-
3b7e51
 2 files changed, 220 insertions(+), 2 deletions(-)
3b7e51
 create mode 100644 dirsrvtests/tests/suites/acl/acl_deny_test.py
3b7e51
3b7e51
diff --git a/dirsrvtests/tests/suites/acl/acl_deny_test.py b/dirsrvtests/tests/suites/acl/acl_deny_test.py
3b7e51
new file mode 100644
3b7e51
index 000000000..285664150
3b7e51
--- /dev/null
3b7e51
+++ b/dirsrvtests/tests/suites/acl/acl_deny_test.py
3b7e51
@@ -0,0 +1,198 @@
3b7e51
+import logging
3b7e51
+import pytest
3b7e51
+import os
3b7e51
+import ldap
3b7e51
+import time
3b7e51
+from lib389._constants import *
3b7e51
+from lib389.topologies import topology_st as topo
3b7e51
+from lib389.idm.user import UserAccount, UserAccounts, TEST_USER_PROPERTIES
3b7e51
+from lib389.idm.domain import Domain
3b7e51
+
3b7e51
+DEBUGGING = os.getenv("DEBUGGING", default=False)
3b7e51
+if DEBUGGING:
3b7e51
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
3b7e51
+else:
3b7e51
+    logging.getLogger(__name__).setLevel(logging.INFO)
3b7e51
+log = logging.getLogger(__name__)
3b7e51
+
3b7e51
+BIND_DN2 = 'uid=tuser,ou=People,dc=example,dc=com'
3b7e51
+BIND_RDN2 = 'tuser'
3b7e51
+BIND_DN = 'uid=tuser1,ou=People,dc=example,dc=com'
3b7e51
+BIND_RDN = 'tuser1'
3b7e51
+SRCH_FILTER = "uid=tuser1"
3b7e51
+SRCH_FILTER2 = "uid=tuser"
3b7e51
+
3b7e51
+aci_list_A = ['(targetattr != "userPassword") (version 3.0; acl "Anonymous access"; allow (read, search, compare)userdn = "ldap:///anyone";)',
3b7e51
+              '(targetattr = "*") (version 3.0;acl "allow tuser";allow (all)(userdn = "ldap:///uid=tuser5,ou=People,dc=example,dc=com");)',
3b7e51
+              '(targetattr != "uid || mail") (version 3.0; acl "deny-attrs"; deny (all) (userdn = "ldap:///anyone");)',
3b7e51
+              '(targetfilter = "(inetUserStatus=1)") ( version 3.0; acl "deny-specific-entry"; deny(all) (userdn = "ldap:///anyone");)']
3b7e51
+
3b7e51
+aci_list_B = ['(targetattr != "userPassword") (version 3.0; acl "Anonymous access"; allow (read, search, compare)userdn = "ldap:///anyone";)',
3b7e51
+              '(targetattr != "uid || mail") (version 3.0; acl "deny-attrs"; deny (all) (userdn = "ldap:///anyone");)',
3b7e51
+              '(targetfilter = "(inetUserStatus=1)") ( version 3.0; acl "deny-specific-entry"; deny(all) (userdn = "ldap:///anyone");)']
3b7e51
+
3b7e51
+
3b7e51
+@pytest.fixture(scope="module")
3b7e51
+def aci_setup(topo):
3b7e51
+    topo.standalone.log.info("Add {}".format(BIND_DN))
3b7e51
+    user = UserAccount(topo.standalone, BIND_DN)
3b7e51
+    user_props = TEST_USER_PROPERTIES.copy()
3b7e51
+    user_props.update({'sn': BIND_RDN,
3b7e51
+                       'cn': BIND_RDN,
3b7e51
+                       'uid': BIND_RDN,
3b7e51
+                       'inetUserStatus': '1',
3b7e51
+                       'objectclass': 'extensibleObject',
3b7e51
+                       'userpassword': PASSWORD})
3b7e51
+    user.create(properties=user_props, basedn=SUFFIX)
3b7e51
+
3b7e51
+    topo.standalone.log.info("Add {}".format(BIND_DN2))
3b7e51
+    user2 = UserAccount(topo.standalone, BIND_DN2)
3b7e51
+    user_props = TEST_USER_PROPERTIES.copy()
3b7e51
+    user_props.update({'sn': BIND_RDN2,
3b7e51
+                       'cn': BIND_RDN2,
3b7e51
+                       'uid': BIND_RDN2,
3b7e51
+                       'userpassword': PASSWORD})
3b7e51
+    user2.create(properties=user_props, basedn=SUFFIX)
3b7e51
+
3b7e51
+
3b7e51
+def test_multi_deny_aci(topo, aci_setup):
3b7e51
+    """Test that mutliple deny rules work, and that they the cache properly
3b7e51
+    stores the result
3b7e51
+
3b7e51
+    :id: 294c366d-850e-459e-b5a0-3cc828ec3aca
3b7e51
+    :setup: Standalone Instance
3b7e51
+    :steps:
3b7e51
+        1. Add aci_list_A aci's and verify two searches on the same connection
3b7e51
+           behave the same
3b7e51
+        2. Add aci_list_B aci's and verify search fails as expected
3b7e51
+    :expectedresults:
3b7e51
+        1. Both searches do not return any entries
3b7e51
+        2. Seaches do not return any entries
3b7e51
+    """
3b7e51
+
3b7e51
+    if DEBUGGING:
3b7e51
+        # Maybe add aci logging?
3b7e51
+        pass
3b7e51
+
3b7e51
+    suffix = Domain(topo.standalone, DEFAULT_SUFFIX)
3b7e51
+
3b7e51
+    for run in range(2):
3b7e51
+        topo.standalone.log.info("Pass " + str(run + 1))
3b7e51
+
3b7e51
+        # Test ACI List A
3b7e51
+        topo.standalone.log.info("Testing two searches behave the same...")
3b7e51
+        topo.standalone.simple_bind_s(DN_DM, PASSWORD)
3b7e51
+        suffix.set('aci', aci_list_A, ldap.MOD_REPLACE)
3b7e51
+        time.sleep(1)
3b7e51
+
3b7e51
+        topo.standalone.simple_bind_s(BIND_DN, PASSWORD)
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 1")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 2")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        # Bind a different user who has rights
3b7e51
+        topo.standalone.simple_bind_s(BIND_DN2, PASSWORD)
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user (2)")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        if run > 0:
3b7e51
+            # Second pass
3b7e51
+            topo.standalone.restart()
3b7e51
+
3b7e51
+        # Reset ACI's and do the second test
3b7e51
+        topo.standalone.log.info("Testing search does not return any entries...")
3b7e51
+        topo.standalone.simple_bind_s(DN_DM, PASSWORD)
3b7e51
+        suffix.set('aci', aci_list_B, ldap.MOD_REPLACE)
3b7e51
+        time.sleep(1)
3b7e51
+
3b7e51
+        topo.standalone.simple_bind_s(BIND_DN, PASSWORD)
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 1")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 2")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        if run > 0:
3b7e51
+            # Second pass
3b7e51
+            topo.standalone.restart()
3b7e51
+
3b7e51
+        # Bind as different user who has rights
3b7e51
+        topo.standalone.simple_bind_s(BIND_DN2, PASSWORD)
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as good user (2)")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 1")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 2")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        # back to user 1
3b7e51
+        topo.standalone.simple_bind_s(BIND_DN, PASSWORD)
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as user1")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER2)
3b7e51
+        if entries is None or len(entries) == 0:
3b7e51
+            topo.standalone.log.fatal("Failed to get entry as user1 (2)")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 1")
3b7e51
+            assert False
3b7e51
+
3b7e51
+        entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, SRCH_FILTER)
3b7e51
+        if entries and entries[0]:
3b7e51
+            topo.standalone.log.fatal("Incorrectly got an entry returned from search 2")
3b7e51
+            assert False
3b7e51
+
3b7e51
+    topo.standalone.log.info("Test PASSED")
3b7e51
+
3b7e51
+
3b7e51
+if __name__ == '__main__':
3b7e51
+    # Run isolated
3b7e51
+    # -s for DEBUG mode
3b7e51
+    CURRENT_FILE = os.path.realpath(__file__)
3b7e51
+    pytest.main(["-s", CURRENT_FILE])
3b7e51
+
3b7e51
diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c
3b7e51
index bc154c78f..6d105f4fa 100644
3b7e51
--- a/ldap/servers/plugins/acl/acl.c
3b7e51
+++ b/ldap/servers/plugins/acl/acl.c
3b7e51
@@ -1088,9 +1088,23 @@ acl_read_access_allowed_on_entry(
3b7e51
                     ** a DENY rule, then we don't have access to
3b7e51
                     ** the entry ( nice trick to get in )
3b7e51
                     */
3b7e51
-                    if (aclpb->aclpb_state &
3b7e51
-                        ACLPB_EXECUTING_DENY_HANDLES)
3b7e51
+                    if (aclpb->aclpb_state & ACLPB_EXECUTING_DENY_HANDLES) {
3b7e51
+                        aclEvalContext *c_ContextEval = &aclpb->aclpb_curr_entryEval_context;
3b7e51
+                        AclAttrEval *c_attrEval = NULL;
3b7e51
+                        /*
3b7e51
+                         * The entire entry is blocked, but previously evaluated allow aci's might
3b7e51
+                         * show some of the attributes as readable in the acl cache, so reset all
3b7e51
+                         * the cached attributes' status to FAIL.
3b7e51
+                         */
3b7e51
+                        for (size_t j = 0; j < c_ContextEval->acle_numof_attrs; j++) {
3b7e51
+                            c_attrEval = &c_ContextEval->acle_attrEval[j];
3b7e51
+                            c_attrEval->attrEval_r_status &= ~ACL_ATTREVAL_SUCCESS;
3b7e51
+                            c_attrEval->attrEval_r_status |= ACL_ATTREVAL_FAIL;
3b7e51
+                            c_attrEval->attrEval_s_status &= ~ACL_ATTREVAL_SUCCESS;
3b7e51
+                            c_attrEval->attrEval_s_status |= ACL_ATTREVAL_FAIL;
3b7e51
+                        }
3b7e51
                         return LDAP_INSUFFICIENT_ACCESS;
3b7e51
+                    }
3b7e51
 
3b7e51
                     /* The other case is I don't have an
3b7e51
                     ** explicit allow rule -- which is fine.
3b7e51
@@ -2908,6 +2922,12 @@ acl__TestRights(Acl_PBlock *aclpb, int access, const char **right, const char **
3b7e51
         result_reason->deciding_aci = NULL;
3b7e51
         result_reason->reason = ACL_REASON_NO_MATCHED_RESOURCE_ALLOWS;
3b7e51
 
3b7e51
+        /* If we have deny handles we should process them */
3b7e51
+        if (aclpb->aclpb_num_deny_handles > 0) {
3b7e51
+            aclpb->aclpb_state &= ~ACLPB_EXECUTING_ALLOW_HANDLES;
3b7e51
+            aclpb->aclpb_state |= ACLPB_EXECUTING_DENY_HANDLES;
3b7e51
+        }
3b7e51
+
3b7e51
         TNF_PROBE_1_DEBUG(acl__TestRights_end, "ACL", "",
3b7e51
                           tnf_string, no_allows, "");
3b7e51
 
3b7e51
-- 
3b7e51
2.17.0
3b7e51