From 2923940ffa0db88df986dd00d74ad812ccd71188 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Wed, 20 Jan 2021 16:42:15 -0500

Subject: [PATCH 2/4] Issue 5442 - Search results are different between RHDS10

 and RHDS11

Bug Description:  In 1.4.x we introduced a change that was overly strict about

                  how a search on a non-existent subtree returned its error code.

                  It was changed from returning an error 32 to an error 0 with

                  zero entries returned.

Fix Description:  When finding the entry and processing acl's make sure to

                  gather the aci's that match the resource even if the resource

                  does not exist.  This requires some extra checks when processing

                  the target attribute.

relates: https://github.com/389ds/389-ds-base/issues/4542

Reviewed by: firstyear, elkris, and tbordaz (Thanks!)

Apply Thierry's changes

round 2

Apply more suggestions from Thierry

---

 dirsrvtests/tests/suites/acl/misc_test.py | 108 +++++++-

 ldap/servers/plugins/acl/acl.c            | 296 ++++++++++------------

 ldap/servers/slapd/back-ldbm/findentry.c  |   6 +-

 src/lib389/lib389/_mapped_object.py       |   4 +-

 4 files changed, 239 insertions(+), 175 deletions(-)

diff --git a/dirsrvtests/tests/suites/acl/misc_test.py b/dirsrvtests/tests/suites/acl/misc_test.py

index 8f122b7a7..b64961c0c 100644

--- a/dirsrvtests/tests/suites/acl/misc_test.py

+++ b/dirsrvtests/tests/suites/acl/misc_test.py

@@ -11,7 +11,7 @@

 import os

 import pytest

-from lib389._constants import DEFAULT_SUFFIX, PW_DM

+from lib389._constants import DEFAULT_SUFFIX, PW_DM, DN_DM

 from lib389.idm.user import UserAccount, UserAccounts

 from lib389._mapped_object import DSLdapObject

 from lib389.idm.account import Accounts, Anonymous

@@ -399,14 +399,112 @@ def test_do_bind_as_201_distinct_users(topo, clean, aci_of_user):

         user = uas.create_test_user(uid=i, gid=i)

         user.set('userPassword', PW_DM)

-    for i in range(len(uas.list())):

-        uas.list()[i].bind(PW_DM)

+    users = uas.list()

+    for user in users:

+        user.bind(PW_DM)

     ACLPlugin(topo.standalone).replace("nsslapd-aclpb-max-selected-acls", '220')

     topo.standalone.restart()

-    for i in range(len(uas.list())):

-        uas.list()[i].bind(PW_DM)

+    users = uas.list()

+    for user in users:

+        user.bind(PW_DM)

+

+

+def test_info_disclosure(request, topo):

+    """Test that a search returns 32 when base entry does not exist

+

+    :id: f6dec4c2-65a3-41e4-a4c0-146196863333

+    :setup: Standalone Instance

+    :steps:

+        1. Add aci

+        2. Add test user

+        3. Bind as user and search for non-existent entry

+    :expectedresults:

+        1. Success

+        2. Success

+        3. Error 32 is returned

+    """

+

+    ACI_TARGET = "(targetattr = \"*\")(target = \"ldap:///%s\")" % (DEFAULT_SUFFIX)

+    ACI_ALLOW = "(version 3.0; acl \"Read/Search permission for all users\"; allow (read,search)"

+    ACI_SUBJECT = "(userdn=\"ldap:///all\");)"

+    ACI = ACI_TARGET + ACI_ALLOW + ACI_SUBJECT

+

+    # Get current ACi's so we can restore them when we are done

+    suffix = Domain(topo.standalone, DEFAULT_SUFFIX)

+    preserved_acis = suffix.get_attr_vals_utf8('aci')

+

+    def finofaci():

+        domain = Domain(topo.standalone, DEFAULT_SUFFIX)

+        try:

+            domain.remove_all('aci')

+            domain.replace_values('aci', preserved_acis)

+        except:

+            pass

+    request.addfinalizer(finofaci)

+

+    # Remove aci's

+    suffix.remove_all('aci')

+

+    # Add test user

+    USER_DN = "uid=test,ou=people," + DEFAULT_SUFFIX

+    users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)

