On Sat, May 12, 2018 at 08:56:48PM +1000, Jonathan Matthew wrote:
> > This only supports "write" (modify, add, delete) and not "read"
> > (search) filter rules.  The search mode will be more complicated and I
> > will look at this later.
> > 
> > Thoughts?  OK?
> 
> ok.  Read filters would be good to have, but this is a nice addition as it is.
> 

OK, here is one attempt at implementing attribute read filters:
it simply filters out forbidden attributes from the search result.

1. But what should be the desired logic for read attribute filter rules?
a) filter out each attribute that is not readable and continue
b) fail with error 50 (insuf. access) if a child attribute is not readable

2. Should read access to the parent dn be required?
For example:
        deny read access to any by self
        allow read access to any attribute helloWorld by self
Should this
a) deny access to the helloWorld attribute or
b) allow access to the helloWorld attribute (but no other attribute)

The diff below implements the easiest option 1a) + 2a).

Thoughts?

Reyk

Index: usr.sbin/ldapd/ldapd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/ldapd.conf.5,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 ldapd.conf.5
--- usr.sbin/ldapd/ldapd.conf.5 14 May 2018 07:53:47 -0000      1.23
+++ usr.sbin/ldapd/ldapd.conf.5 14 May 2018 08:58:40 -0000
@@ -252,7 +252,6 @@ The scope scope can be restricted to an 
 .Bl -tag -width Ds
 .It attribute Ar name
 The filter rule applies to the specified attribute.
-Attributes can only be specified for write access rules.
 .El
 .Pp
 Finally, the filter rule can match a bind DN:
Index: usr.sbin/ldapd/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/parse.y,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 parse.y
--- usr.sbin/ldapd/parse.y      14 May 2018 07:53:47 -0000      1.27
+++ usr.sbin/ldapd/parse.y      14 May 2018 08:58:40 -0000
@@ -1164,12 +1164,6 @@ mk_aci(int type, int rights, enum scope 
            aci->scope,
            aci->subject ? aci->subject : "any");
 
-       if (aci->attribute && aci->rights != ACI_WRITE) {
-               yyerror("attributes only supported for write access filters");
-               free(aci);
-               return NULL;
-       }
-
        return aci;
 }
 
Index: usr.sbin/ldapd/search.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldapd/search.c,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 search.c
--- usr.sbin/ldapd/search.c     14 May 2018 07:53:47 -0000      1.19
+++ usr.sbin/ldapd/search.c     14 May 2018 08:58:40 -0000
@@ -102,7 +102,7 @@ search_result(const char *dn, size_t dnl
        struct ber_element      *root, *elm, *filtered_attrs = NULL, *link, *a;
        struct ber_element      *prev, *next;
        char                    *adesc;
-       void                    *buf;
+       void                    *buf, *searchdn = NULL;
 
        if ((root = ber_add_sequence(NULL)) == NULL)
                goto fail;
@@ -111,10 +111,20 @@ search_result(const char *dn, size_t dnl
                goto fail;
        link = filtered_attrs;
 
+       if ((searchdn = strndup(dn, dnlen)) == NULL)
+               goto fail;
+
        for (prev = NULL, a = attrs->be_sub; a; a = next) {
                if (ber_get_string(a->be_sub, &adesc) != 0)
                        goto fail;
-               if (should_include_attribute(adesc, search, 0)) {
+               /*
+                * Check if read access to the attribute is allowed and if it
+                * should be included in the search result.  The attribute is
+                * filtered out in the result if one of these conditions fails.
+                */
+               if (authorized(search->conn, search->ns, ACI_READ,
+                   searchdn, adesc, LDAP_SCOPE_BASE) &&
+                   should_include_attribute(adesc, search, 0)) {
                        next = a->be_next;
                        if (prev != NULL)
                                prev->be_next = a->be_next;     /* unlink a */
@@ -152,11 +162,13 @@ search_result(const char *dn, size_t dnl
                return -1;
        }
 
+       free(searchdn);
        return 0;
 fail:
        log_warn("search result");
        if (root)
                ber_free_elements(root);
+       free(searchdn);
        return -1;
 }
 

Reply via email to