Blame SOURCES/0050-Ticket-49278-GetEffectiveRights-gives-false-negative.patch

081b2d
From 6e00c3bac13811bc6d94b810b17a59f9428c29f6 Mon Sep 17 00:00:00 2001
081b2d
From: Ludwig Krispenz <lkrispen@redhat.com>
081b2d
Date: Thu, 11 Jan 2018 15:17:56 +0100
081b2d
Subject: [PATCH]    Ticket 49278 - GetEffectiveRights gives false-negative
081b2d
081b2d
    Bug: If geteffective rights was issued for an non existing entry the
081b2d
         mechanism to genrate a template entry no longer worked and no results were
081b2d
         returned.
081b2d
    Fix: Improve the handling in itreating the result set, so that template entries (if
081b2d
         requested) are genereated and are not applied to existing entries.
081b2d
         Also some code cleanup in iterate()
081b2d
    Reviewed by: Thierry, thanks
081b2d
---
081b2d
 ldap/servers/slapd/opshared.c | 239 ++++++++++++++++++++----------------------
081b2d
 1 file changed, 114 insertions(+), 125 deletions(-)
081b2d
081b2d
diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c
081b2d
index 24157120e..46dcf6fba 100644
081b2d
--- a/ldap/servers/slapd/opshared.c
081b2d
+++ b/ldap/servers/slapd/opshared.c
081b2d
@@ -33,6 +33,7 @@ static char *pwpolicy_lock_attrs_all[] = {"passwordRetryCount",
081b2d
 static void compute_limits(Slapi_PBlock *pb);
081b2d
 static int send_results_ext(Slapi_PBlock *pb, int send_result, int *nentries, int pagesize, unsigned int *pr_stat);
081b2d
 static int process_entry(Slapi_PBlock *pb, Slapi_Entry *e, int send_result);
081b2d
+static void send_entry(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Operation *operation, char **attrs, int attrsonly, int *pnentries);
081b2d
 
081b2d
 int
081b2d
 op_shared_is_allowed_attr(const char *attr_name, int replicated_op)
081b2d
@@ -1040,6 +1041,31 @@ process_entry(Slapi_PBlock *pb, Slapi_Entry *e, int send_result)
081b2d
 
081b2d
     return 0;
081b2d
 }
081b2d
+static void
081b2d
+send_entry(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Operation *operation, char **attrs, int attrsonly, int *pnentries)
081b2d
+{
081b2d
+                /*
081b2d
+             * It's a regular entry, or it's a referral and
081b2d
+             * managedsait control is on.  In either case, send
081b2d
+             * the entry.
081b2d
+             */
081b2d
+                switch (send_ldap_search_entry(pb, e, NULL, attrs, attrsonly)) {
081b2d
+                case 0: /* entry sent ok */
081b2d
+                    (*pnentries)++;
081b2d
+                    slapi_pblock_set(pb, SLAPI_NENTRIES, pnentries);
081b2d
+                    break;
081b2d
+                case 1: /* entry not sent */
081b2d
+                    break;
081b2d
+                case -1: /* connection closed */
081b2d
+                    /*
081b2d
+                     * mark the operation as abandoned so the backend
081b2d
+                     * next entry function gets called again and has
081b2d
+                     * a chance to clean things up.
081b2d
+                     */
081b2d
+                    operation->o_status = SLAPI_OP_STATUS_ABANDONED;
081b2d
+                    break;
081b2d
+                }
081b2d
+}
081b2d
 
081b2d
 #if 0
081b2d
 /* Loops through search entries and sends them to the client.
081b2d
@@ -1214,7 +1240,7 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result, int *pnentries, in
081b2d
     *pnentries = 0;
081b2d
 
081b2d
     while (!done) {
081b2d
-        Slapi_Entry *gerentry = NULL;
081b2d
+        Slapi_Entry *ger_template_entry = NULL;
081b2d
         Slapi_Operation *operation;
081b2d
 
081b2d
         slapi_pblock_get(pb, SLAPI_OPERATION, &operation);
081b2d
@@ -1236,57 +1262,57 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result, int *pnentries, in
081b2d
         slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
081b2d
 
081b2d
         /* Check for possible get_effective_rights control */
