Re: SLAPD: Should access checks take place before filter matching?

[Simon Spero <ses@tipper.oit.unc.edu>]

On Fri, 22 Jun 2001, Kurt D. Zeilenga wrote:

> If one doesn't evaluate the ACL during filter matching, then
> all entries matching the filter will be returned without the

I think we're talking at cross-purposes here.

I'm talking about avoiding evaluating the ACL if the filter doesn't match
the candidate. If test_filter returns anything other than
LDAP_COMPARE_TRUE, nothing is send to the client.

In the example given: In the current implementation, the backend call
test_filter for each candidate, check for search access to userPassword,
fail to find it, and LDAP_INSUFFICIENT_ACCESSM. The backend will compare
this to LDAP_COMPARE_TRUE, and since it isn't that value, skip to the next
candidate. It will evaluate the ACL for every candidate, regardless of
whether the password was == secret or not; if the ACL test is passed it
will go on to perform the actual filter comparison, which is usually an
in-core compare. This test is only performed if the ACL test is passed.

In the proposed change, test_filter will check the password to see if it
is == secret. If it isn't then it will return LDAP_COMPARE_FALSE. If the
filter does match, the backend will then check the ACL. If the ACL test
fails, test_filter will return LDAP_INSUFFICIENT_ACCESS.

In both cases the server will only return a search result iff BOTH tests
are passed.

I'm attaching a few lines of diffs at the end, which explains what I mean
far better than I can :-) This change yields a 3-1 speed up for a
substring search on a field with no substring index. Relative performance
was the same both for an ACL that did not permit searches, and one that
did. In both cases both  the modified and unmodified servers returned the
same results as each other.

Simon

diff -c -r1.36 filterentry.c
*** filterentry.c       2001/01/17 15:35:53     1.36
--- filterentry.c       2001/06/22 19:55:35
***************
*** 218,225 ****
  }


  static int
! test_ava_filter(
      Backend   *be,
      Connection        *conn,
      Operation *op,
--- 218,226 ----
  }


+
  static int
! unchecked_test_ava_filter(
      Backend   *be,
      Connection        *conn,
      Operation *op,
***************
*** 230,242 ****
  {
        int             i;
        Attribute       *a;
!
!       if ( be != NULL && ! access_allowed( be, conn, op, e,
!               ava->aa_desc, ava->aa_value, ACL_SEARCH ) )
!       {
!               return LDAP_INSUFFICIENT_ACCESS;
!       }
!
        for(a = attrs_find( e->e_attrs, ava->aa_desc );
                a != NULL;
                a = attrs_find( a->a_next, ava->aa_desc ) )
--- 231,237 ----
  {
        int             i;
        Attribute       *a;
!
        for(a = attrs_find( e->e_attrs, ava->aa_desc );
                a != NULL;
                a = attrs_find( a->a_next, ava->aa_desc ) )
***************
*** 304,309 ****
--- 299,328 ----
        }

        return( LDAP_COMPARE_FALSE );
+ }
+
+
+ static int
+ test_ava_filter(
+     Backend   *be,
+     Connection        *conn,
+     Operation *op,
+     Entry     *e,
+       AttributeAssertion *ava,
+     int               type
+ )
+ {
+
+   int rc = unchecked_test_ava_filter(be,conn,op,e,ava,type);
+   if(rc == LDAP_COMPARE_TRUE) {
+     if ( be != NULL && ! access_allowed( be, conn, op, e,
+               ava->aa_desc, ava->aa_value, ACL_SEARCH ) )
+       {
+               return LDAP_INSUFFICIENT_ACCESS;
+       }
+   } else {
+     return rc;
+   }
  }