+    users.create(properties={

+        'uid': 'test',

+        'cn': 'test',

+        'sn': 'test',

+        'uidNumber': '1000',

+        'gidNumber': '2000',

+        'homeDirectory': '/home/test',

+        'userPassword': PW_DM

+    })

+

+    # bind as user

+    conn = UserAccount(topo.standalone, USER_DN).bind(PW_DM)

+

+    # Search fo existing base DN

+    test = Domain(conn, DEFAULT_SUFFIX)

+    try:

+        test.get_attr_vals_utf8_l('dc')

+        assert False

+    except IndexError:

+        pass

+

+    # Search for a non existent bases

+    subtree = Domain(conn, "ou=does_not_exist," + DEFAULT_SUFFIX)

+    try:

+        subtree.get_attr_vals_utf8_l('objectclass')

+    except IndexError:

+        pass

+    subtree = Domain(conn, "ou=also does not exist,ou=does_not_exist," + DEFAULT_SUFFIX)

+    try:

+        subtree.get_attr_vals_utf8_l('objectclass')

+    except IndexError:

+        pass

+    # Try ONE level search instead of BASE

+    try:

+        Accounts(conn, "ou=does_not_exist," + DEFAULT_SUFFIX).filter("(objectclass=top)", ldap.SCOPE_ONELEVEL)

+    except IndexError:

+        pass

+

+    # add aci

+    suffix.add('aci', ACI)

+

+    # Search for a non existent entry which should raise an exception

+    with pytest.raises(ldap.NO_SUCH_OBJECT):

+        conn = UserAccount(topo.standalone, USER_DN).bind(PW_DM)

+        subtree = Domain(conn, "ou=does_not_exist," + DEFAULT_SUFFIX)

+        subtree.get_attr_vals_utf8_l('objectclass')

+    with pytest.raises(ldap.NO_SUCH_OBJECT):

+        conn = UserAccount(topo.standalone, USER_DN).bind(PW_DM)

+        subtree = Domain(conn, "ou=also does not exist,ou=does_not_exist," + DEFAULT_SUFFIX)

+        subtree.get_attr_vals_utf8_l('objectclass')

+    with pytest.raises(ldap.NO_SUCH_OBJECT):

+        conn = UserAccount(topo.standalone, USER_DN).bind(PW_DM)

+        DN = "ou=also does not exist,ou=does_not_exist," + DEFAULT_SUFFIX

+        Accounts(conn, DN).filter("(objectclass=top)", ldap.SCOPE_ONELEVEL, strict=True)

+

 if __name__ == "__main__":

diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c

index 41a909a18..4e811f73a 100644

--- a/ldap/servers/plugins/acl/acl.c

+++ b/ldap/servers/plugins/acl/acl.c

@@ -2111,10 +2111,11 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     aci_right = aci->aci_access;

     res_right = aclpb->aclpb_access;

     if (!(aci_right & res_right)) {

-        /* If we are looking for read/search and the acl has read/search

-        ** then go further because if targets match we may keep that

-        ** acl in  the entry cache list.

-        */

+        /*

+         * If we are looking for read/search and the acl has read/search

+         * then go further because if targets match we may keep that

+         * acl in the entry cache list.

+         */

         if (!((res_right & (SLAPI_ACL_SEARCH | SLAPI_ACL_READ)) &&

               (aci_right & (SLAPI_ACL_SEARCH | SLAPI_ACL_READ)))) {

             matches = ACL_FALSE;

@@ -2122,30 +2123,29 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

         }

     }

-

-    /* first Let's see if the entry is under the subtree where the

-    ** ACL resides. We can't let somebody affect a target beyond the

-    ** scope of where the ACL resides

-    ** Example: ACL is located in "ou=engineering, o=ace industry, c=us

-    ** but if the target is "o=ace industry, c=us", then we are in trouble.

-    **

-    ** If the aci is in the rootdse and the entry is not, then we do not

-    ** match--ie. acis in the rootdse do NOT apply below...for the moment.

-    **

-    */

+    /*

+     * First Let's see if the entry is under the subtree where the

+     * ACL resides. We can't let somebody affect a target beyond the

+     * scope of where the ACL resides

+     * Example: ACL is located in "ou=engineering, o=ace industry, c=us

+     * but if the target is "o=ace industry, c=us", then we are in trouble.

+     *

+     * If the aci is in the rootdse and the entry is not, then we do not

+     * match--ie. acis in the rootdse do NOT apply below...for the moment.

+     */

     res_ndn = slapi_sdn_get_ndn(aclpb->aclpb_curr_entry_sdn);

     aci_ndn = slapi_sdn_get_ndn(aci->aci_sdn);

