From fc2ebae7f4364fe73975d7af74c72ddedc071a06 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 8 Jul 2019 16:59:33 +1200 Subject: [PATCH 1/5] ldap_server: Regression in 0559430ab6e5c48d6e853fda0d8b63f2e149015c Extended DN requests seem to have been incorrectly handled. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029 Signed-off-by: Garming Sam Reviewed-by: Gary Lockyer Autobuild-User(master): Gary Lockyer Autobuild-Date(master): Thu Jul 11 05:25:26 UTC 2019 on sn-devel-184 (cherry picked from commit 9f6b87d3f6cc9930d75c1f8d38ad4f5a37da34ab) --- source4/ldap_server/ldap_backend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/ldap_server/ldap_backend.c b/source4/ldap_server/ldap_backend.c index c6a65122ab0..bf724335a25 100644 --- a/source4/ldap_server/ldap_backend.c +++ b/source4/ldap_server/ldap_backend.c @@ -826,6 +826,7 @@ static NTSTATUS ldapsrv_SearchRequest(struct ldapsrv_call *call) } else { extended_type = 0; } + callback_ctx->extended_type = extended_type; } notification_control = ldb_request_get_control(lreq, LDB_CONTROL_NOTIFICATION_OID); -- 2.17.1 From 86957a756ec7910f3a511feb0b776737602c2fae Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 31 Jul 2019 01:08:23 +0000 Subject: [PATCH 2/5] tldap: Make memcpy of no controls safe Static analyzers sometimes complain about this case. Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029 (cherry picked from commit e5452a37425484a95f90604a3e58e8a731460793) --- source3/lib/tldap_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c index 152942dab2c..bdf8eb031a5 100644 --- a/source3/lib/tldap_util.c +++ b/source3/lib/tldap_util.c @@ -588,7 +588,9 @@ struct tldap_control *tldap_add_control(TALLOC_CTX *mem_ctx, if (result == NULL) { return NULL; } - memcpy(result, ctrls, sizeof(struct tldap_control) * num_ctrls); + if (num_ctrls > 0) { + memcpy(result, ctrls, sizeof(struct tldap_control) * num_ctrls); + } result[num_ctrls] = *ctrl; return result; } -- 2.17.1 From a726b72fd9a1ab6dd419e6f6176ba0d0a03b8b2a Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 31 Jul 2019 13:39:13 +1200 Subject: [PATCH 3/5] tldap: Paged searches fail when they get to the end The normal case hit the goto label, and should have just returned. Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029 (cherry picked from commit bff466943e01540b4d3210392e0fd5b1c882c0b9) --- source3/lib/tldap_util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c index bdf8eb031a5..1b86962a32e 100644 --- a/source3/lib/tldap_util.c +++ b/source3/lib/tldap_util.c @@ -810,7 +810,8 @@ static void tldap_search_paged_done(struct tevent_req *subreq) } tevent_req_set_callback(subreq, tldap_search_paged_done, req); - err: + return; +err: TALLOC_FREE(asn1); tevent_req_ldap_error(req, TLDAP_DECODING_ERROR); -- 2.17.1 From 9fb2f77adfd1a1236983e9842eef822f9a8c0ee8 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 31 Jul 2019 15:29:07 +1200 Subject: [PATCH 4/5] tests/tldap: Actually check the paging return code The test never worked correctly because the code was overlooked. It was also the case that the connection was never authenticated, and so an LDAP BIND call has now been added. Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029 (cherry picked from commit 85a7b594c56f7729bdfa194fee9299a08f6b4785) --- source3/torture/torture.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 2cb32efea46..20a7459b4db 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -11286,6 +11286,8 @@ static bool run_shortname_test(int dummy) return correct; } +TLDAPRC callback_code; + static void pagedsearch_cb(struct tevent_req *req) { TLDAPRC rc; @@ -11296,6 +11298,7 @@ static void pagedsearch_cb(struct tevent_req *req) if (!TLDAP_RC_IS_SUCCESS(rc)) { d_printf("tldap_search_paged_recv failed: %s\n", tldap_rc2string(rc)); + callback_code = rc; return; } if (tldap_msg_type(msg) != TLDAP_RES_SEARCH_ENTRY) { @@ -11360,6 +11363,18 @@ static bool run_tldap(int dummy) return false; } + rc = tldap_gensec_bind(ld, torture_creds, "ldap", host, NULL, + loadparm_init_s3(talloc_tos(), + loadparm_s3_helpers()), + GENSEC_FEATURE_SIGN | GENSEC_FEATURE_SEAL); + + if (!TLDAP_RC_IS_SUCCESS(rc)) { + d_printf("tldap_gensec_bind failed\n"); + return false; + } + + callback_code = TLDAP_SUCCESS; + req = tldap_search_paged_send(talloc_tos(), ev, ld, basedn, TLDAP_SCOPE_SUB, "(objectclass=*)", NULL, 0, 0, @@ -11374,6 +11389,14 @@ static bool run_tldap(int dummy) TALLOC_FREE(req); + rc = callback_code; + + if (!TLDAP_RC_IS_SUCCESS(rc)) { + d_printf("tldap_search with paging failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + /* test search filters against rootDSE */ filter = "(&(|(name=samba)(nextRid<=10000000)(usnChanged>=10)(samba~=ambas)(!(name=s*m*a)))" "(|(name:=samba)(name:dn:2.5.13.5:=samba)(:dn:2.5.13.5:=samba)(!(name=*samba))))"; -- 2.17.1 From 0b16851dc6d1a31ed05924bb0ca41b9bb2b43db1 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Wed, 31 Jul 2019 01:14:42 +0000 Subject: [PATCH 5/5] tests/ldap: Use TLDAP to check the extended DN return Tests commit 9f6b87d3f6cc9930d75c1f8d38ad4f5a37da34ab To run: make test TESTS="samba3.smbtorture_s3.plain.TLDAP" Reverting the above commit makes this test fail: 'GUID format in control (no hyphens) doesn't match output tldap_search with extended dn (no val) failed: LDAP error 0 (TLDAP_SUCCESS), TEST TLDAP FAILED!' This behaviour couldn't be tested via LDB libraries because they never deal with the underlying DN string. Signed-off-by: Garming Sam Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14029 Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Thu Aug 1 06:20:28 UTC 2019 on sn-devel-184 (adapted from commit 464fef34d1d047d73be347cd446b74e0f5eb2370) --- source3/torture/torture.c | 155 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 20a7459b4db..f26c634b7a7 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -26,6 +26,7 @@ #include "libcli/security/security.h" #include "tldap.h" #include "tldap_util.h" +#include "tldap_gensec_bind.h" #include "../librpc/gen_ndr/svcctl.h" #include "../lib/util/memcache.h" #include "nsswitch/winbind_client.h" @@ -45,6 +46,9 @@ #include "lib/util/base64.h" #include "lib/util/time.h" #include "lib/gencache.h" +#include "lib/util/asn1.h" +#include "lib/param/param.h" +#include "auth/gensec/gensec.h" #include #include @@ -11313,6 +11317,134 @@ static void pagedsearch_cb(struct tevent_req *req) TALLOC_FREE(msg); } +enum tldap_extended_val { + EXTENDED_ZERO = 0, + EXTENDED_ONE = 1, + EXTENDED_NONE = 2, +}; + +/* + * Construct an extended dn control with either no value, 0 or 1 + * + * No value and 0 are equivalent (non-hyphenated GUID) + * 1 has the hyphenated GUID + */ +static struct tldap_control * +tldap_build_extended_control(enum tldap_extended_val val) +{ + struct tldap_control empty_control; + struct asn1_data *data; + + ZERO_STRUCT(empty_control); + + if (val != EXTENDED_NONE) { + data = asn1_init(talloc_tos()); + + if (!data) { + return NULL; + } + + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) { + return NULL; + } + + if (!asn1_write_Integer(data, (int)val)) { + return NULL; + } + + if (!asn1_pop_tag(data)) { + return NULL; + } + + if (!asn1_blob(data, &empty_control.value)) { + return NULL; + } + } + + empty_control.oid = "1.2.840.113556.1.4.529"; + empty_control.critical = true; + + return tldap_add_control(talloc_tos(), NULL, 0, &empty_control); + +} + +static bool tldap_test_dn_guid_format(struct tldap_context *ld, const char *basedn, + enum tldap_extended_val control_val) +{ + struct tldap_control *control = tldap_build_extended_control(control_val); + char *dn = NULL; + struct tldap_message **msg; + TLDAPRC rc; + + rc = tldap_search(ld, basedn, TLDAP_SCOPE_BASE, + "(objectClass=*)", NULL, 0, 0, + control, 1, NULL, + 0, 0, 0, 0, talloc_tos(), &msg); + if (!TLDAP_RC_IS_SUCCESS(rc)) { + d_printf("tldap_search for domain DN failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + + if (!tldap_entry_dn(msg[0], &dn)) { + d_printf("tldap_search domain DN fetch failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + + d_printf("%s\n", dn); + { + uint32_t time_low; + uint32_t time_mid, time_hi_and_version; + uint32_t clock_seq[2]; + uint32_t node[6]; + char next; + + switch (control_val) { + case EXTENDED_NONE: + case EXTENDED_ZERO: + /* + * When reading GUIDs with hyphens, scanf will treat + * hyphen as a hex character (and counts as part of the + * width). This creates leftover GUID string which we + * check will for with 'next' and closing '>'. + */ + if (12 == sscanf(dn, "%c", + &time_low, &time_mid, + &time_hi_and_version, &clock_seq[0], + &clock_seq[1], &node[0], &node[1], + &node[2], &node[3], &node[4], + &node[5], &next)) { + /* This GUID is good */ + } else { + d_printf("GUID format in control (no hyphens) doesn't match output\n"); + return false; + } + + break; + case EXTENDED_ONE: + if (12 == sscanf(dn, + "%c", + &time_low, &time_mid, + &time_hi_and_version, &clock_seq[0], + &clock_seq[1], &node[0], &node[1], + &node[2], &node[3], &node[4], + &node[5], &next)) { + /* This GUID is good */ + } else { + d_printf("GUID format in control (with hyphens) doesn't match output\n"); + return false; + } + + break; + default: + return false; + } + } + + return true; +} + static bool run_tldap(int dummy) { struct tldap_context *ld; @@ -11410,6 +11542,29 @@ static bool run_tldap(int dummy) return false; } + /* + * Tests to check for regression of: + * + * https://bugzilla.samba.org/show_bug.cgi?id=14029 + * + * TLDAP used here to pick apart the original string DN (with GUID) + */ + if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_NONE)) { + d_printf("tldap_search with extended dn (no val) failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_ZERO)) { + d_printf("tldap_search with extended dn (0) failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + if (!tldap_test_dn_guid_format(ld, basedn, EXTENDED_ONE)) { + d_printf("tldap_search with extended dn (1) failed: %s\n", + tldap_errstr(talloc_tos(), ld, rc)); + return false; + } + TALLOC_FREE(ld); return true; } -- 2.17.1