dsdb: Avoid performance hit if PSOs aren't actually used
authorTim Beale <timbeale@catalyst.net.nz>
Tue, 15 May 2018 02:02:32 +0000 (14:02 +1200)
committerGarming Sam <garming@samba.org>
Wed, 23 May 2018 04:55:32 +0000 (06:55 +0200)
The new PSO code adds some additional overhead in extra lookups. To
avoid penalizing existing setups, we can short-circuit the PSO
processing and return early if there are no actual PSO objects in the
DB. The one-level search should be very quick, and it avoids the need to
do more complicated PSO processing (i.e. expanding the nested groups).

The longer-term plan is to rework the tokenGroups lookup so that it only
gets done once, and the result can then be reused by the resultant-PSO
code (rather than computing the nested-groups again). However, in the
short-term, a slight decrease in performance is the price for any users
that want to deploy PSOs.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>
source4/dsdb/samdb/ldb_modules/operational.c

index c2968a4b802fd83e6eafbb8ba67d84deba6892ff..c99adb1dcd779a3040a2a44a10ded51d36a9ec5a 100644 (file)
@@ -981,6 +981,43 @@ static bool pso_is_supported(struct ldb_context *ldb, struct ldb_message *msg)
        return true;
 }
 
+/*
+ * Returns the number of PSO objects that exist in the DB
+ */
+static int get_pso_count(struct ldb_module *module, TALLOC_CTX *mem_ctx,
+                        struct ldb_request *parent, int *pso_count)
+{
+       static const char * const attrs[] = { NULL };
+       int ret;
+       struct ldb_dn *domain_dn = NULL;
+       struct ldb_dn *psc_dn = NULL;
+       struct ldb_result *res = NULL;
+       struct ldb_context *ldb = ldb_module_get_ctx(module);
+
+       domain_dn = ldb_get_default_basedn(ldb);
+       psc_dn = ldb_dn_new_fmt(mem_ctx, ldb,
+                               "CN=Password Settings Container,CN=System,%s",
+                               ldb_dn_get_linearized(domain_dn));
+       if (psc_dn == NULL) {
+               return ldb_oom(ldb);
+       }
+
+       /* get the number of PSO children */
+       ret = dsdb_module_search(module, mem_ctx, &res, psc_dn,
+                                LDB_SCOPE_ONELEVEL, attrs,
+                                DSDB_FLAG_NEXT_MODULE, parent,
+                                "(objectClass=msDS-PasswordSettings)");
+       if (ret != LDB_SUCCESS) {
+               return ret;
+       }
+
+       *pso_count = res->count;
+       talloc_free(res);
+       talloc_free(psc_dn);
+
+       return LDB_SUCCESS;
+}
+
 /*
  * Compares two PSO objects returned by a search, to work out the better PSO.
  * The PSO with the lowest precedence is better, otherwise (if the precedence
@@ -1111,6 +1148,7 @@ static int get_pso_for_user(struct ldb_module *module,
        int ret;
        struct ldb_message_element *el = NULL;
        TALLOC_CTX *tmp_ctx = NULL;
+       int pso_count = 0;
 
        *pso_msg = NULL;
 
@@ -1150,8 +1188,24 @@ static int get_pso_for_user(struct ldb_module *module,
 
        /*
         * If no valid PSO applies directly to the user, then try its groups.
-        * Work out the SIDs of any account groups the user is a member of
+        * 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
+        * already retrieved by the user search.
         */
+       ret = get_pso_count(module, tmp_ctx, parent, &pso_count);
+       if (ret != LDB_SUCCESS) {
+               DBG_ERR("Error %d determining PSOs in system\n", ret);
+               talloc_free(tmp_ctx);
+               return ret;
+       }
+
+       if (pso_count == 0) {
+               talloc_free(tmp_ctx);
+               return LDB_SUCCESS;
+       }
+
+       /* Work out the SIDs of any account groups the user is a member of */
        ret = get_group_sids(ldb, tmp_ctx, user_msg,
                             "msDS-ResultantPSO", ACCOUNT_GROUPS,
                             &groupSIDs, &num_groupSIDs);