Merge branch 'main' into fix/dns-af-unspec-fallback

2536442a changed _is_signing_required from (self, conn, smbv1) to a
method reading self.conn and self.smbv1, but the tests kept passing conn
and smbv1 as arguments and failed with TypeError.

nxc/connection.py

@@ -10,7 +10,7 @@ from functools import wraps
 from time import sleep
 from ipaddress import ip_address
 from dns import resolver, rdatatype
-from socket import AF_UNSPEC, SOCK_DGRAM, IPPROTO_IP, AI_CANONNAME, getaddrinfo
+from socket import AF_INET, AF_INET6, AF_UNSPEC, SOCK_DGRAM, IPPROTO_IP, AI_CANONNAME, getaddrinfo
 
 from nxc.config import pwned_label
 from nxc.helpers.logger import highlight
@@ -30,6 +30,26 @@ global_failed_logins = 0
 user_failed_logins = {}
 
 
+def getaddrinfo_by_family(target, families):
+    """Resolve target with getaddrinfo, keeping the first address of each family."""
+    addresses = {"AF_INET": "", "AF_INET6": ""}
+    canonname = ""
+
+    for family in families:
+        try:
+            results = getaddrinfo(target, None, family, SOCK_DGRAM, IPPROTO_IP, AI_CANONNAME)
+        except OSError:
+            continue
+
+        for af, _, _, name, sa in results:
+            if not canonname:
+                canonname = name
+            if not addresses[af.name]:
+                addresses[af.name] = sa[0]
+
+    return addresses, canonname
+
+
 def get_host_addr_info(target, force_ipv6, dns_server, dns_tcp, dns_timeout):
     result = {
         "host": "",
@@ -46,9 +66,12 @@ def get_host_addr_info(target, force_ipv6, dns_server, dns_tcp, dns_timeout):
     except Exception:
         # If the target is not an IP address, we need to resolve it
         if not (dns_server or dns_tcp):
-            for res in getaddrinfo(target, None, AF_UNSPEC, SOCK_DGRAM, IPPROTO_IP, AI_CANONNAME):
-                af, _, _, canonname, sa = res
-                address_info[af.name] = sa[0]
+            # glibc fails an AF_UNSPEC lookup outright when one family's UDP answer is
+            # truncated, so retry each family on its own before giving up.
+            addresses, canonname = getaddrinfo_by_family(target, (AF_UNSPEC,))
+            if not (addresses["AF_INET"] or addresses["AF_INET6"]):
+                addresses, canonname = getaddrinfo_by_family(target, (AF_INET, AF_INET6))
+            address_info.update(addresses)
 
             if address_info["AF_INET6"] and ip_address(address_info["AF_INET6"]).is_link_local:
                 address_info["AF_INET6"] = canonname

tests/test_dns_resolution.py

@@ -0,0 +1,130 @@
+"""Regression tests for hostname resolution (nxc/connection.py).
+
+Background: the system-resolver branch of ``get_host_addr_info`` made a single
+``AF_UNSPEC`` ``getaddrinfo`` call with no fallback. glibc fails an ``AF_UNSPEC``
+lookup with ``EAI_AGAIN`` when one family's UDP answer is truncated, even though
+querying each family on its own succeeds. NetExec then reported the name as
+unresolvable, which left ``self.kdcHost`` unset and broke Kerberos operations.
+
+The dnspython branch already queried A and AAAA separately; the fix makes the
+system-resolver branch behave the same way.
+"""
+from socket import AF_INET, AF_INET6, AF_UNSPEC, EAI_AGAIN, gaierror
+
+import pytest
+
+from nxc.connection import get_host_addr_info
+
+HOST = "dc.example.com"
+
+
+def addrinfo(family, address, canonname=""):
+    return (family, 2, 17, canonname, (address, 0) if family == AF_INET else (address, 0, 0, 0))
+
+
+def fake_getaddrinfo(responses):
+    """Build a getaddrinfo stub driven by a {family: result-or-exception} map."""
+    def stub(target, port, family, *args, **kwargs):
+        outcome = responses[family]
+        if isinstance(outcome, Exception):
+            raise outcome
+        return outcome
+    return stub
+
+
+def resolve(monkeypatch, responses, force_ipv6=False):
+    monkeypatch.setattr("nxc.connection.getaddrinfo", fake_getaddrinfo(responses))
+    return get_host_addr_info(HOST, force_ipv6, None, False, 3)
+
+
+TRUNCATED_AF_UNSPEC = {
+    AF_UNSPEC: gaierror(EAI_AGAIN, "Temporary failure in name resolution"),
+    AF_INET: [addrinfo(AF_INET, "10.0.0.1", HOST)],
+    AF_INET6: [addrinfo(AF_INET6, "2001:db8::1", HOST)],
+}
+
+
+def test_af_unspec_failure_falls_back_to_per_family(monkeypatch):
+    """The regression: AF_UNSPEC fails but both families resolve on their own."""
+    result = resolve(monkeypatch, TRUNCATED_AF_UNSPEC)
+    assert result["host"] == "10.0.0.1"
+    assert result["is_ipv6"] is False
+
+
+def test_af_unspec_failure_falls_back_for_ipv6(monkeypatch):
+    result = resolve(monkeypatch, TRUNCATED_AF_UNSPEC, force_ipv6=True)
+    assert result["host"] == "2001:db8::1"
+    assert result["is_ipv6"] is True
+
+
+def test_af_unspec_success_is_not_retried(monkeypatch):
+    """A working AF_UNSPEC lookup must not trigger the per-family fallback."""
+    calls = []
+    responses = {AF_UNSPEC: [addrinfo(AF_INET, "10.0.0.1", HOST)]}
+
+    def stub(target, port, family, *args, **kwargs):
+        calls.append(family)
+        return responses[family]
+
+    monkeypatch.setattr("nxc.connection.getaddrinfo", stub)
+    result = get_host_addr_info(HOST, False, None, False, 3)
+    assert result["host"] == "10.0.0.1"
+    assert calls == [AF_UNSPEC]
+
+
+def test_first_address_of_each_family_is_kept(monkeypatch):
+    """Matches the dnspython branch, which keeps answers[0] per family."""
+    responses = {
+        AF_UNSPEC: [
+            addrinfo(AF_INET, "10.0.0.1", HOST),
+            addrinfo(AF_INET, "10.0.0.2"),
+            addrinfo(AF_INET6, "2001:db8::1"),
+            addrinfo(AF_INET6, "2001:db8::2"),
+        ]
+    }
+    assert resolve(monkeypatch, responses)["host"] == "10.0.0.1"
+    assert resolve(monkeypatch, responses, force_ipv6=True)["host"] == "2001:db8::1"
+
+
+def test_link_local_ipv6_uses_canonical_name(monkeypatch):
+    """The canonical name comes from the first record, the only one glibc fills."""
+    responses = {
+        AF_UNSPEC: [
+            addrinfo(AF_INET6, "fe80::1", HOST),
+            addrinfo(AF_INET6, "fe80::2"),
+        ]
+    }
+    result = resolve(monkeypatch, responses, force_ipv6=True)
+    assert result["host"] == HOST
+    assert result["is_link_local_ipv6"] is True
+
+
+def test_every_family_failing_raises(monkeypatch):
+    responses = dict.fromkeys(
+        (AF_UNSPEC, AF_INET, AF_INET6),
+        gaierror(EAI_AGAIN, "Temporary failure in name resolution"),
+    )
+    with pytest.raises(Exception, match="The DNS query name does not exist"):
+        resolve(monkeypatch, responses)
+
+
+def test_ip_targets_skip_resolution(monkeypatch):
+    def stub(*args, **kwargs):
+        raise AssertionError("getaddrinfo must not be called for an IP target")
+
+    monkeypatch.setattr("nxc.connection.getaddrinfo", stub)
+    assert get_host_addr_info("10.0.0.1", False, None, False, 3)["host"] == "10.0.0.1"
+    assert get_host_addr_info("2001:db8::1", False, None, False, 3)["host"] == "2001:db8::1"
+
+
+def test_getaddrinfo_by_family_skips_failing_family(monkeypatch):
+    from nxc.connection import getaddrinfo_by_family
+
+    responses = {
+        AF_INET: gaierror(EAI_AGAIN, "Temporary failure in name resolution"),
+        AF_INET6: [addrinfo(AF_INET6, "2001:db8::1", HOST)],
+    }
+    monkeypatch.setattr("nxc.connection.getaddrinfo", fake_getaddrinfo(responses))
+    addresses, canonname = getaddrinfo_by_family(HOST, (AF_INET, AF_INET6))
+    assert addresses == {"AF_INET": "", "AF_INET6": "2001:db8::1"}
+    assert canonname == HOST

tests/test_smb_signing.py

@@ -60,7 +60,7 @@ def test_smb311_signing_enabled_not_required_reports_false(smb_module):
         server_security_mode=SMB2_NEGOTIATE_SIGNING_ENABLED,
         require_signing=True,
     )
