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

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