From: Alex Rousskov Date: Tue, 7 Nov 2023 02:50:47 +0000 (+0000) Subject: Fix ipv4 and expand ipv6 ACL parameter matching (#1568) X-Git-Tag: SQUID_7_0_1~293 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=51c518d5c60b42db3277c8902a3299449f4f1f91;p=thirdparty%2Fsquid.git Fix ipv4 and expand ipv6 ACL parameter matching (#1568) The undocumented "ipv4", "ipv6", and "all" parameters can be used in src, dst, and localip ACLs. This refactoring fixes "ipv4" matching and expands "ipv6" scope as detailed further below. These parameters remain undocumented. Together, the fixed ipv4 parameter and the adjusted ipv6 parameter now match any IP address known to Squid. So does the undocumented "all" parameter (which is currently used to implement the "all" dst ACL). Also speed up matching of ACLs containing "ipv4", "ipv6", and "all" parameters (at the cost of adding two boolean tests to other src, dst, and localip ACLs). This performance improvement is a side effect of the corresponding code improvements and is not the focus of these changes. Also improved mgr:config reporting of ACLs containing these special parameters. They are now reported using their names (e.g., "all") rather than IP addresses (e.g., `::/0` instead of "all"). ### Fix ipv4 ACL parameter to match (any IPv4 address) The "ipv4" parameter is supposed to match any IPv4 address, but it matched none because an unmasked[^1] IPv4 input address was compared to an all-0s address[^2], while any IPv4 address in Squid has some 1s due to IPv4-to-IPv6 mapping used by Ip::Address. Those 1s could not match. The "ipv4" parameter did match one address -- an IPv6 address `::`[^2]. This matching was possibly broken since inception (2009 commit 7764e92). [^1]: Calling mask.setNoAddr() sets all sin6_addr bits to 1 while calling mask.applyMask(0, AF_INET) does nothing at all. [^2]: We did not set q->addr1. Ip::Address default constructor sets all Ip::Address bits to 0. For ACL IP comparison purposes, that is equivalent to an IPv6 `::` address (all sin6_addr bits are zero). ### Expand ipv6 ACL parameter scope to match all IPv6 addresses The "ipv6" parameter was not matching many IPv6 addresses, including some frequently seen ones like `::1` (i.e. IPv6 localhost). Now it matches any IP address that Squid code itself considers an IPv6 address, reducing surprises among admins that need to treat all IPv6 addresses specially. --- diff --git a/src/acl/Ip.cc b/src/acl/Ip.cc index 6d2d202db7..657b817f35 100644 --- a/src/acl/Ip.cc +++ b/src/acl/Ip.cc @@ -221,103 +221,6 @@ acl_ip_data::FactoryParse(const char *t) debugs(28, 5, "aclIpParseIpData: " << t); - /* Special ACL RHS "all" matches entire Internet */ - if (strcmp(t, "all") == 0) { - debugs(28, 9, "aclIpParseIpData: magic 'all' found."); - q->addr1.setAnyAddr(); - q->addr2.setEmpty(); - q->mask.setAnyAddr(); - return q; - } - - /* Detect some old broken strings equivalent to 'all'. - * treat them nicely. But be loud until its fixed. */ - if (strcmp(t, "0/0") == 0 || strcmp(t, "0.0.0.0/0") == 0 || strcmp(t, "0.0.0.0/0.0.0.0") == 0 || - strcmp(t, "0.0.0.0-255.255.255.255") == 0 || strcmp(t, "0.0.0.0-0.0.0.0/0") == 0) { - - debugs(28,DBG_CRITICAL, "ERROR: '" << t << "' needs to be replaced by the term 'all'."); - debugs(28,DBG_CRITICAL, "SECURITY NOTICE: Overriding config setting. Using 'all' instead."); - q->addr1.setAnyAddr(); - q->addr2.setEmpty(); - q->mask.setAnyAddr(); - return q; - } - - /* Special ACL RHS "ipv4" matches IPv4 Internet - * A nod to IANA; we include the entire class space in case - * they manage to find a way to recover and use it */ - if (strcmp(t, "ipv4") == 0) { - q->mask.setNoAddr(); - q->mask.applyMask(0, AF_INET); - return q; - } - - /* Special ACL RHS "ipv6" matches IPv6-Unicast Internet */ - if (strcmp(t, "ipv6") == 0) { - debugs(28, 9, "aclIpParseIpData: magic 'ipv6' found."); - r = q; // save head of the list for result. - - /* 0000::/4 is a mix of localhost and obsolete IPv4-mapping space. Not valid outside this host. */ - - /* Future global unicast space: 1000::/4 */ - q->addr1 = "1000::"; - q->mask.setNoAddr(); - q->mask.applyMask(4, AF_INET6); - - /* Current global unicast space: 2000::/4 = (2000::/4 - 3000::/4) */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "2000::"; - q->mask.setNoAddr(); - q->mask.applyMask(3, AF_INET6); - - /* Future global unicast space: 4000::/2 = (4000::/4 - 7000::/4) */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "4000::"; - q->mask.setNoAddr(); - q->mask.applyMask(2, AF_INET6); - - /* Future global unicast space: 8000::/2 = (8000::/4 - B000::/4) */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "8000::"; - q->mask.setNoAddr(); - q->mask.applyMask(2, AF_INET6); - - /* Future global unicast space: C000::/3 = (C000::/4 - D000::/4) */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "C000::"; - q->mask.setNoAddr(); - q->mask.applyMask(3, AF_INET6); - - /* Future global unicast space: E000::/4 */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "E000::"; - q->mask.setNoAddr(); - q->mask.applyMask(4, AF_INET6); - - /* F000::/4 is mostly reserved non-unicast. With some exceptions ... */ - - /* RFC 4193 Unique-Local unicast space: FC00::/7 */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "FC00::"; - q->mask.setNoAddr(); - q->mask.applyMask(7, AF_INET6); - - /* Link-Local unicast space: FE80::/10 */ - q->next = new acl_ip_data; - q = q->next; - q->addr1 = "FE80::"; - q->mask.setNoAddr(); - q->mask.applyMask(10, AF_INET6); - - return r; - } - // IPv4 if (sscanf(t, SCAN_ACL1_4, addr1, addr2, mask) == 3) { debugs(28, 9, "aclIpParseIpData: '" << t << "' matched: SCAN1-v4: " << SCAN_ACL1_4); @@ -471,6 +374,54 @@ acl_ip_data::FactoryParse(const char *t) return q; } +/// handles special ACL data parameters that apply to the whole ACLIP object +/// \returns true if input token is such a special parameter +bool +ACLIP::parseGlobal(const char * const token) +{ + // "all" matches entire Internet + if (strcmp(token, "all") == 0) { + debugs(28, 8, "found " << token); + matchAnyIpv4 = true; + matchAnyIpv6 = true; + // TODO: Ignore all other ACL data parameters, with a once/ACL warning. + return true; + } + + // "ipv4" matches IPv4 Internet + if (strcmp(token, "ipv4") == 0) { + debugs(28, 8, "found " << token); + matchAnyIpv4 = true; + // TODO: Ignore all IPv4 data parameters, with a once/ACL warning. + return true; + } + + // "ipv4" matches IPv6 Internet + if (strcmp(token, "ipv6") == 0) { + debugs(28, 8, "found " << token); + matchAnyIpv6 = true; + // TODO: Ignore all IPv6 data parameters, with a once/ACL warning. + return true; + } + + /* Detect some old broken strings equivalent to 'all'. + * treat them nicely. But be loud until its fixed. */ + if (strcmp(token, "0/0") == 0 || + strcmp(token, "0.0.0.0/0") == 0 || + strcmp(token, "0.0.0.0/0.0.0.0") == 0 || + strcmp(token, "0.0.0.0-255.255.255.255") == 0 || + strcmp(token, "0.0.0.0-0.0.0.0/0") == 0) { + + debugs(28,DBG_CRITICAL, "ERROR: '" << token << "' needs to be replaced by the term 'all'."); + debugs(28,DBG_CRITICAL, "SECURITY NOTICE: Overriding config setting. Using 'all' instead."); + matchAnyIpv4 = true; + matchAnyIpv6 = true; + return true; + } + + return false; +} + void ACLIP::parse() { @@ -478,6 +429,9 @@ ACLIP::parse() data = new IPSplay(); while (char *t = ConfigParser::strtokFile()) { + if (parseGlobal(t)) + continue; + acl_ip_data *q = acl_ip_data::FactoryParse(t); while (q != nullptr) { @@ -510,6 +464,14 @@ SBufList ACLIP::dump() const { IpAclDumpVisitor visitor; + + if (matchAnyIpv4 && matchAnyIpv6) + visitor.contents.push_back(SBuf("all")); + else if (matchAnyIpv4) + visitor.contents.push_back(SBuf("ipv4")); + else if (matchAnyIpv6) + visitor.contents.push_back(SBuf("ipv6")); + data->visit(visitor); return visitor.contents; } @@ -517,12 +479,30 @@ ACLIP::dump() const bool ACLIP::empty() const { - return data->empty(); + return data->empty() && !matchAnyIpv4 && !matchAnyIpv6; } int ACLIP::match(const Ip::Address &clientip) { + if (matchAnyIpv4) { + if (matchAnyIpv6) { + debugs(28, 3, clientip << " found, matched 'all'"); + return true; + } + if (clientip.isIPv4()) { + debugs(28, 3, clientip << " found, matched 'ipv4'"); + return true; + } + // fall through to look for an IPv6 match among IP parameters + } else if (matchAnyIpv6) { + if (clientip.isIPv6()) { + debugs(28, 3, clientip << " found, matched 'ipv6'"); + return true; + } + // fall through to look for an IPv4 match among IP parameters + } + static acl_ip_data ClientAddress; /* * aclIpAddrNetworkCompare() takes two acl_ip_data pointers as diff --git a/src/acl/Ip.h b/src/acl/Ip.h index 86754f46c3..10dd7ce36f 100644 --- a/src/acl/Ip.h +++ b/src/acl/Ip.h @@ -64,6 +64,14 @@ protected: int match(const Ip::Address &); IPSplay *data; +private: + bool parseGlobal(const char *); + + /// whether match() should return 1 for any IPv4 parameter + bool matchAnyIpv4 = false; + + /// whether match() should return 1 for any IPv6 parameter + bool matchAnyIpv6 = false; }; #endif /* SQUID_ACLIP_H */