-    assert smb_module.smb._is_signing_required(None, conn, smbv1=False) is False
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=False, conn=conn)) is False
 
 
 def test_smb311_signing_required_reports_true(smb_module):
@@ -69,7 +69,7 @@ def test_smb311_signing_required_reports_true(smb_module):
         server_security_mode=SMB2_NEGOTIATE_SIGNING_ENABLED | SMB2_NEGOTIATE_SIGNING_REQUIRED,
         require_signing=True,
     )
-    assert smb_module.smb._is_signing_required(None, conn, smbv1=False) is True
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=False, conn=conn)) is True
 
 
 def test_smb30_signing_not_required_reports_false(smb_module):
@@ -78,7 +78,7 @@ def test_smb30_signing_not_required_reports_false(smb_module):
         server_security_mode=SMB2_NEGOTIATE_SIGNING_ENABLED,
         require_signing=False,
     )
-    assert smb_module.smb._is_signing_required(None, conn, smbv1=False) is False
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=False, conn=conn)) is False
 
 
 def test_smb21_falls_back_to_require_signing(smb_module):
@@ -89,9 +89,9 @@ def test_smb21_falls_back_to_require_signing(smb_module):
         server_security_mode=0,
         require_signing=True,
     )
-    assert smb_module.smb._is_signing_required(None, conn, smbv1=False) is True
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=False, conn=conn)) is True
 
 
 def test_smbv1_uses_session_signing_flag(smb_module):
-    assert smb_module.smb._is_signing_required(None, make_smbv1_conn(True), smbv1=True) is True
-    assert smb_module.smb._is_signing_required(None, make_smbv1_conn(False), smbv1=True) is False
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=True, conn=make_smbv1_conn(True))) is True
+    assert smb_module.smb._is_signing_required(SimpleNamespace(smbv1=True, conn=make_smbv1_conn(False))) is False