The branch, master has been updated via 6206e15b4de winbind: Fix "wbinfo -u" on a Samba AD DC with >1000 users via f633389f36e winbind: Test wbinfo -u with more than 1000 users via 5ac65fdf9ac build:wafsamba: Fix TypeError in read_submodule_status() via 1dbdeaa8d7f gp: get_gpo() should re-raise the Exception, not return via 9755206f6dd s4:ntvfs:posix: avoid parsing empty blob in posix_eadb_add_list() via 46ae5568fa7 lib:ldb: do not offset against NULL pointer in ldb_ldif_read() from 5fcb675a8b0 s4/scripting: fix % len(res) was in the wrong place
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 6206e15b4de0ba67d713124c2be353dabf3878c8 Author: Volker Lendecke <v...@samba.org> Date: Wed Apr 26 17:19:29 2023 +0200 winbind: Fix "wbinfo -u" on a Samba AD DC with >1000 users BUG: https://bugzilla.samba.org/show_bug.cgi?id=15366 Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Tue May 9 02:58:45 UTC 2023 on atb-devel-224 commit f633389f36e79d3e772777ad7ca13012e3616273 Author: Volker Lendecke <v...@samba.org> Date: Thu Apr 27 12:25:24 2023 +0200 winbind: Test wbinfo -u with more than 1000 users winbind asks dcerpc_samr_LookupRids in one batch, where samr.idl has NTSTATUS samr_LookupRids( [in,ref] policy_handle *domain_handle, [in,range(0,1000)] uint32 num_rids, [in,size_is(1000),length_is(num_rids)] uint32 rids[], [out,ref] lsa_Strings *names, [out,ref] samr_Ids *types ); limiting num_rids to 1000 entries. Test this. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15366 Signed-off-by: Volker Lendecke <v...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 5ac65fdf9acb286a152032cc8913b5ce28fe30fc Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Thu May 4 15:25:31 2023 +1200 build:wafsamba: Fix TypeError in read_submodule_status() parts = l.split(" ") ^^^^^^^^^^^^ TypeError: a bytes-like object is required, not 'str' Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1dbdeaa8d7fcfa4b620bbd24e457ee7f2e6c132d Author: David Mulder <dmul...@samba.org> Date: Fri Apr 28 07:37:31 2023 -0600 gp: get_gpo() should re-raise the Exception, not return If we return from this failure, then `new_gpo` is set to `None` and we will fail in some obscure way within a CSE later (since we append `None` to the GPO list). Instead, re-raise the Exception so we see that an error happened when fetching the GPO. Signed-off-by: David Mulder <dmul...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9755206f6dde7ee4f9852bbd81cb79f4457faf86 Author: Dmitry Antipov <danti...@cloudlinux.com> Date: Tue May 2 13:45:01 2023 +0300 s4:ntvfs:posix: avoid parsing empty blob in posix_eadb_add_list() Strictly speaking, this is not a bug because parsing loop will just skip an empty ({NULL}, 0) blob. But it's better to avoid this case because UBSan (as of clang-17 at least) may complain on such a parsing attempt: source4/ntvfs/posix/posix_eadb.c:56:62: runtime error: applying zero offset to null pointer #0 0x7f9d71ce7b2a in posix_eadb_add_list source4/ntvfs/posix/posix_eadb.c:56 #1 0x7f9d71ce7b2a in push_xattr_blob_tdb_raw source4/ntvfs/posix/posix_eadb.c:178 #2 0x7f9d71cec1f5 in py_wrap_setxattr source4/ntvfs/posix/python/pyposix_eadb.c:64 #3 0x7f9d88bd4507 in cfunction_call (/lib64/libpython3.11.so.1.0+0x1d4507) [... a lot of Python calls skipped...] Signed-off-by: Dmitry Antipov <danti...@cloudlinux.com> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 46ae5568fa7b9a96018d0eedadee6400632112ba Author: Dmitry Antipov <danti...@cloudlinux.com> Date: Tue May 2 13:43:54 2023 +0300 lib:ldb: do not offset against NULL pointer in ldb_ldif_read() Fix the following error observed running samba.test.registry compiled with clang-17 and UBsan: lib/ldb/common/ldb_ldif.c:881:9: runtime error: applying non-zero offset 137438953440 to null pointer #0 0x7faa0eb3932f in ldb_ldif_read lib/ldb/common/ldb_ldif.c:881 #1 0x7faa0eb3aec6 in ldb_ldif_read_string lib/ldb/common/ldb_ldif.c:1004 #2 0x7faa077ed759 in dsdb_set_schema_from_ldif source4/dsdb/schema/schema_set.c:1113 #3 0x7faa068fcbbf in py_dsdb_set_schema_from_ldif source4/dsdb/pydsdb.c:929 #4 0x7faa1d1d4507 in cfunction_call (/lib64/libpython3.11.so.1.0+0x1d4507) [... a lot of Python calls skipped...] I.e. number of elements should be checked against zero before making an attempt to access an element by index. Signed-off-by: Dmitry Antipov <danti...@cloudlinux.com> Reviewed-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: buildtools/wafsamba/samba_git.py | 1 + lib/ldb/common/ldb_ldif.c | 8 +- python/samba/gp/gpclass.py | 2 +- source3/script/tests/test_wbinfo_u_large_ad.sh | 28 +++++++ source3/winbindd/winbindd_samr.c | 102 +++++++++++++++---------- source4/ntvfs/posix/posix_eadb.c | 22 +++--- source4/selftest/tests.py | 5 ++ 7 files changed, 114 insertions(+), 54 deletions(-) create mode 100755 source3/script/tests/test_wbinfo_u_large_ad.sh Changeset truncated at 500 lines: diff --git a/buildtools/wafsamba/samba_git.py b/buildtools/wafsamba/samba_git.py index 09a204f1f4f..fe540ecccff 100644 --- a/buildtools/wafsamba/samba_git.py +++ b/buildtools/wafsamba/samba_git.py @@ -43,6 +43,7 @@ def read_submodule_status(path, env=None): cwd=path) (stdout, stderr) = p.communicate(None) for l in stdout.splitlines(): + l = l.decode('utf-8') l = l.rstrip() status = l[0] l = l[1:] diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c index 748e44ed2b9..96237dd0abf 100644 --- a/lib/ldb/common/ldb_ldif.c +++ b/lib/ldb/common/ldb_ldif.c @@ -878,12 +878,12 @@ struct ldb_ldif *ldb_ldif_read(struct ldb_context *ldb, continue; } - el = &msg->elements[msg->num_elements-1]; - a = ldb_schema_attribute_by_name(ldb, attr); + el = (msg->num_elements > 0 + ? &msg->elements[msg->num_elements - 1] + : NULL); - if (msg->num_elements > 0 && ldb_attr_cmp(attr, el->name) == 0 && - flags == el->flags) { + if (el && ldb_attr_cmp(attr, el->name) == 0 && flags == el->flags) { /* its a continuation */ el->values = talloc_realloc(msg->elements, el->values, diff --git a/python/samba/gp/gpclass.py b/python/samba/gp/gpclass.py index 4992f17eeac..6083a0f8029 100644 --- a/python/samba/gp/gpclass.py +++ b/python/samba/gp/gpclass.py @@ -618,7 +618,7 @@ def get_gpo(samdb, gpo_dn): controls=['sd_flags:1:%d' % sd_flags]) except Exception: log.error('Failed to fetch gpo object with nTSecurityDescriptor') - return + raise if res.count != 1: raise ldb.LdbError(ldb.ERR_NO_SUCH_OBJECT, 'get_gpo: search failed') diff --git a/source3/script/tests/test_wbinfo_u_large_ad.sh b/source3/script/tests/test_wbinfo_u_large_ad.sh new file mode 100755 index 00000000000..ab5f0ca1f6a --- /dev/null +++ b/source3/script/tests/test_wbinfo_u_large_ad.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +LDBMODIFY="$VALGRIND ${LDBMODIFY:-$BINDIR/ldbmodify} $CONFIGURATION" +LDBSEARCH="$VALGRIND ${LDBSEARCH:-$BINDIR/ldbsearch} $CONFIGURATION" +WBINFO="$VALGRIND ${WBINFO:-$BINDIR/wbinfo} $CONFIGURATION" + +NUM_USERS=1234 + +BASE_DN=$($LDBSEARCH -H ldap://$DC_SERVER -b "" --scope=base defaultNamingContext | awk '/^defaultNamingContext/ {print $2}') + +incdir=$(dirname $0)/../../../testprogs/blackbox +. $incdir/subunit.sh + +seq -w 1 "$NUM_USERS" | + xargs -INUM echo -e "dn:cn=large_ad_NUM,cn=users,$BASE_DN\nchangetype:add\nobjectclass:user\nsamaccountname:large_ad_NUM\n" | + $LDBMODIFY -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD" + +testit_grep_count \ + "Make sure $NUM_USERS $DOMAIN users are returned" \ + "$DOMAIN/large_ad_" \ + "$NUM_USERS" \ + ${WBINFO} -u || failed=$(expr $failed + 1) + +seq -w 1 "$NUM_USERS" | + xargs -INUM echo -e "dn:cn=large_ad_NUM,cn=users,$BASE_DN\nchangetype:delete\n" | + $LDBMODIFY -H ldap://$DC_SERVER -U "$DOMAIN\Administrator%$DC_PASSWORD" + +testok $0 $failed diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c index ebf9c24b9e4..92dd1851abd 100644 --- a/source3/winbindd/winbindd_samr.c +++ b/source3/winbindd/winbindd_samr.c @@ -914,8 +914,6 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain, struct rpc_pipe_client *samr_pipe = NULL; struct dcerpc_binding_handle *h = NULL; struct policy_handle dom_pol = { .handle_type = 0, }; - struct lsa_Strings lsa_names = { .count = 0, }; - struct samr_Ids samr_types = { .count = 0, }; enum lsa_SidType *types = NULL; char **names = NULL; const char *domain_name = NULL; @@ -997,49 +995,73 @@ again: } h = samr_pipe->binding_handle; - status = dcerpc_samr_LookupRids( - h, - tmp_ctx, - &dom_pol, - num_rids, - rids, - &lsa_names, - &samr_types, - &result); - - if (!retry && reset_connection_on_error(domain, samr_pipe, status)) { - retry = true; - goto again; - } + /* + * Magic number 1000 comes from samr.idl + */ - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("dcerpc_samr_LookupRids failed: %s\n", - nt_errstr(status)); - goto fail; - } - if (!NT_STATUS_IS_OK(result) && - !NT_STATUS_EQUAL(result, STATUS_SOME_UNMAPPED)) { - DBG_DEBUG("dcerpc_samr_LookupRids resulted in %s\n", - nt_errstr(result)); - status = result; - goto fail; - } + for (i = 0; i < num_rids; i += 1000) { + uint32_t num_lookup_rids = MIN(num_rids - i, 1000); + struct lsa_Strings lsa_names = { + .count = 0, + }; + struct samr_Ids samr_types = { + .count = 0, + }; + uint32_t j; + + status = dcerpc_samr_LookupRids(h, + tmp_ctx, + &dom_pol, + num_lookup_rids, + &rids[i], + &lsa_names, + &samr_types, + &result); + + if (!retry && + reset_connection_on_error(domain, samr_pipe, status)) { + retry = true; + goto again; + } - for (i=0; i<num_rids; i++) { - types[i] = samr_types.ids[i]; - names[i] = talloc_move( - names, - discard_const_p(char *, &lsa_names.names[i].string)); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dcerpc_samr_LookupRids failed: %s\n", + nt_errstr(status)); + goto fail; + } + if (!NT_STATUS_IS_OK(result) && + !NT_STATUS_EQUAL(result, STATUS_SOME_UNMAPPED)) { + DBG_DEBUG("dcerpc_samr_LookupRids resulted in %s\n", + nt_errstr(result)); + status = result; + goto fail; + } - if (names[i] != NULL) { - char *normalized = NULL; - NTSTATUS nstatus = normalize_name_map( - names, domain_name, names[i], &normalized); - if (NT_STATUS_IS_OK(nstatus) || - NT_STATUS_EQUAL(nstatus, NT_STATUS_FILE_RENAMED)) { - names[i] = normalized; + for (j = 0; j < num_lookup_rids; j++) { + uint32_t dst = i + j; + + types[dst] = samr_types.ids[j]; + names[dst] = talloc_move( + names, + discard_const_p(char *, + &lsa_names.names[j].string)); + if (names[dst] != NULL) { + char *normalized = NULL; + NTSTATUS nstatus = + normalize_name_map(names, + domain_name, + names[dst], + &normalized); + if (NT_STATUS_IS_OK(nstatus) || + NT_STATUS_EQUAL(nstatus, + NT_STATUS_FILE_RENAMED)) { + names[dst] = normalized; + } } } + + TALLOC_FREE(samr_types.ids); + TALLOC_FREE(lsa_names.names); } done: diff --git a/source4/ntvfs/posix/posix_eadb.c b/source4/ntvfs/posix/posix_eadb.c index e08597c1c19..2d181fd42a4 100644 --- a/source4/ntvfs/posix/posix_eadb.c +++ b/source4/ntvfs/posix/posix_eadb.c @@ -37,7 +37,6 @@ static NTSTATUS posix_eadb_add_list(struct tdb_wrap *ea_tdb, TALLOC_CTX *ctx, co { DATA_BLOB blob; TALLOC_CTX *mem_ctx; - const char *s; NTSTATUS status; size_t len; @@ -49,15 +48,20 @@ static NTSTATUS posix_eadb_add_list(struct tdb_wrap *ea_tdb, TALLOC_CTX *ctx, co status = pull_xattr_blob_tdb_raw(ea_tdb, mem_ctx, XATTR_LIST_ATTR, fname, fd, 100, &blob); - if (!NT_STATUS_IS_OK(status)) { - blob = data_blob(NULL, 0); - } - - for (s=(const char *)blob.data; s < (const char *)(blob.data+blob.length); s += strlen(s) + 1) { - if (strcmp(attr_name, s) == 0) { - talloc_free(mem_ctx); - return NT_STATUS_OK; + if (NT_STATUS_IS_OK(status)) { + const char *s; + + for (s = (const char *)blob.data; + s < (const char *)(blob.data + blob.length); + s += strlen(s) + 1) { + if (strcmp(attr_name, s) == 0) { + talloc_free(mem_ctx); + return NT_STATUS_OK; + } } + } else { + blob = data_blob(NULL, 0); + /* No need to parse an empty blob */ } len = strlen(attr_name) + 1; diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 19764a14397..b9cbdfed223 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1012,6 +1012,11 @@ for env in ["nt4_dc", "nt4_member", "ad_dc", "ad_member", "chgdcpass", "rodc"]: planpythontestsuite(env + ":local", "samba.tests.ntlm_auth") +plantestsuite( + "samba.wbinfo_u_large_ad.(ad_dc:local)", + "ad_dc:local", + [os.path.join(samba3srcdir, "script/tests/test_wbinfo_u_large_ad.sh")]) + for env in ["ktest"]: planpythontestsuite(env + ":local", "samba.tests.ntlm_auth_krb5") -- Samba Shared Repository