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 <nturner@exagrid.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Mulder <dmulder@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Thu Mar  9 19:12:15 UTC 2023 on atb-devel-224

(cherry picked from commit f55a357c6b9387883a7628a1b1083263a10121a6)

Autobuild-User(v4-18-test): Jule Anger <janger@samba.org>
Autobuild-Date(v4-18-test): Mon May  8 10:17:16 UTC 2023 on atb-devel-224

diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
index 42714fcb2a116d8666a2cfd21cafd6a70cdeec1d..e0462d5fb24ced02020b6970dc6c8c499c3135bf 100644 (file)
--- 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);