-    if (!slapi_sdn_issuffix(aclpb->aclpb_curr_entry_sdn, aci->aci_sdn) || (!slapi_is_rootdse(res_ndn) && slapi_is_rootdse(aci_ndn))) {

-

-        /* cant' poke around */

+    if (!slapi_sdn_issuffix(aclpb->aclpb_curr_entry_sdn, aci->aci_sdn) ||

+        (!slapi_is_rootdse(res_ndn) && slapi_is_rootdse(aci_ndn)))

+    {

+        /* can't poke around */

         matches = ACL_FALSE;

         goto acl__resource_match_aci_EXIT;

     }

     /*

-    ** We have a single ACI which we need to find if it applies to

-    ** the resource or not.

-    */

+     * We have a single ACI which we need to find if it applies to the resource or not.

+     */

     if ((aci->aci_type & ACI_TARGET_DN) && (aclpb->aclpb_curr_entry_sdn)) {

         char *avaType;

         struct berval *avaValue;

@@ -2173,25 +2173,23 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

         char *avaType;

         struct berval *avaValue;

         char logbuf[1024];

-

-        /* We are evaluating the moddn permission.

-                 * The aci contains target_to and target_from

-                 *

-                 * target_to filter must be checked against the resource ndn that was stored in

-                 * aclpb->aclpb_curr_entry_sdn

-                 *

-                 * target_from filter must be check against the entry ndn that is in aclpb->aclpb_moddn_source_sdn

-                 * (sdn was stored in the pblock)

-                 */

+        /*

+         * We are evaluating the moddn permission.

+         * The aci contains target_to and target_from

+         *

+         * target_to filter must be checked against the resource ndn that was stored in

+         * aclpb->aclpb_curr_entry_sdn

+         *

+         * target_from filter must be check against the entry ndn that is in aclpb->aclpb_moddn_source_sdn

+         * (sdn was stored in the pblock)

+         */

         if (aci->target_to) {

             f = aci->target_to;

             dn_matched = ACL_TRUE;

             /* Now check if the filter is a simple or substring filter */

             if (aci->aci_type & ACI_TARGET_MODDN_TO_PATTERN) {

-                /* This is a filter with substring

-                     * e.g. ldap:///uid=*,cn=accounts,dc=example,dc=com

-                     */

+                /* This is a filter with substring e.g. ldap:///uid=*,cn=accounts,dc=example,dc=com */

                 slapi_log_err(SLAPI_LOG_ACL, plugin_name, "acl__resource_match_aci - moddn target_to substring: %s\n",

                               slapi_filter_to_string(f, logbuf, sizeof(logbuf)));

                 if ((rv = acl_match_substring(f, (char *)res_ndn, 0 /* match suffix */)) != ACL_TRUE) {

@@ -2204,9 +2202,7 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                     }

                 }

             } else {

-                /* This is a filter without substring

-                     * e.g. ldap:///cn=accounts,dc=example,dc=com

-                     */

+                /* This is a filter without substring  e.g. ldap:///cn=accounts,dc=example,dc=com */

                 slapi_log_err(SLAPI_LOG_ACL, plugin_name, "acl__resource_match_aci - moddn target_to: %s\n",

                               slapi_filter_to_string(f, logbuf, sizeof(logbuf)));

                 slapi_filter_get_ava(f, &avaType, &avaValue);

@@ -2230,8 +2226,8 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

             /* Now check if the filter is a simple or substring filter */

             if (aci->aci_type & ACI_TARGET_MODDN_FROM_PATTERN) {

                 /* This is a filter with substring

-                         * e.g. ldap:///uid=*,cn=accounts,dc=example,dc=com

-                         */

+                 * e.g. ldap:///uid=*,cn=accounts,dc=example,dc=com

+                 */

                 slapi_log_err(SLAPI_LOG_ACL, plugin_name, "acl__resource_match_aci - moddn target_from substring: %s\n",

                               slapi_filter_to_string(f, logbuf, sizeof(logbuf)));

                 if ((rv = acl_match_substring(f, (char *)slapi_sdn_get_dn(aclpb->aclpb_moddn_source_sdn), 0 /* match suffix */)) != ACL_TRUE) {

@@ -2243,11 +2239,8 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                         goto acl__resource_match_aci_EXIT;

                     }

                 }

-

             } else {

-                /* This is a filter without substring

-                         * e.g. ldap:///cn=accounts,dc=example,dc=com

-                         */

+                /* This is a filter without substring  e.g. ldap:///cn=accounts,dc=example,dc=com */

                 slapi_log_err(SLAPI_LOG_ACL, plugin_name, "acl__resource_match_aci - moddn target_from: %s\n",

                               slapi_filter_to_string(f, logbuf, sizeof(logbuf)));

                 if (!slapi_dn_issuffix(slapi_sdn_get_dn(aclpb->aclpb_moddn_source_sdn), avaValue->bv_val)) {

@@ -2269,10 +2262,8 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     }

     if (aci->aci_type & ACI_TARGET_PATTERN) {

-

         f = aci->target;

         dn_matched = ACL_TRUE;

-

         if ((rv = acl_match_substring(f, (char *)res_ndn, 0 /* match suffux */)) != ACL_TRUE) {

             dn_matched = ACL_FALSE;

             if (rv == ACL_ERR) {

@@ -2296,7 +2287,7 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     /*

      * Is it a (target="ldap://cn=*,($dn),o=sun.com") kind of thing.

-    */

+     */

     if (aci->aci_type & ACI_TARGET_MACRO_DN) {

         /*

          * See if the ($dn) component matches the string and

@@ -2306,8 +2297,7 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

          * entry is the same one don't recalculate it--

          * this flag only works for search right now, could

          * also optimise for mods by making it work for mods.

-        */

-

+         */

         if ((aclpb->aclpb_res_type & ACLPB_NEW_ENTRY) == 0) {

             /*

              * Here same entry so just look up the matched value,

@@ -2356,8 +2346,7 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                  * If there is already an entry for this aci in this

                  * aclpb then remove it--it's an old value for a

                  * different entry.

-                */

-

+                 */

                 acl_ht_add_and_freeOld(aclpb->aclpb_macro_ht,

                                        (PLHashNumber)aci->aci_index,

                                        matched_val);

@@ -2381,30 +2370,27 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     }

     /*

-    ** Here, if there's a targetfilter field, see if it matches.

-    **

-    ** The commented out code below was an erroneous attempt to skip

-    ** this test.  It is wrong because: 1. you need to store

-    ** whether the last test matched or not (you cannot just assume it did)

-    ** and 2. It may not be the same aci, so the previous matched

-    ** value is a function of the aci.

-    ** May be interesting to build such a cache...but no evidence for

-    ** for that right now. See Bug 383424.

-    **

-    **

-    **   && ((aclpb->aclpb_state & ACLPB_SEARCH_BASED_ON_LIST) ||

-    **    (aclpb->aclpb_res_type & ACLPB_NEW_ENTRY))

-    */

+     * Here, if there's a targetfilter field, see if it matches.

+     *

+     * The commented out code below was an erroneous attempt to skip

+     * this test.  It is wrong because: 1. you need to store

+     * whether the last test matched or not (you cannot just assume it did)

+     * and 2. It may not be the same aci, so the previous matched

+     * value is a function of the aci.

+     * May be interesting to build such a cache...but no evidence for

+     * for that right now. See Bug 383424.

+     *

+     *

+     *   && ((aclpb->aclpb_state & ACLPB_SEARCH_BASED_ON_LIST) ||

+     *    (aclpb->aclpb_res_type & ACLPB_NEW_ENTRY))

+     */

     if (aci->aci_type & ACI_TARGET_FILTER) {

         int filter_matched = ACL_TRUE;

-

         /*

          * Check for macros.

          * For targetfilter we need to fake the lasinfo structure--it's

          * created "naturally" for subjects but not targets.

-        */

-

-

+         */

         if (aci->aci_type & ACI_TARGET_FILTER_MACRO_DN) {

             lasInfo *lasinfo = NULL;

@@ -2419,11 +2405,9 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                                                     ACL_EVAL_TARGET_FILTER);

             slapi_ch_free((void **)&lasinfo);

         } else {

-

-

             if (slapi_vattr_filter_test(NULL, aclpb->aclpb_curr_entry,

                                         aci->targetFilter,

-                                        0 /*don't do acess chk*/) != 0) {

+                                        0 /*don't do access check*/) != 0) {

                 filter_matched = ACL_FALSE;

             }

         }

@@ -2450,7 +2434,7 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

      * Check to see if we need to evaluate any targetattrfilters.

      * They look as follows:

      * (targetattrfilters="add=sn:(sn=rob) && gn:(gn!=byrne),

-     *                       del=sn:(sn=rob) && gn:(gn=byrne)")

+     *                     del=sn:(sn=rob) && gn:(gn=byrne)")

      *

      * For ADD/DELETE:

      * If theres's a targetattrfilter then each add/del filter

@@ -2458,29 +2442,25 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

      * by each value of the attribute in the entry.

      *

      * For MODIFY:

-     *    If there's a targetattrfilter then the add/del filter

+     * If there's a targetattrfilter then the add/del filter

      * must be satisfied by the attribute to be added/deleted.

      * (MODIFY acl is evaluated one value at a time).

      *

      *

-    */

-

+     */

     if (((aclpb->aclpb_access & SLAPI_ACL_ADD) &&

          (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||

         ((aclpb->aclpb_access & SLAPI_ACL_DELETE) &&

-         (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS))) {

-

+         (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)))

+    {

         Targetattrfilter **attrFilterArray = NULL;

-

         Targetattrfilter *attrFilter = NULL;

-

         Slapi_Attr *attr_ptr = NULL;

         Slapi_Value *sval;

         const struct berval *attrVal;

         int k;

         int done;

-

         if ((aclpb->aclpb_access & SLAPI_ACL_ADD) &&

             (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) {

@@ -2497,28 +2477,20 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

         while (attrFilterArray && attrFilterArray[num_attrs] && attr_matched) {

             attrFilter = attrFilterArray[num_attrs];

-

             /*

-                 * If this filter applies to an attribute in the entry,

-                 * apply it to the entry.

-                 * Otherwise just ignore it.

-                 *

-                */

-

-            if (slapi_entry_attr_find(aclpb->aclpb_curr_entry,

-                                      attrFilter->attr_str,

-                                      &attr_ptr) == 0) {

-

+             * If this filter applies to an attribute in the entry,

+             * apply it to the entry.

+             * Otherwise just ignore it.

+             *

+             */

+            if (slapi_entry_attr_find(aclpb->aclpb_curr_entry, attrFilter->attr_str, &attr_ptr) == 0) {

                 /*

-                     * This is an applicable filter.

-                     *  The filter is to be appplied to the entry being added

-                     * or deleted.

-                     * The filter needs to be satisfied by _each_ occurence

-                     * of the attribute in the entry--otherwise you

-                     * could satisfy the filter and then put loads of other

-                     * values in on the back of it.

-                     */

-

+                 * This is an applicable filter.

+                 * The filter is to be applied to the entry being added or deleted.

+                 * The filter needs to be satisfied by _each_ occurrence of the

+                 * attribute in the entry--otherwise you could satisfy the filter

+                 * and then put loads of other values in on the back of it.

+                 */

                 sval = NULL;

                 attrVal = NULL;

                 k = slapi_attr_first_value(attr_ptr, &sval);

@@ -2528,12 +2500,11 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                     if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,

                                                     attrFilter->attr_str,

-                                                    (struct berval *)attrVal) == LDAP_SUCCESS) {

-

+                                                    (struct berval *)attrVal) == LDAP_SUCCESS)

+                    {

                         attr_matched = acl__test_filter(aclpb->aclpb_filter_test_entry,

                                                         attrFilter->filter,

-                                                        1 /* Do filter sense evaluation below */

-                                                        );

+                                                        1 /* Do filter sense evaluation below */);

                         done = !attr_matched;

                         slapi_entry_free(aclpb->aclpb_filter_test_entry);

                     }

@@ -2542,19 +2513,19 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                 } /* while */

                 /*

-                     * Here, we applied an applicable filter to the entry.

-                     * So if attr_matched is ACL_TRUE then every value

-                     * of the attribute in the entry satisfied the filter.

-                     * Otherwise, attr_matched is ACL_FALSE and not every

-                     * value satisfied the filter, so we will teminate the

-                     * scan of the filter list.

-                     */

+                 * Here, we applied an applicable filter to the entry.

+                 * So if attr_matched is ACL_TRUE then every value

+                 * of the attribute in the entry satisfied the filter.

+                 * Otherwise, attr_matched is ACL_FALSE and not every

+                 * value satisfied the filter, so we will terminate the

+                 * scan of the filter list.

+                 */

             }

             num_attrs++;

         } /* while */

-/*

+        /*

          * Here, we've applied all the applicable filters to the entry.

          * Each one must have been satisfied by all the values of the attribute.

          * The result of this is stored in attr_matched.

@@ -2585,7 +2556,8 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     } else if (((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&

                 (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||

                ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&

-                (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS))) {

+                (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)))

+    {

         /*

          * Here, it's a modify add/del and we have attr filters.

          * So, we need to scan the add/del filter list to find the filter

@@ -2629,11 +2601,10 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

          * Otherwise, ignore the targetattrfilters.

          */

         if (found) {

-

             if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,

                                             aclpb->aclpb_curr_attrEval->attrEval_name,

-                                            aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS) {

-

+                                            aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS)

+            {

                 attr_matched = acl__test_filter(aclpb->aclpb_filter_test_entry,

                                                 attrFilter->filter,

                                                 1 /* Do filter sense evaluation below */

@@ -2651,20 +2622,21 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

              * Here this attribute appeared and was matched in a

              * targetattrfilters list, so record this fact so we do

              * not have to scan the targetattr list for the attribute.

-            */

+             */

             attr_matched_in_targetattrfilters = 1;

         }

     } /* targetvaluefilters */

-    /* There are 3 cases  by which acis are selected.

-    ** 1) By scanning the whole list and picking based on the resource.

-    ** 2) By picking a subset of the list which will be used for the whole

-    **    acl evaluation.

-    ** 3) A finer granularity, i.e, a selected list of acls which will be

-    ** used for only that entry's evaluation.

-    */

+    /*

+     * There are 3 cases  by which acis are selected.

+     * 1) By scanning the whole list and picking based on the resource.

+     * 2) By picking a subset of the list which will be used for the whole

+     *    acl evaluation.

+     * 3) A finer granularity, i.e, a selected list of acls which will be

+     * used for only that entry's evaluation.

+     */

     if (!(skip_attrEval) && (aclpb->aclpb_state & ACLPB_SEARCH_BASED_ON_ENTRY_LIST) &&

         (res_right & SLAPI_ACL_SEARCH) &&

         ((aci->aci_access & SLAPI_ACL_READ) || (aci->aci_access & SLAPI_ACL_SEARCH))) {

@@ -2680,7 +2652,6 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

         }

     }

-

     /* If we are suppose to skip attr eval, then let's skip it */

     if ((aclpb->aclpb_access & SLAPI_ACL_SEARCH) && (!skip_attrEval) &&

         (aclpb->aclpb_res_type & ACLPB_NEW_ENTRY)) {

@@ -2697,9 +2668,10 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

         goto acl__resource_match_aci_EXIT;

     }

-    /* We need to check again because we don't want to select this handle

-    ** if the right doesn't match for now.

-    */

+    /*

+     * We need to check again because we don't want to select this handle

+     * if the right doesn't match for now.

+     */

     if (!(aci_right & res_right)) {

         matches = ACL_FALSE;

         goto acl__resource_match_aci_EXIT;

@@ -2718,20 +2690,16 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

      * rbyrneXXX if we had a proper permission for modrdn eg SLAPI_ACL_MODRDN

      * then we would not need this crappy way of telling it was a MODRDN

      * request ie. SLAPI_ACL_WRITE && !(c_attrEval).

-    */

-

+     */

     c_attrEval = aclpb->aclpb_curr_attrEval;

     /*

      * If we've already matched on targattrfilter then do not

      * bother to look at the attrlist.

-    */

-

+     */

     if (!attr_matched_in_targetattrfilters) {

-

         /* match target attr */

-        if ((c_attrEval) &&

-            (aci->aci_type & ACI_TARGET_ATTR)) {

+        if ((c_attrEval) && (aci->aci_type & ACI_TARGET_ATTR)) {

             /* there is a target ATTR */

             Targetattr **attrArray = aci->targetAttr;

             Targetattr *attr = NULL;

@@ -2773,46 +2741,43 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

                 matches = (attr_matched ? ACL_TRUE : ACL_FALSE);

             }

-

             aclpb->aclpb_state &= ~ACLPB_ATTR_STAR_MATCHED;

             /* figure out how it matched, i.e star matched */

-            if (matches && star_matched && num_attrs == 1 &&

-                !(aclpb->aclpb_state & ACLPB_FOUND_ATTR_RULE))

+            if (matches && star_matched && num_attrs == 1 && !(aclpb->aclpb_state & ACLPB_FOUND_ATTR_RULE)) {

                 aclpb->aclpb_state |= ACLPB_ATTR_STAR_MATCHED;

-            else {

+            } else {

                 /* we are here means that there is a specific

-                ** attr in the rule for this resource.

-                ** We need to avoid this case

-                ** Rule 1: (targetattr = "uid")

-                ** Rule 2: (targetattr = "*")

-                ** we cannot use STAR optimization

-                */

+                 * attr in the rule for this resource.

+                 * We need to avoid this case

+                 * Rule 1: (targetattr = "uid")

+                 * Rule 2: (targetattr = "*")

+                 * we cannot use STAR optimization

+                 */

                 aclpb->aclpb_state |= ACLPB_FOUND_ATTR_RULE;

                 aclpb->aclpb_state &= ~ACLPB_ATTR_STAR_MATCHED;

             }

-        } else if ((c_attrEval) ||

-                   (aci->aci_type & ACI_TARGET_ATTR)) {

+        } else if ((c_attrEval) || (aci->aci_type & ACI_TARGET_ATTR)) {

             if ((aci_right & ACL_RIGHTS_TARGETATTR_NOT_NEEDED) &&

                 (aclpb->aclpb_access & ACL_RIGHTS_TARGETATTR_NOT_NEEDED)) {

                 /*

-            ** Targetattr rule doesn't  make any sense

-            ** in this case. So select this rule

-            ** default: matches = ACL_TRUE;

-            */

+                 * Targetattr rule doesn't make any sense

+                 * in this case. So select this rule

+                 * default: matches = ACL_TRUE;

+                 */

                 ;

-            } else if (aci_right & SLAPI_ACL_WRITE &&

+            } else if ((aci_right & SLAPI_ACL_WRITE) &&

                        (aci->aci_type & ACI_TARGET_ATTR) &&

                        !(c_attrEval) &&

                        (aci->aci_type & ACI_HAS_ALLOW_RULE)) {

                 /* We need to handle modrdn operation.  Modrdn doesn't

-            ** change any attrs but changes the RDN and so (attr=NULL).

-            ** Here we found an acl which has a targetattr but

-            ** the resource doesn't need one. In that case, we should

-            ** consider this acl.

-            ** the opposite is true if it is a deny rule, only a deny without

-            ** any targetattr should deny modrdn

-            ** default: matches = ACL_TRUE;

-            */

+                 * change any attrs but changes the RDN and so (attr=NULL).

+                 * Here we found an acl which has a targetattr but

+                 * the resource doesn't need one. In that case, we should

+                 * consider this acl.

+                 * the opposite is true if it is a deny rule, only a deny without

+                 * any targetattr should deny modrdn

+                 * default: matches = ACL_TRUE;

+                 */

                 ;

             } else {

                 matches = ACL_FALSE;

@@ -2821,16 +2786,16 @@ acl__resource_match_aci(Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *a

     } /* !attr_matched_in_targetattrfilters */

     /*

-    ** Here we are testing if we find a entry test rule (which should

-    ** be rare). In that case, just remember it. An entry test rule

-    ** doesn't have "(targetattr)".

-    */

+     * Here we are testing if we find a entry test rule (which should

+     * be rare). In that case, just remember it. An entry test rule

+     * doesn't have "(targetattr)".

+     */

     if ((aclpb->aclpb_state & ACLPB_EVALUATING_FIRST_ATTR) &&

         (!(aci->aci_type & ACI_TARGET_ATTR))) {

         aclpb->aclpb_state |= ACLPB_FOUND_A_ENTRY_TEST_RULE;

     }

-/*

+    /*

      * Generic exit point for this routine:

      * matches is ACL_TRUE if the aci matches the target of the resource,

      * ACL_FALSE othrewise.