From: Gary Lockyer Date: Wed, 17 Oct 2018 21:16:24 +0000 (+1300) Subject: source4 samr: cache samr_EnumDomainGroups results X-Git-Tag: tdb-1.3.17~718 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=564813b58830142699edb4f82539f0834434c2e4;p=thirdparty%2Fsamba.git source4 samr: cache samr_EnumDomainGroups results Add a cache of GUID's that matched the last samr_EnunDomainGroups made on a domain handle. The cache is cleared if resume_handle is zero, and when the final results are returned to the caller. Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett --- diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr deleted file mode 100644 index 23e5e1572cf..00000000000 --- a/selftest/knownfail.d/samr +++ /dev/null @@ -1,2 +0,0 @@ -^samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainGroups\(ad_dc_ntvfs:local\) -^samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainGroups\(ad_dc_ntvfs:local\) diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c index 58be23a0a27..0a22c7aad2e 100644 --- a/source4/rpc_server/samr/dcesrv_samr.c +++ b/source4/rpc_server/samr/dcesrv_samr.c @@ -1125,6 +1125,63 @@ static int compare_SamEntry(struct samr_SamEntry *e1, struct samr_SamEntry *e2) return e1->idx - e2->idx; } +static int compare_msgRid(struct ldb_message **m1, struct ldb_message **m2) { + struct dom_sid *sid1 = NULL; + struct dom_sid *sid2 = NULL; + uint32_t rid1; + uint32_t rid2; + int res = 0; + NTSTATUS status; + TALLOC_CTX *frame = talloc_stackframe(); + + sid1 = samdb_result_dom_sid(frame, *m1, "objectSid"); + sid2 = samdb_result_dom_sid(frame, *m2, "objectSid"); + + /* + * If entries don't have a SID we want to sort them to the end of + * the list. + */ + if (sid1 == NULL && sid2 == NULL) { + res = 0; + goto exit; + } else if (sid2 == NULL) { + res = 1; + goto exit; + } else if (sid1 == NULL) { + res = -1; + goto exit; + } + + /* + * Get and compare the rids, if we fail to extract a rid treat it as a + * missing SID and sort to the end of the list + */ + status = dom_sid_split_rid(NULL, sid1, NULL, &rid1); + if (!NT_STATUS_IS_OK(status)) { + res = 1; + goto exit; + } + + status = dom_sid_split_rid(NULL, sid2, NULL, &rid2); + if (!NT_STATUS_IS_OK(status)) { + res = -1; + goto exit; + } + + if (rid1 == rid2) { + res = 0; + } + else if (rid1 > rid2) { + res = 1; + } + else { + res = -1; + } +exit: + TALLOC_FREE(frame); + return res; +} + /* samr_EnumDomainGroups */ @@ -1134,11 +1191,17 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call, struct dcesrv_handle *h; struct samr_domain_state *d_state; struct ldb_message **res; - int i, ldb_cnt; - uint32_t first, count; + uint32_t i; + uint32_t count; + uint32_t results; + uint32_t max_entries; + uint32_t remaining_entries; + uint32_t resume_handle; struct samr_SamEntry *entries; const char * const attrs[] = { "objectSid", "sAMAccountName", NULL }; + const char * const cache_attrs[] = { "objectSid", "objectGUID", NULL }; struct samr_SamArray *sam; + struct samr_guid_cache *cache = NULL; *r->out.resume_handle = 0; *r->out.sam = NULL; @@ -1147,77 +1210,173 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call, DCESRV_PULL_HANDLE(h, r->in.domain_handle, SAMR_HANDLE_DOMAIN); d_state = h->data; + cache = &d_state->guid_caches[SAMR_ENUM_DOMAIN_GROUPS_CACHE]; - /* search for all domain groups in this domain. This could possibly be - cached and resumed based on resume_key */ - ldb_cnt = samdb_search_domain(d_state->sam_ctx, mem_ctx, - d_state->domain_dn, &res, attrs, - d_state->domain_sid, - "(&(|(groupType=%d)(groupType=%d))(objectClass=group))", - GTYPE_SECURITY_UNIVERSAL_GROUP, - GTYPE_SECURITY_GLOBAL_GROUP); - if (ldb_cnt < 0) { - return NT_STATUS_INTERNAL_DB_CORRUPTION; - } + /* + * If the resume_handle is zero, query the database and cache the + * matching GUID's + */ + if (*r->in.resume_handle == 0) { + NTSTATUS status; + int ldb_cnt; + clear_guid_cache(cache); + /* + * search for all domain groups in this domain. + */ + ldb_cnt = samdb_search_domain( + d_state->sam_ctx, + mem_ctx, + d_state->domain_dn, + &res, + cache_attrs, + d_state->domain_sid, + "(&(|(groupType=%d)(groupType=%d))(objectClass=group))", + GTYPE_SECURITY_UNIVERSAL_GROUP, + GTYPE_SECURITY_GLOBAL_GROUP); + if (ldb_cnt < 0) { + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + /* + * Sort the results into RID order, while the spec states there + * is no order, Windows appears to sort the results by RID and + * so it is possible that there are clients that depend on + * this ordering + */ + TYPESAFE_QSORT(res, ldb_cnt, compare_msgRid); - /* convert to SamEntry format */ - entries = talloc_array(mem_ctx, struct samr_SamEntry, ldb_cnt); - if (!entries) { - return NT_STATUS_NO_MEMORY; + /* + * cache the sorted GUID's + */ + status = load_guid_cache(cache, d_state, ldb_cnt, res); + TALLOC_FREE(res); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + cache->handle = 0; } - count = 0; - for (i=0;iin.resume_handle >= cache->size) { + clear_guid_cache(cache); + sam = talloc(mem_ctx, struct samr_SamArray); + if (!sam) { + return NT_STATUS_NO_MEMORY; } + sam->entries = NULL; + sam->count = 0; - entries[count].idx = - group_sid->sub_auths[group_sid->num_auths-1]; - entries[count].name.string = - ldb_msg_find_attr_as_string(res[i], "sAMAccountName", ""); - count += 1; + *r->out.sam = sam; + *r->out.resume_handle = 0; + return NT_STATUS_OK; } - /* sort the results by rid */ - TYPESAFE_QSORT(entries, count, compare_SamEntry); - /* find the first entry to return */ - for (first=0; - firstin.resume_handle; - first++) ; + /* + * Calculate the number of entries to return limit by max_size. + * Note that we use the w2k3 element size value of 54 + */ + max_entries = 1 + (r->in.max_size/SAMR_ENUM_USERS_MULTIPLIER); + remaining_entries = cache->size - *r->in.resume_handle; + results = MIN(remaining_entries, max_entries); - /* return the rest, limit by max_size. Note that we - use the w2k3 element size value of 54 */ - *r->out.num_entries = count - first; - *r->out.num_entries = MIN(*r->out.num_entries, - 1+(r->in.max_size/SAMR_ENUM_USERS_MULTIPLIER)); + /* + * Process the list of result GUID's. + * Read the details of each object and populate the Entries + * for the current level. + */ + count = 0; + resume_handle = *r->in.resume_handle; + entries = talloc_array(mem_ctx, struct samr_SamEntry, results); + if (entries == NULL) { + clear_guid_cache(cache); + return NT_STATUS_NO_MEMORY; + } + for (i = 0; i < results; i++) { + struct dom_sid *sid; + struct ldb_result *rec; + const uint32_t idx = *r->in.resume_handle + i; + int ret; + const char *name = NULL; + + resume_handle++; + /* + * Read an object from disk using the GUID as the key + * + * If the object can not be read, or it does not have a SID + * it is ignored. + * + * As a consequence of this, if all the remaining GUID's + * have been deleted an empty result will be returned. + * i.e. even if the previous call returned a non zero + * resume_handle it is possible for no results to be returned. + * + */ + ret = dsdb_search_by_dn_guid(d_state->sam_ctx, + mem_ctx, + &rec, + &cache->entries[idx], + attrs, + 0); + if (ret == LDB_ERR_NO_SUCH_OBJECT) { + char *guid_str = + GUID_string(mem_ctx, &cache->entries[idx]); + DBG_WARNING("GUID [%s] not found\n", guid_str); + continue; + } else if (ret != LDB_SUCCESS) { + clear_guid_cache(cache); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + sid = samdb_result_dom_sid(mem_ctx, rec->msgs[0], "objectSID"); + if (sid == NULL) { + char *guid_str = + GUID_string(mem_ctx, &cache->entries[idx]); + DBG_WARNING("objectSID for GUID [%s] not found\n", + guid_str); + continue; + } + entries[count].idx = sid->sub_auths[sid->num_auths - 1]; + name = ldb_msg_find_attr_as_string( + rec->msgs[0], "sAMAccountName", ""); + entries[count].name.string = talloc_strdup(entries, name); + count++; + } sam = talloc(mem_ctx, struct samr_SamArray); if (!sam) { + clear_guid_cache(cache); return NT_STATUS_NO_MEMORY; } - sam->entries = entries+first; - sam->count = *r->out.num_entries; + sam->entries = entries; + sam->count = count; *r->out.sam = sam; + *r->out.resume_handle = resume_handle; + *r->out.num_entries = count; - if (first == count) { + /* + * Signal no more results by returning zero resume handle, + * the cache is also cleared at this point + */ + if (*r->out.resume_handle >= cache->size) { + *r->out.resume_handle = 0; + clear_guid_cache(cache); return NT_STATUS_OK; } - - if (*r->out.num_entries < count - first) { - *r->out.resume_handle = entries[first+*r->out.num_entries-1].idx; - return STATUS_MORE_ENTRIES; - } - - return NT_STATUS_OK; + /* + * There are more results to be returned. + */ + return STATUS_MORE_ENTRIES; } diff --git a/source4/rpc_server/samr/dcesrv_samr.h b/source4/rpc_server/samr/dcesrv_samr.h index f08bac053c8..04866aca757 100644 --- a/source4/rpc_server/samr/dcesrv_samr.h +++ b/source4/rpc_server/samr/dcesrv_samr.h @@ -53,6 +53,7 @@ struct samr_guid_cache { enum samr_guid_cache_id { SAMR_QUERY_DISPLAY_INFO_CACHE, + SAMR_ENUM_DOMAIN_GROUPS_CACHE, SAMR_LAST_CACHE };