diff --git a/changedetectionio/tests/unit/test_fetch_url_gate.py b/changedetectionio/tests/unit/test_fetch_url_gate.py index b2c524d91..a247bfd8e 100644 --- a/changedetectionio/tests/unit/test_fetch_url_gate.py +++ b/changedetectionio/tests/unit/test_fetch_url_gate.py @@ -23,11 +23,30 @@ from unittest.mock import patch from changedetectionio.browser_steps.browser_steps import steppable_browser_interface from changedetectionio.validate_url import ( is_fetch_url_allowed, + is_private_hostname, is_special_purpose_ip, validate_fetch_url, validate_fetch_url_async, ) + +def _gai(*addresses): + """Build a socket.getaddrinfo() return value from (address, family[, scope_id]) tuples. + + Only family and sockaddr are read by is_private_hostname(), and the scope id is the whole + point of the link-local tests, so the sockaddr shape has to be the real one: 2-tuple for + AF_INET, 4-tuple (addr, port, flowinfo, scope_id) for AF_INET6. + """ + import socket + out = [] + for addr, version, *rest in addresses: + scope_id = rest[0] if rest else 0 + if version == 6: + out.append((socket.AF_INET6, socket.SOCK_STREAM, 6, '', (addr, 0, 0, scope_id))) + else: + out.append((socket.AF_INET, socket.SOCK_STREAM, 6, '', (addr, 0))) + return out + # tests/conftest.py sets ALLOW_IANA_RESTRICTED_ADDRESSES=true for the functional suite, so the # locked-down default has to be re-asserted explicitly rather than assumed. LOCKED_DOWN = {'ALLOW_IANA_RESTRICTED_ADDRESSES': 'false', 'ALLOW_FILE_URI': 'false'} @@ -168,6 +187,54 @@ class TestFetchUrlGate(unittest.TestCase): with self.subTest(url=url): self.assertAllowed(url) + def test_junk_link_local_aaaa_record_does_not_block_a_public_site(self): + """Real sites publish a bare 'fe80::...' AAAA by mistake - competa.com and upnano.com + both do (a Plesk/TransIP box leaking its own NIC autoconf address into DNS). Nothing can + connect to a link-local address without a zone id, and DNS cannot carry one, so refusing + the whole hostname over that record blocked legitimate sites for no security gain.""" + with patch.dict('os.environ', LOCKED_DOWN): + with patch('socket.getaddrinfo', return_value=_gai( + ('149.210.194.177', 4), ('fe80::5054:ff:fe3b:3cbd', 6))): + self.assertFalse(is_private_hostname('competa.example')) + self.assertAllowed('https://competa.example/') + + def test_link_local_only_hostname_is_still_blocked(self): + """The exception above is only for a junk record riding alongside a usable one. A name + that resolves to nothing but link-local space is what deliberately aiming the fetcher at + its own link segment looks like, so it stays refused.""" + with patch.dict('os.environ', LOCKED_DOWN): + with patch('socket.getaddrinfo', return_value=_gai(('fe80::1', 6))): + self.assertTrue(is_private_hostname('linklocal.example')) + self.assertBlocked('https://linklocal.example/') + + def test_scoped_link_local_is_still_blocked(self): + """A zone id (fe80::1%eth0, scope_id != 0) makes the address genuinely connectable, so + the 'unreachable record' reasoning does not apply to it.""" + with patch.dict('os.environ', LOCKED_DOWN): + with patch('socket.getaddrinfo', return_value=_gai( + ('93.184.216.34', 4), ('fe80::1', 6, 2))): + self.assertTrue(is_private_hostname('scoped.example')) + + def test_link_local_exception_does_not_extend_to_ipv4(self): + """169.254.0.0/16 is link-local too, but it IS routable - and 169.254.169.254 is cloud + metadata. A public A record next to it must not buy it a pass.""" + with patch.dict('os.environ', LOCKED_DOWN): + with patch('socket.getaddrinfo', return_value=_gai( + ('93.184.216.34', 4), ('169.254.169.254', 4))): + self.assertTrue(is_private_hostname('metadata.example')) + self.assertBlocked('https://metadata.example/') + + def test_link_local_exception_does_not_extend_to_other_blocked_ranges(self): + """Only link-local is deferred; a public record alongside CGNAT/private/reserved space + still refuses the hostname (GHSA-gwph-fp79-379w).""" + with patch.dict('os.environ', LOCKED_DOWN): + for other in ('100.64.0.1', '10.0.0.1', '127.0.0.1', 'fc00::1', '::1'): + with self.subTest(other=other): + fam = 6 if ':' in other else 4 + with patch('socket.getaddrinfo', return_value=_gai( + ('93.184.216.34', 4), (other, fam))): + self.assertTrue(is_private_hostname('mixed.example')) + def test_validate_fetch_url_raises_with_the_reason(self): with patch.dict('os.environ', LOCKED_DOWN): with self.assertRaises(ValueError): diff --git a/changedetectionio/validate_url.py b/changedetectionio/validate_url.py index a4c6a725c..8eecaec59 100644 --- a/changedetectionio/validate_url.py +++ b/changedetectionio/validate_url.py @@ -141,6 +141,29 @@ def is_special_purpose_ip(ip): return False, '' +def _is_unreachable_link_local(family, ip, sockaddr): + """True for an IPv6 link-local address that arrived with no zone id, i.e. one nothing can + connect to. + + fe80::/10 is only routable together with an interface scope ('fe80::1%eth0'). DNS AAAA + records cannot carry a zone id, so a link-local answer from getaddrinfo() comes back with + sin6_scope_id == 0 and connect() rejects it outright (EINVAL on Linux) - it is a dead + record, not a route into the fetcher's link segment. + + Deliberately narrow: + * IPv4 link-local (169.254.0.0/16, and with it 169.254.169.254 cloud metadata) is NOT + covered - that one IS reachable and must stay blocked. + * A scoped literal such as http://[fe80::1%25eth0]/ resolves with scope_id != 0 and so + is not covered either. + """ + return ( + family == socket.AF_INET6 + and ip.is_link_local + and len(sockaddr) > 3 + and sockaddr[3] == 0 + ) + + def is_private_hostname(hostname): """Return True if hostname resolves to an IANA-restricted (private/reserved/non-global) IP address. @@ -152,14 +175,29 @@ def is_private_hostname(hostname): A hostname is refused if ANY of its A/AAAA records is off-limits: a name that answers with one public and one CGNAT address is still a route to the CGNAT address. + + ONE exception, and it is narrow (see _is_unreachable_link_local): a bare IPv6 link-local + AAAA record alongside at least one address we would allow. Plenty of real sites publish a + junk 'fe80::...' AAAA - it is the server's own NIC autoconf address leaked into DNS by a + misconfigured control panel - and refusing the whole hostname over a record nothing can + connect to blocks legitimate sites for no security gain. """ + allowed_addresses = 0 + deferred = [] # unreachable link-local records - only fatal if nothing else resolves + try: for info in socket.getaddrinfo(hostname, None): - ip = ipaddress.ip_address(info[4][0]) + family, sockaddr = info[0], info[4] + ip = ipaddress.ip_address(sockaddr[0]) blocked, why = is_special_purpose_ip(ip) - if blocked: - logger.warning(f"Hostname '{hostname}' resolves to {ip} which is {why} — refused.") - return True + if not blocked: + allowed_addresses += 1 + continue + if _is_unreachable_link_local(family, ip, sockaddr): + deferred.append(ip) + continue + logger.warning(f"Hostname '{hostname}' resolves to {ip} which is {why} — refused.") + return True except socket.gaierror as e: logger.warning(f"{hostname} error checking {str(e)}") return False @@ -167,6 +205,19 @@ def is_private_hostname(hostname): # getaddrinfo handed back something ip_address() won't parse - fail closed. logger.warning(f"Hostname '{hostname}' produced an unparseable address ({e}) — refused.") return True + + if deferred and not allowed_addresses: + # Every answer was link-local. Nothing legitimate looks like this, and it is what + # deliberately pointing a public name at the fetcher's own link segment would look + # like, so keep refusing it. + logger.warning(f"Hostname '{hostname}' resolves only to link-local addresses " + f"({', '.join(str(i) for i in deferred)}) — refused.") + return True + if deferred: + logger.debug(f"Hostname '{hostname}' has unroutable link-local AAAA record(s) " + f"({', '.join(str(i) for i in deferred)}) - ignored, it also resolves to " + f"{allowed_addresses} usable address(es).") + logger.info(f"Hostname '{hostname}' is NOT private/IANA restricted.") return False