From 7a7ca7808404d130b3b7ebe3590f57c76b230644 Mon Sep 17 00:00:00 2001 From: "Nathaniel W. Turner" Date: Fri, 23 Sep 2022 16:37:46 -0400 Subject: [PATCH] dsgetdcname: do not assume local system uses IPv4 Return the first IPv4 and the first IPv6 address found for each DC. This is slightly inelegant, but resolves an issue where IPv6-only systems were unable to run "net ads join" against domain controllers that have both A and AAAA records in DNS. While this impacts performance due to the additional LDAP ping attempts, in practice an attempt to connect to an IPv6 address on an IPv4-only system (or vice versa) will fail immediately with NT_STATUS_NETWORK_UNREACHABLE, and thus the performance impact should be negligible. The alternative approach, using an smb.conf setting to control whether the logic prefers a single address of one family or the other ends up being a bit awkward, as it pushes the problem onto admins and tools such as "realm join" that want to dynamically synthesize an smb.conf on the fly. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15325 Signed-off-by: Nathaniel W. Turner Reviewed-by: Jeremy Allison Reviewed-by: David Mulder Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Mar 9 19:12:15 UTC 2023 on atb-devel-224 (cherry picked from commit f55a357c6b9387883a7628a1b1083263a10121a6) --- source3/libsmb/dsgetdcname.c | 49 +++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c index 42714fcb2a1..e0462d5fb24 100644 --- a/source3/libsmb/dsgetdcname.c +++ b/source3/libsmb/dsgetdcname.c @@ -551,14 +551,20 @@ static NTSTATUS discover_dc_dns(TALLOC_CTX *mem_ctx, return NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND; } + /* Check for integer wrap. */ + if (numdcs + numdcs < numdcs) { + TALLOC_FREE(dcs); + return NT_STATUS_INVALID_PARAMETER; + } + /* - * We're only returning one address per - * DC name, so just allocate size numdcs. + * We're only returning up to 2 addresses per + * DC name, so just allocate size numdcs x 2. */ dclist = talloc_zero_array(mem_ctx, struct ip_service_name, - numdcs); + numdcs * 2); if (!dclist) { TALLOC_FREE(dcs); return NT_STATUS_NO_MEMORY; @@ -571,17 +577,16 @@ static NTSTATUS discover_dc_dns(TALLOC_CTX *mem_ctx, ret_count = 0; for (i = 0; i < numdcs; i++) { size_t j; + bool have_v4_addr = false; + bool have_v6_addr = false; if (dcs[i].num_ips == 0) { continue; } - dclist[ret_count].hostname = - talloc_move(dclist, &dcs[i].hostname); - /* - * Pick the first IPv4 address, - * if none pick the first address. + * Pick up to 1 address from each address + * family (IPv4, IPv6). * * This is different from the previous * code which picked a 'next ip' address @@ -589,8 +594,11 @@ static NTSTATUS discover_dc_dns(TALLOC_CTX *mem_ctx, * Too complex to maintain :-(. */ for (j = 0; j < dcs[i].num_ips; j++) { - if (dcs[i].ss_s[j].ss_family == AF_INET) { + if ((dcs[i].ss_s[j].ss_family == AF_INET && !have_v4_addr) || + (dcs[i].ss_s[j].ss_family == AF_INET6 && !have_v6_addr)) { bool ok; + dclist[ret_count].hostname = + talloc_strdup(dclist, dcs[i].hostname); ok = sockaddr_storage_to_samba_sockaddr( &dclist[ret_count].sa, &dcs[i].ss_s[j]); @@ -599,22 +607,17 @@ static NTSTATUS discover_dc_dns(TALLOC_CTX *mem_ctx, TALLOC_FREE(dclist); return NT_STATUS_INVALID_PARAMETER; } - break; - } - } - if (j == dcs[i].num_ips) { - /* No IPv4- use the first IPv6 addr. */ - bool ok; - ok = sockaddr_storage_to_samba_sockaddr( - &dclist[ret_count].sa, - &dcs[i].ss_s[0]); - if (!ok) { - TALLOC_FREE(dcs); - TALLOC_FREE(dclist); - return NT_STATUS_INVALID_PARAMETER; + ret_count++; + if (dcs[i].ss_s[j].ss_family == AF_INET) { + have_v4_addr = true; + } else { + have_v6_addr = true; + } + if (have_v4_addr && have_v6_addr) { + break; + } } } - ret_count++; } TALLOC_FREE(dcs); -- 2.34.1