dsdb: Avoid calculating the PSO multiple times

In a typical user login query, the code tries to work out the PSO 2-3
times - once for the msDS-ResultantPSO attribute, and then again for the
msDS-User-Account-Control-Computed & msDS-UserPasswordExpiryTimeComputed
constructed attributes.

The PSO calculation is reasonably expensive, mostly due to the nested
groups calculation. If we've already constructed the msDS-ResultantPSO
attribute, then we can save ourselves extra work by just re-fetching the
result directly, rather than expanding the nested groups again from
scratch.

The previous patch improves efficiency when there are no PSOs in the
system. This should improve the case where there are PSOs that apply to
the users. (Unfortunately, it won't help where there are some PSOs in
the system, but no PSO applies to the user being queried).

Also updated sam.c so the msDS-ResultantPSO gets calculated first,
before the other constructed attributes.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
Autobuild-User(master): Garming Sam <garming@samba.org>
Autobuild-Date(master): Wed May 23 10:09:11 CEST 2018 on sn-devel-144

diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index 22ec8cef2a1f057d9d3023946121120a61a91917..07cfbd06b3363659504580ce7518062e63f7e383 100644 (file)
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -68,6 +68,14 @@ const char *server_attrs[] = {
 };
 
 const char *user_attrs[] = {
+       /*
+        * This ordering (having msDS-ResultantPSO first) is
+        * important.  By processing this attribute first it is
+        * available in the operational module for the other PSO
+        * attribute calcuations to use.
+        */
+       "msDS-ResultantPSO",
+
        KRBTGT_ATTRS,
 
        "logonHours",
@@ -103,7 +111,6 @@ const char *user_attrs[] = {
        "badPasswordTime",
        "lmPwdHistory",
        "ntPwdHistory",
-       "msDS-ResultantPSO",
        NULL,
 };
 

diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
index c99adb1dcd779a3040a2a44a10ded51d36a9ec5a..cdcec1c10463c7ab076c344cde746b48d00a4ace 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/operational.c
+++ b/source4/dsdb/samdb/ldb_modules/operational.c
@@ -1145,10 +1145,17 @@ static int get_pso_for_user(struct ldb_module *module,
        unsigned int num_groupSIDs = 0;
        struct ldb_context *ldb = ldb_module_get_ctx(module);
        struct ldb_message *best_pso = NULL;
+       struct ldb_dn *pso_dn = NULL;
        int ret;
        struct ldb_message_element *el = NULL;
        TALLOC_CTX *tmp_ctx = NULL;
        int pso_count = 0;
+       struct ldb_result *res = NULL;
+       static const char *attrs[] = {
+               "msDS-LockoutDuration",
+               "msDS-MaximumPasswordAge",
+               NULL
+       };
 
        *pso_msg = NULL;
 
@@ -1161,6 +1168,31 @@ static int get_pso_for_user(struct ldb_module *module,
 
        tmp_ctx = talloc_new(user_msg);
 
+       /*
+        * Several different constructed attributes try to use the PSO info. If
+        * we've already constructed the msDS-ResultantPSO for this user, we can
+        * just re-use the result, rather than calculating it from scratch again
+        */
+       pso_dn = ldb_msg_find_attr_as_dn(ldb, tmp_ctx, user_msg,
+                                        "msDS-ResultantPSO");
+
+       if (pso_dn != NULL) {
+               ret = dsdb_module_search_dn(module, tmp_ctx, &res, pso_dn,
+                                           attrs, DSDB_FLAG_NEXT_MODULE,
+                                           parent);
+               if (ret != LDB_SUCCESS) {
+                       DBG_ERR("Error %d retrieving PSO %s\n", ret,
+                               ldb_dn_get_linearized(pso_dn));
+                       talloc_free(tmp_ctx);
+                       return ret;
+               }
+
+               if (res->count == 1) {
+                       *pso_msg = res->msgs[0];
+                       return LDB_SUCCESS;
+               }
+       }
+
        /*
         * if any PSOs apply directly to the user, they are considered first
         * before we check group membership PSOs
@@ -1190,7 +1222,7 @@ static int get_pso_for_user(struct ldb_module *module,
         * If no valid PSO applies directly to the user, then try its groups.
         * The group expansion is expensive, so check there are actually
         * PSOs in the DB first (which is a quick search). Note in the above
-        * case we could tell that a PSO applied to the user, based on info
+        * cases we could tell that a PSO applied to the user, based on info
         * already retrieved by the user search.
         */
        ret = get_pso_count(module, tmp_ctx, parent, &pso_count);
@@ -1661,7 +1693,10 @@ static int operational_search(struct ldb_module *module, struct ldb_request *req
        ac->attrs_to_replace_size = 0;
        /* in the list of attributes we are looking for, rename any
           attributes to the alias for any hidden attributes that can
-          be fetched directly using non-hidden names */
+          be fetched directly using non-hidden names.
+          Note that order here can affect performance, e.g. we should process
+          msDS-ResultantPSO before msDS-User-Account-Control-Computed (as the
+          latter is also dependent on the PSO information) */
        for (a=0;ac->attrs && ac->attrs[a];a++) {
                if (check_keep_control_for_attribute(ac->controls_flags, ac->attrs[a])) {
                        continue;