081b2d
-        if (e) {
081b2d
-            if (operation->o_flags & OP_FLAG_GET_EFFECTIVE_RIGHTS) {
081b2d
-                char *errbuf = NULL;
081b2d
+        if (operation->o_flags & OP_FLAG_GET_EFFECTIVE_RIGHTS) {
081b2d
+            char *errbuf = NULL;
081b2d
+
081b2d
+            if (PAGEDRESULTS_PAGE_END == pr_stat) {
081b2d
+                /*
081b2d
+                 * read ahead -- there is at least more entry.
081b2d
+                 * undo it and return the PAGE_END
081b2d
+                 */
081b2d
+                be->be_prev_search_results(pb);
081b2d
+                done = 1;
081b2d
+                continue;
081b2d
+            }
081b2d
+            if ( e == NULL ) {
081b2d
                 char **gerattrs = NULL;
081b2d
                 char **gerattrsdup = NULL;
081b2d
                 char **gap = NULL;
081b2d
                 char *gapnext = NULL;
081b2d
-
081b2d
-                if (PAGEDRESULTS_PAGE_END == pr_stat) {
081b2d
-                    /*
081b2d
-                 * read ahead -- there is at least more entry.
081b2d
-                 * undo it and return the PAGE_END
081b2d
+                /* we have no more entries
081b2d
+                 * but we might create a template entry for GER
081b2d
+                 * so we need to continue, but make sure to stop
081b2d
+                 * after handling the template entry.
081b2d
+                 * the template entry is a temporary entry returned by the acl
081b2d
+                 * plugin in the pblock and will be freed
081b2d
                  */
081b2d
-                    be->be_prev_search_results(pb);
081b2d
-                    done = 1;
081b2d
-                    continue;
081b2d
-                }
081b2d
+                done = 1;
081b2d
+                pr_stat = PAGEDRESULTS_SEARCH_END;
081b2d
 
081b2d
                 slapi_pblock_get(pb, SLAPI_SEARCH_GERATTRS, &gerattrs);
081b2d
                 gerattrsdup = cool_charray_dup(gerattrs);
081b2d
                 gap = gerattrsdup;
081b2d
-                do {
081b2d
+                while (gap && *gap) {
081b2d
                     gapnext = NULL;
081b2d
-                    if (gap) {
081b2d
-                        if (*gap && *(gap + 1)) {
081b2d
-                            gapnext = *(gap + 1);
081b2d
-                            *(gap + 1) = NULL;
081b2d
-                        }
081b2d
-                        slapi_pblock_set(pb, SLAPI_SEARCH_GERATTRS, gap);
081b2d
-                        rc = plugin_call_acl_plugin(pb, e, attrs, NULL,
081b2d
-                                                    SLAPI_ACL_ALL, ACLPLUGIN_ACCESS_GET_EFFECTIVE_RIGHTS,
081b2d
-                                                    &errbuf);
081b2d
-                        if (NULL != gapnext) {
081b2d
-                            *(gap + 1) = gapnext;
081b2d
-                        }
081b2d
-                    } else if (NULL != e) {
081b2d
-                        rc = plugin_call_acl_plugin(pb, e, attrs, NULL,
081b2d
-                                                    SLAPI_ACL_ALL, ACLPLUGIN_ACCESS_GET_EFFECTIVE_RIGHTS,
081b2d
-                                                    &errbuf);
081b2d
+                    if (*(gap + 1)) {
081b2d
+                        gapnext = *(gap + 1);
081b2d
+                        *(gap + 1) = NULL;
081b2d
+                    }
081b2d
+                    slapi_pblock_set(pb, SLAPI_SEARCH_GERATTRS, gap);
081b2d
+                    rc = plugin_call_acl_plugin(pb, e, attrs, NULL,
081b2d
+                                                SLAPI_ACL_ALL, ACLPLUGIN_ACCESS_GET_EFFECTIVE_RIGHTS,
081b2d
+                                                &errbuf);
081b2d
+                    if (NULL != gapnext) {
081b2d
+                        *(gap + 1) = gapnext;
081b2d
                     }
081b2d
+                    gap++;
081b2d
+                    /* get the template entry, if any */
081b2d
+                    slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
081b2d
                     if (NULL == e) {
081b2d
-                        /* get the template entry, if any */
081b2d
-                        slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_ENTRY, &e);
081b2d
-                        if (NULL == e) {
081b2d
-                            /* everything is ok - don't send the result */
081b2d
-                            pr_stat = PAGEDRESULTS_SEARCH_END;
081b2d
-                            done = 1;
081b2d
-                            continue;
081b2d
-                        }
081b2d
-                        gerentry = e;
081b2d
+                        /* everything is ok - don't send the result */
081b2d
+                        continue;
081b2d
                     }
081b2d
+                    ger_template_entry = e;
081b2d
                     if (rc != LDAP_SUCCESS) {
081b2d
                         /* Send error result and
081b2d
                        abort op if the control is critical */
081b2d
@@ -1294,65 +1320,53 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result, int *pnentries, in
081b2d
                                       "Failed to get effective rights for entry (%s), rc=%d\n",
081b2d
                                       slapi_entry_get_dn_const(e), rc);
081b2d
                         send_ldap_result(pb, rc, NULL, errbuf, 0, NULL);
081b2d
-                        slapi_ch_free((void **)&errbuf);
081b2d
-                        if (gerentry) {
081b2d
-                            slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL);
081b2d
-                            slapi_entry_free(gerentry);
081b2d
-                            gerentry = e = NULL;
081b2d
-                        }
081b2d
-                        pr_stat = PAGEDRESULTS_SEARCH_END;
081b2d
                         rval = -1;
081b2d
-                        done = 1;
081b2d
-                        continue;
081b2d
-                    }
081b2d
-                    slapi_ch_free((void **)&errbuf);
081b2d
-                    if (process_entry(pb, e, send_result)) {
081b2d
-                        /* shouldn't send this entry */
081b2d
-                        if (gerentry) {
081b2d
-                            slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL);
081b2d
-                            slapi_entry_free(gerentry);
081b2d
-                            gerentry = e = NULL;
081b2d
+                    } else {
081b2d
+                        if (!process_entry(pb, e, send_result)) {
081b2d
+                            /* should send this entry now*/
081b2d
+                            send_entry(pb, e, operation, attrs, attrsonly, pnentries);
081b2d
                         }
081b2d
-                        continue;
081b2d
                     }
081b2d
 
081b2d
-                    /*
081b2d
-                 * It's a regular entry, or it's a referral and
081b2d
-                 * managedsait control is on.  In either case, send
081b2d
-                 * the entry.
081b2d
-                 */
081b2d
-                    switch (send_ldap_search_entry(pb, e, NULL, attrs, attrsonly)) {
081b2d
-                    case 0: /* entry sent ok */
081b2d
-                        (*pnentries)++;
081b2d
-                        slapi_pblock_set(pb, SLAPI_NENTRIES, pnentries);
081b2d
-                        break;
081b2d
-                    case 1: /* entry not sent */
081b2d
-                        break;
081b2d
-                    case -1: /* connection closed */
081b2d
-                        /*
081b2d
-                         * mark the operation as abandoned so the backend
081b2d
-                         * next entry function gets called again and has
081b2d
-                         * a chance to clean things up.
081b2d
-                         */
081b2d
-                        operation->o_status = SLAPI_OP_STATUS_ABANDONED;
081b2d
-                        break;
081b2d
-                    }
081b2d
-                    if (gerentry) {
081b2d
+                    slapi_ch_free((void **)&errbuf);
081b2d
+                    if (ger_template_entry) {
081b2d
                         slapi_pblock_set(pb, SLAPI_SEARCH_RESULT_ENTRY, NULL);
081b2d
-                        slapi_entry_free(gerentry);
081b2d
-                        gerentry = e = NULL;
081b2d
+                        slapi_entry_free(ger_template_entry);
081b2d
+                        ger_template_entry = e = NULL;
081b2d
                     }
081b2d
-                } while (gap && ++gap && *gap);
081b2d
+                } /* while ger template */
081b2d
                 slapi_pblock_set(pb, SLAPI_SEARCH_GERATTRS, gerattrs);
081b2d
                 cool_charray_free(gerattrsdup);
081b2d
-                if (pagesize == *pnentries) {
081b2d
-                    /* PAGED RESULTS: reached the pagesize */
081b2d
-                    /* We don't set "done = 1" here.
081b2d
-                 * We read ahead next entry to check whether there is
081b2d
-                 * more entries to return or not. */
081b2d
-                    pr_stat = PAGEDRESULTS_PAGE_END;
081b2d
+            } else {
081b2d
+                /* we are processing geteffective rights for an existing entry */
081b2d
+                rc = plugin_call_acl_plugin(pb, e, attrs, NULL,
081b2d
+                                            SLAPI_ACL_ALL, ACLPLUGIN_ACCESS_GET_EFFECTIVE_RIGHTS,
081b2d
+                                             &errbuf);
081b2d
+                if (rc != LDAP_SUCCESS) {
081b2d
+                    /* Send error result and
081b2d
+                       abort op if the control is critical */
081b2d
+                    slapi_log_err(SLAPI_LOG_ERR, "iterate",
081b2d
+                                  "Failed to get effective rights for entry (%s), rc=%d\n",
081b2d
+                                  slapi_entry_get_dn_const(e), rc);
081b2d
+                    send_ldap_result(pb, rc, NULL, errbuf, 0, NULL);
081b2d
+                    rval = -1;
081b2d
+                } else {
081b2d
+                    if (!process_entry(pb, e, send_result)) {
081b2d
+                        /* should send this entry now*/
081b2d
+                        send_entry(pb, e, operation, attrs, attrsonly, pnentries);
081b2d
+                        if (pagesize == *pnentries) {
081b2d
+                            /* PAGED RESULTS: reached the pagesize */
081b2d
+                            /* We don't set "done = 1" here.
081b2d
+                             * We read ahead next entry to check whether there is
081b2d
+                             * more entries to return or not. */
081b2d
+                            pr_stat = PAGEDRESULTS_PAGE_END;
081b2d
+                        }
081b2d
+                    }
081b2d
                 }
081b2d
-            } else { /* not GET_EFFECTIVE_RIGHTS */
081b2d
+                slapi_ch_free((void **)&errbuf);
081b2d
+            }
081b2d
+        /* not GET_EFFECTIVE_RIGHTS */
081b2d
+        } else if (e) {
081b2d
                 if (PAGEDRESULTS_PAGE_END == pr_stat) {
081b2d
                     /*
081b2d
                  * read ahead -- there is at least more entry.
081b2d
@@ -1364,46 +1378,21 @@ iterate(Slapi_PBlock *pb, Slapi_Backend *be, int send_result, int *pnentries, in
081b2d
                 }
081b2d
                 /* Adding shadow password attrs. */
081b2d
                 add_shadow_ext_password_attrs(pb, &e);
081b2d
-                if (process_entry(pb, e, send_result)) {
081b2d
-                    /* shouldn't  send this entry */
081b2d
-                    struct slapi_entry *pb_pw_entry = slapi_pblock_get_pw_entry(pb);
081b2d
-                    slapi_entry_free(pb_pw_entry);
081b2d
-                    slapi_pblock_set_pw_entry(pb, NULL);
081b2d
-                    continue;
081b2d
-                }
081b2d
-
081b2d
-                /*
081b2d
-             * It's a regular entry, or it's a referral and
081b2d
-             * managedsait control is on.  In either case, send
081b2d
-             * the entry.
081b2d
-             */
081b2d
-                switch (send_ldap_search_entry(pb, e, NULL, attrs, attrsonly)) {
081b2d
-                case 0: /* entry sent ok */
081b2d
-                    (*pnentries)++;
081b2d
-                    slapi_pblock_set(pb, SLAPI_NENTRIES, pnentries);
081b2d
-                    break;
081b2d
-                case 1: /* entry not sent */
081b2d
-                    break;
081b2d
-                case -1: /* connection closed */
081b2d
-                    /*
081b2d
-                     * mark the operation as abandoned so the backend
081b2d
-                     * next entry function gets called again and has
081b2d
-                     * a chance to clean things up.
081b2d
-                     */
081b2d
-                    operation->o_status = SLAPI_OP_STATUS_ABANDONED;
081b2d
-                    break;
081b2d
+                if (!process_entry(pb, e, send_result)) {
081b2d
+                    /*this entry was not sent, do it now*/
081b2d
+                    send_entry(pb, e, operation, attrs, attrsonly, pnentries);
081b2d
+                    if (pagesize == *pnentries) {
081b2d
+                        /* PAGED RESULTS: reached the pagesize */
081b2d
+                        /* We don't set "done = 1" here.
081b2d
+                         * We read ahead next entry to check whether there is
081b2d
+                         * more entries to return or not. */
081b2d
+                        pr_stat = PAGEDRESULTS_PAGE_END;
081b2d
+                    }
081b2d
                 }
081b2d
+                /* cleanup pw entry . sent or not */
081b2d
                 struct slapi_entry *pb_pw_entry = slapi_pblock_get_pw_entry(pb);
081b2d
                 slapi_entry_free(pb_pw_entry);
081b2d
                 slapi_pblock_set_pw_entry(pb, NULL);
081b2d
-                if (pagesize == *pnentries) {
081b2d
-                    /* PAGED RESULTS: reached the pagesize */
081b2d
-                    /* We don't set "done = 1" here.
081b2d
-                 * We read ahead next entry to check whether there is
081b2d
-                 * more entries to return or not. */
081b2d
-                    pr_stat = PAGEDRESULTS_PAGE_END;
081b2d
-                }
081b2d
-            }
081b2d
         } else {
081b2d
             /* no more entries */
081b2d
             done = 1;
081b2d
-- 
081b2d
2.13.6
081b2d