Squid was dangerously mishandling duplicate[^1] values in ACLs with
Splay tree storage: src, dst, localip, http_status, dstdomain,
srcdomain, and ssl::server_name. These problems were all rooted in the
same old code but had diverged across two ACL groups, as detailed in the
corresponding sections below. Fortunately, all known problems were
accompanied with Squid cache.log WARNINGs: Squid configurations that do
not emit cited or similar warnings are not affected by these bugs.
[^1]: Squid Splay trees can only store unique values. Uniqueness is
defined in terms of a configurable comparison function that returns zero
when the two values are "not unique" (i.e. are considered to be
"duplicated" or "equal" in that Splay context). Those two "equal" values
may differ a lot in other contexts! For example, the following two
status code ranges are equal from acl_httpstatus_data::compare() point
of view, but are obviously very different from an admin and http_access
rules points of view: 200-200 and 200-299.
### Group 1: src, dst, localip, and http_status ACLs
These ACLs ignored (i.e. never matched) some configured ACL values in
problematic use cases. They also gave wrong "remove X" advice and
incorrectly classified values as being subranges or subsets:
Processing: acl testA http_status 200 200-300
WARNING: '200-300' is a subrange of '200'
WARNING: because of this '200-300' is ignored ...
Processing: acl testB http_status 300-400 200-300
WARNING: '200-300' is a subrange of '300-400'
WARNING: because of this '200-300' is ignored ...
WARNING: You should probably remove '300-400' from the ACL...
Processing: acl testC src 10.0.0.1 10.0.0.0-10.0.0.255
WARNING: (B) 10.0.0.1 is a subnetwork of (A) 10.0.0.0-10.0.0.255
WARNING: because of this 10.0.0.0-10.0.0.255 is ignored...
Processing: acl testD src 10.0.0.0-10.0.0.1 10.0.0.1-10.0.0.255
WARNING: (A) 10.0.0.1-10.0.0.255 is a subnetwork of (B) 10.0.0.0-...
WARNING: because of this 10.0.0.1-10.0.0.255 is ignored...
WARNING: You should probably remove 10.0.0.1-10.0.0.255 from the ACL
Since 2002 commit
96da6e8, IP-based ACLs like src, dst, and localip
eliminate duplicates[^1] among configured ACL values. That elimination
code was buggy since inception. Those bugs were later duplicated in
http_status code (2005 commit
a0ec9f6). This change fixes those bugs.
To correctly eliminate duplicates when facing two (fully or partially)
overlapping ranges -- new A and old B -- we must pick the right
corrective action depending on the kind of overlap:
* A is a subrange of B: Ignore fully duplicated new range A. Keep B.
* B is a subrange of A: Remove fully duplicated old range B. Add A.
* A partially overlaps with B: Add a union of A and B. Remove B.
Both acl_httpstatus_data::compare() and acl_ip_data::NetworkCompare()
mishandled the last two cases because these functions effectively
implemented the following buggy logic instead:
- in all three cases: Ignore new range A. Keep B.
Their WARNINGs also suggested wrong corrective actions in two mishandled
cases (see the last WARNING lines in testB and testD output above).
### Group 2: dstdomain, srcdomain, and ssl::server_name ACLs
Processing: acl testE dstdomain .example.com example.com
ERROR: '.example.com' is a subdomain of 'example.com'
ERROR: You need to remove '.example.com' from the ACL named 'testE'
2002 commit
96da6e8 bugs mentioned in Group 1 section stopped affecting
domain-based ACLs (i.e. dstdomain, srcdomain, and ssl::server_name) in
2011 commit
14e563c that adjusted aclDomainCompare() to self_destruct()
in problematic cases. However, that adjustment emitted wrong advice and
incorrect subdomain classification in cases where strlen() checks do not
work for determining which of the two configured ACL values should be
removed (see testE ERRORs above).
This change improves that handling by replacing the call to
self_destruct() with proper duplicate[^1] resolution code (and fixing
cache.log messages). We (now) support duplicate values instead of
rejecting configurations containing them because duplicate values do not
invalidate an ACL -- an ACL with duplicates could match as expected. It
may be difficult for some admins to avoid duplication, especially when
ACL values come from multiple sources. Squid should continue to warn
about duplicates (because they waste resources and may indicate a deeper
problem), but killing Squid or otherwise rejecting ACLs with duplicates
is bad UX.
N.B. Domain-based ACLs use sets of values rather than "ranges" discussed
in Group 1 section, but domain sets follow the same basic principles.
### All of the above seven ACLs
The problems in both ACL groups were fixed by factoring out "insert ACL
values while correctly handling duplicates" algorithm (see the three
bullets in Group 1 section) into Acl::SplayInserter::Merge() function.
Without duplicates, the new ACL value insertion code has the same cost
as the old one. The vast majority of duplicate cases incur a constant
additional overhead because Splay tree dynamic reorganization makes the
right tree nodes immediately available. A few cases duplicate double the
number of comparisons during tree searches (when Splay reorganization
cannot cope with a particularly "bad" ACL value order), but since these
"bad" cases ought to be very rare, and since all problematic cases are
accompanied by WARNINGs, this extra cost is deemed acceptable.
This change also fixes memory leaks associated with ignored ACL values.
Also added test-suite/test-squid-conf.sh support for matching multiple
stderr messages. Without this, testing a variety of closely-related
cases requires creating lots of test configuration files (two per test
case), increasing noise and, more importantly, making it difficult to
handle related test cases as one coherent collection. The new
"expect-messages" feature is barely sufficient for testing these
changes, but we are now at (or perhaps even beyond) the limit of what
can be reasonably done using shell scripts and test instruction files:
The next step is to convert instruction files (and likely some test
cases themselves!) to scripts written in Perl or a better language.
template <class FindValue> Value const *find (FindValue const &, int( * compare)(FindValue const &a, Value const &b)) const;
- void insert(Value const &, SPLAYCMP *compare);
+ /// If the given value matches a stored one, returns that matching value.
+ /// Otherwise, stores the given unique value and returns nil.
+ const Value *insert(const Value &, SPLAYCMP *);
void remove(Value const &, SPLAYCMP *compare);
}
template <class V>
-void
+typename Splay<V>::Value const *
Splay<V>::insert(Value const &value, SPLAYCMP *compare)
{
- if (find(value, compare) != nullptr) // ignore duplicates
- return;
+ if (const auto similarValue = find(value, compare))
+ return similarValue; // do not insert duplicates
if (head == nullptr)
head = new SplayNode<V>(value);
else
head = head->insert(value, compare);
++elements;
+
+ return nullptr; // no duplicates found
}
template <class V>
/*
* Here we set AclMatchedName in case we need to use it in a
- * warning message in aclDomainCompare().
+ * warning message in Acl::SplayInserter::Merge().
*/
AclMatchedName = A->name; /* ugly */
#include "squid.h"
#include "acl/Checklist.h"
#include "acl/DomainData.h"
+#include "acl/SplayInserter.h"
#include "anyp/Uri.h"
#include "cache_cf.h"
#include "ConfigParser.h"
return matchDomainName(h, d);
}
-/* compare two domains */
-
-template<class T>
-int
-aclDomainCompare(T const &a, T const &b)
-{
- char * const d1 = static_cast<char *>(b);
- char * const d2 = static_cast<char *>(a);
- int ret;
- ret = aclHostDomainCompare(d1, d2);
-
- if (ret != 0) {
- char *const d3 = d2;
- char *const d4 = d1;
- ret = aclHostDomainCompare(d3, d4);
- if (ret == 0) {
- // When a.example.com comes after .example.com in an ACL
- // sub-domain is ignored. That is okay. Just important
- bool d3big = (strlen(d3) > strlen(d4)); // Always suggest removing the longer one.
- debugs(28, DBG_IMPORTANT, "WARNING: '" << (d3big?d3:d4) << "' is a subdomain of '" << (d3big?d4:d3) << "'");
- debugs(28, DBG_IMPORTANT, "WARNING: You should remove '" << (d3big?d3:d4) << "' from the ACL named '" << AclMatchedName << "'");
- debugs(28, 2, "Ignore '" << d3 << "' to keep splay tree searching predictable");
- }
- } else if (ret == 0) {
- // It may be an exact duplicate. No problem. Just drop.
- if (strcmp(d1,d2)==0) {
- debugs(28, 2, "WARNING: '" << d2 << "' is duplicated in the list.");
- debugs(28, 2, "WARNING: You should remove one '" << d2 << "' from the ACL named '" << AclMatchedName << "'");
- return ret;
- }
- // When a.example.com comes before .example.com in an ACL
- // discarding the wildcard is critically bad.
- // or Maybe even both are wildcards. Things are very weird in those cases.
- bool d1big = (strlen(d1) > strlen(d2)); // Always suggest removing the longer one.
- debugs(28, DBG_CRITICAL, "ERROR: '" << (d1big?d1:d2) << "' is a subdomain of '" << (d1big?d2:d1) << "'");
- debugs(28, DBG_CRITICAL, "ERROR: You need to remove '" << (d1big?d1:d2) << "' from the ACL named '" << AclMatchedName << "'");
- self_destruct();
- }
-
- return ret;
-}
-
bool
ACLDomainData::match(char const *host)
{
return visitor.contents;
}
+template <>
+int
+Acl::SplayInserter<char*>::Compare(const Value &a, const Value &b)
+{
+ // If X represents a set of matching domain names (e.g., .example.com), then
+ // matchDomainName(X, Y) uses a single domain name from X by removing the
+ // leading dot (e.g., example.com). We call that name "the root of X". If X
+ // is a single domain name, then its root is X itself. Since domain sets
+ // cannot have _partial_ overlaps (unlike IP or integer ranges), testing
+ // roots is enough to detect duplicates and establish correct set order.
+
+ if (matchDomainName(b, a)) {
+ // Set A does not contain B's root. If set B contains A's root, then the
+ // call below will return 0, signaling duplicates. Otherwise, A and B
+ // have no common values, and the call below will correctly order the
+ // two sets, mimicking the order used by the Splay comparison function
+ // in match().
+ return matchDomainName(a, b);
+ } else {
+ // Signal duplicates because set A contains B's root (at least).
+ return 0;
+ }
+}
+
+template <>
+bool
+Acl::SplayInserter<char*>::IsSubset(const Value &a, const Value &b)
+{
+ // A value that starts with a dot matches a set of the corresponding domain
+ // names. Other values are individual domain names that match themselves.
+ // \sa matchDomainName()
+
+ if (*a == '.' && *b == '.') {
+ // A and B are overlapping sets. More characters imply a smaller set.
+ return strlen(a) >= strlen(b);
+ }
+
+ if (*a != '.' && *b != '.') {
+ // A and B are identical individual domain names
+ return true;
+ }
+
+ // Either A or B is a set. The other one is a domain name inside that set.
+ // That domain name may use fewer or more characters (e.g., both example.com
+ // and x.example.com domains belong to the same .example.com set), so we
+ // cannot use a strlen()-based test here.
+ return *b == '.';
+}
+
+template <>
+Acl::SplayInserter<char*>::Value
+Acl::SplayInserter<char*>::MakeCombinedValue(const Value &, const Value &)
+{
+ Assure(!"domain name sets cannot partially overlap");
+ return nullptr; // unreachable code
+}
+
+template <>
+void
+Acl::SplayInserter<char*>::DestroyValue(Value v)
+{
+ xfree(v);
+}
+
void
ACLDomainData::parse()
{
while (char *t = ConfigParser::strtokFile()) {
Tolower(t);
- domains->insert(xstrdup(t), aclDomainCompare);
+ Acl::SplayInserter<char*>::Merge(*domains, xstrdup(t));
}
}
#include "squid.h"
#include "acl/FilledChecklist.h"
#include "acl/HttpStatus.h"
+#include "acl/SplayInserter.h"
#include "debug/Stream.h"
#include "HttpReply.h"
+#include <algorithm>
#include <climits>
static void aclParseHTTPStatusList(Splay<acl_httpstatus_data *> **curlist);
return rv;
}
-int acl_httpstatus_data::compare(acl_httpstatus_data* const& a, acl_httpstatus_data* const& b)
+template <>
+int
+Acl::SplayInserter<acl_httpstatus_data*>::Compare(const Value &a, const Value &b)
{
- int ret;
- ret = aclHTTPStatusCompare(b, a);
-
- if (ret != 0)
- ret = aclHTTPStatusCompare(a, b);
-
- if (ret == 0) {
- const SBuf sa = a->toStr();
- const SBuf sb = b->toStr();
- debugs(28, DBG_CRITICAL, "WARNING: '" << sa << "' is a subrange of '" << sb << "'");
- debugs(28, DBG_CRITICAL, "WARNING: because of this '" << sa << "' is ignored to keep splay tree searching predictable");
- debugs(28, DBG_CRITICAL, "WARNING: You should probably remove '" << sb << "' from the ACL named '" << AclMatchedName << "'");
- }
+ return aclHTTPStatusCompare(a, b);
+}
- return ret;
+template <>
+bool
+Acl::SplayInserter<acl_httpstatus_data*>::IsSubset(const Value &a, const Value &b)
+{
+ return b->status1 <= a->status1 && a->status2 <= b->status2;
+}
+
+template <>
+Acl::SplayInserter<acl_httpstatus_data*>::Value
+Acl::SplayInserter<acl_httpstatus_data*>::MakeCombinedValue(const Value &a, const Value &b)
+{
+ const auto minLeft = std::min(a->status1, b->status1);
+ const auto maxRight = std::max(a->status2, b->status2);
+ return new acl_httpstatus_data(minLeft, maxRight);
+}
+
+/// reports acl_httpstatus_data using squid.conf http_status ACL value format
+static std::ostream &
+operator <<(std::ostream &os, const acl_httpstatus_data *status)
+{
+ if (status)
+ os << status->toStr();
+ return os;
}
ACLHTTPStatus::ACLHTTPStatus (char const *theClass) : data(nullptr), class_ (theClass)
{
while (char *t = ConfigParser::strtokFile()) {
if (acl_httpstatus_data *q = aclParseHTTPStatusData(t))
- (*curlist)->insert(q, acl_httpstatus_data::compare);
+ Acl::SplayInserter<acl_httpstatus_data*>::Merge(**curlist, std::move(q));
}
}
static int
aclHTTPStatusCompare(acl_httpstatus_data * const &a, acl_httpstatus_data * const &b)
{
- if (a->status1 < b->status1)
- return 1;
+ if (a->status2 < b->status1)
+ return 1; // the entire range a is to the left of range b
if (a->status1 > b->status2)
- return -1;
+ return -1; // the entire range a is to the right of range b
- return 0;
+ return 0; // equal or partially overlapping ranges
}
struct HttpStatusAclDumpVisitor {
acl_httpstatus_data(int);
acl_httpstatus_data(int, int);
SBuf toStr() const; // was toStr
-
- static int compare(acl_httpstatus_data* const& a, acl_httpstatus_data* const& b);
};
/// \ingroup ACLAPI
#include "squid.h"
#include "acl/Checklist.h"
#include "acl/Ip.h"
+#include "acl/SplayInserter.h"
#include "cache_cf.h"
#include "ConfigParser.h"
#include "debug/Stream.h"
#include "MemBuf.h"
#include "wordlist.h"
+#include <algorithm>
+
void *
ACLIP::operator new (size_t)
{
return SBuf(tmpbuf);
}
+Ip::Address
+acl_ip_data::firstAddress() const
+{
+ auto ip = addr1;
+ if (!mask.isNoAddr())
+ ip.applyMask(mask);
+ return ip;
+}
+
+Ip::Address
+acl_ip_data::lastAddress() const
+{
+ auto ip = addr2.isAnyAddr() ? addr1 : addr2;
+ if (!mask.isNoAddr())
+ ip.turnMaskedBitsOn(mask);
+ return ip;
+}
+
+template <>
+int
+Acl::SplayInserter<acl_ip_data*>::Compare(const Value &a, const Value &b)
+{
+ if (a->lastAddress() < b->firstAddress())
+ return -1; // the entire range a is to the left of range b
+
+ if (a->firstAddress() > b->lastAddress())
+ return +1; // the entire range a is to the right of range b
+
+ return 0; // equal or partially overlapping ranges
+}
+
+template <>
+bool
+Acl::SplayInserter<acl_ip_data*>::IsSubset(const Value &a, const Value &b)
+{
+ return b->firstAddress() <= a->firstAddress() && a->lastAddress() <= b->lastAddress();
+}
+
+template <>
+Acl::SplayInserter<acl_ip_data*>::Value
+Acl::SplayInserter<acl_ip_data*>::MakeCombinedValue(const Value &a, const Value &b)
+{
+ const auto minLeft = std::min(a->firstAddress(), b->firstAddress());
+ const auto maxRight = std::max(a->lastAddress(), b->lastAddress());
+ return new acl_ip_data(minLeft, maxRight, Ip::Address::NoAddr(), nullptr);
+}
+
+/// reports acl_ip_data using squid.conf ACL value format
+static std::ostream &
+operator <<(std::ostream &os, acl_ip_data *value)
+{
+ if (value)
+ os << value->toSBuf();
+ return os;
+}
+
/*
* aclIpAddrNetworkCompare - The guts of the comparison for IP ACLs
* matching checks. The first argument (p) is a "host" address,
}
}
-/*
- * acl_ip_data::NetworkCompare - Compare two acl_ip_data entries. Strictly
- * used by the splay insertion routine. It emits a warning if it
- * detects a "collision" or overlap that would confuse the splay
- * sorting algorithm. Much like aclDomainCompare.
- * The first argument (p) is a "host" address, i.e. the IP address of a cache client.
- * The second argument (b) is a "network" address that might have a subnet and/or range.
- * We mask the host address bits with the network subnet mask.
- */
-int
-acl_ip_data::NetworkCompare(acl_ip_data * const & a, acl_ip_data * const &b)
-{
- int ret;
- bool bina = true;
- ret = aclIpAddrNetworkCompare(b, a);
-
- if (ret != 0) {
- bina = false;
- ret = aclIpAddrNetworkCompare(a, b);
- }
-
- if (ret == 0) {
- char buf_n1[3*(MAX_IPSTRLEN+1)];
- char buf_n2[3*(MAX_IPSTRLEN+1)];
- if (bina) {
- b->toStr(buf_n1, 3*(MAX_IPSTRLEN+1));
- a->toStr(buf_n2, 3*(MAX_IPSTRLEN+1));
- } else {
- a->toStr(buf_n1, 3*(MAX_IPSTRLEN+1));
- b->toStr(buf_n2, 3*(MAX_IPSTRLEN+1));
- }
- debugs(28, DBG_CRITICAL, "WARNING: (" << (bina?'B':'A') << ") '" << buf_n1 << "' is a subnetwork of (" << (bina?'A':'B') << ") '" << buf_n2 << "'");
- debugs(28, DBG_CRITICAL, "WARNING: because of this '" << (bina?buf_n2:buf_n1) << "' is ignored to keep splay tree searching predictable");
- debugs(28, DBG_CRITICAL, "WARNING: You should probably remove '" << buf_n1 << "' from the ACL named '" << AclMatchedName << "'");
- }
-
- return ret;
-}
-
/**
* Decode an ascii representation (asc) of a IP netmask address or CIDR,
* and place resulting information in mask.
if (changed)
debugs(28, DBG_CRITICAL, "WARNING: aclIpParseIpData: Netmask masks away part of the specified IP in '" << t << "'");
+ // TODO: Either switch match() to Acl::SplayInserter<acl_ip_data*>::Compare()
+ // range logic (that does not have these problems) OR warn that some (or
+ // even all) addresses will never match this configured ACL value when
+ // `q->addr1.applyMask()` above is positive:
+ //
+ // * A single configured IP value will never match:
+ // A.matchIPAddr(q->addr1) in aclIpAddrNetworkCompare() will not return 0.
+ // For example, `acl x src 127.0.0.1/24` does not match any address.
+ //
+ // * A configured IP range will not match any q->addr1/mask IPs:
+ // (A >= q->addr1) in aclIpAddrNetworkCompare() is false and
+ // A.matchIPAddr(q->addr1) will not return 0.
+ // For example, `acl y src 10.0.0.1-10.0.0.255/24` does not match 10.0.0.1.
+
debugs(28,9, "Parsed: " << q->addr1 << "-" << q->addr2 << "/" << q->mask << "(/" << q->mask.cidr() <<")");
/* 1.2.3.4/255.255.255.0 --> 1.2.3.0 */
/* pop each result off the list and add it to the data tree individually */
acl_ip_data *next_node = q->next;
q->next = nullptr;
- if (!data->find(q,acl_ip_data::NetworkCompare))
- data->insert(q, acl_ip_data::NetworkCompare);
+ Acl::SplayInserter<acl_ip_data*>::Merge(*data, std::move(q));
q = next_node;
}
}
public:
static acl_ip_data *FactoryParse(char const *);
- static int NetworkCompare(acl_ip_data * const & a, acl_ip_data * const &b);
acl_ip_data ();
void toStr(char *buf, int len) const;
SBuf toSBuf() const;
+ /// minimum (masked) address that matches this configured ACL value
+ Ip::Address firstAddress() const;
+
+ /// maximum (masked) address that matches this configured ACL value
+ Ip::Address lastAddress() const;
+
Ip::Address addr1;
Ip::Address addr2;
InnerNode.h \
Options.cc \
Options.h \
+ SplayInserter.h \
Tree.cc \
Tree.h \
forward.h
--- /dev/null
+/*
+ * Copyright (C) 1996-2023 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_SRC_ACL_SPLAYINSERTER_H
+#define SQUID_SRC_ACL_SPLAYINSERTER_H
+
+#include "acl/Acl.h"
+#include "debug/Stream.h"
+#include "globals.h"
+#include "splay.h"
+
+namespace Acl {
+
+/// Helps populate a Splay tree with configured ACL parameter values and their
+/// duplicate-handling derivatives (each represented by a DataValue object).
+template <class DataValue>
+class SplayInserter
+{
+public:
+ using Value = DataValue;
+
+ /// If necessary, updates the splay tree to match all individual values that
+ /// match the given parsed ACL parameter value. If the given value itself is
+ /// not added to the tree (e.g., because it is a duplicate), it is destroyed
+ /// using DestroyValue(). Otherwise, the given value will be destroyed
+ /// later, during subsequent calls to this method or free_acl().
+ static void Merge(Splay<Value> &, Value &&);
+
+private:
+ /// SplayInserter users are expected to specialize all or most of the static
+ /// methods below. Most of these methods have no generic implementation.
+
+ /// A Splay::SPLAYCMP function for comparing parsed ACL parameter values.
+ /// This function must work correctly with all valid ACL parameter values,
+ /// including those representing sets or ranges. The order specified by this
+ /// function must be the same as the order specified by the SPLAYCMP
+ /// function used later by ACL::match().
+ /// \retval -1 when a < b (this function defines what "less" means in Merge() context)
+ /// \retval +1 when b < a
+ /// \retval 0 all other cases (i.e. when a and b overlap)
+ /// Here, two values overlap if they are identical, if one contains all
+ /// values from another, or if one contains at least one value from another.
+ static int Compare(const Value &a, const Value &b);
+
+ /// whether the set of values matched by `a` contains the entire set of
+ /// values matched by `b`, including cases where `a` is identical to `b`
+ /// \prec The two values overlap: Compare(a, b) == 0
+ static bool IsSubset(const Value &a, const Value &b);
+
+ /// Creates a new Value that matches all individual values matched by `a`
+ /// and all individual values matched by `b` but no other values.
+ /// \prec The two values overlap: Compare(a, b) == 0
+ static Value MakeCombinedValue(const Value &a, const Value &b);
+
+ /// A Splay::SPLAYFREE-like function that destroys parsed ACL parameter values.
+ static void DestroyValue(Value v) { delete v; }
+};
+
+} // namespace Acl
+
+template <class DataValue>
+void
+Acl::SplayInserter<DataValue>::Merge(Splay<Value> &storage, Value &&newItem)
+{
+ const auto comparator = &SplayInserter<Value>::Compare;
+ while (const auto oldItemPointer = storage.insert(newItem, comparator)) {
+ const auto oldItem = *oldItemPointer;
+
+ if (IsSubset(newItem, oldItem)) {
+ debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Ignoring " << newItem << " because it is already covered by " << oldItem <<
+ Debug::Extra << "advice: Remove value " << newItem << " from the ACL named " << AclMatchedName);
+ DestroyValue(newItem);
+ return;
+ }
+
+ if (IsSubset(oldItem, newItem)) {
+ debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Ignoring earlier " << oldItem << " because it is covered by " << newItem <<
+ Debug::Extra << "advice: Remove value " << oldItem << " from the ACL named " << AclMatchedName);
+ storage.remove(oldItem, comparator);
+ DestroyValue(oldItem);
+ continue; // still need to insert newItem (and it may conflict with other old items)
+ }
+
+ const auto combinedItem = MakeCombinedValue(oldItem, newItem);
+ debugs(28, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Merging overlapping " << newItem << " and " << oldItem << " into " << combinedItem <<
+ Debug::Extra << "advice: Replace values " << newItem << " and " << oldItem << " with " << combinedItem << " in the ACL named " << AclMatchedName);
+ DestroyValue(newItem);
+ newItem = combinedItem;
+ storage.remove(oldItem, comparator);
+ DestroyValue(oldItem);
+ continue; // still need to insert updated newItem (and it may conflict with other old items)
+ }
+}
+
+#endif /* SQUID_SRC_ACL_SPLAYINSERTER_H */
+
return changes;
}
+void
+Ip::Address::turnMaskedBitsOn(const Address &mask)
+{
+ const auto addressWords = reinterpret_cast<uint32_t*>(&mSocketAddr_.sin6_addr);
+ const auto maskWords = reinterpret_cast<const uint32_t*>(&mask.mSocketAddr_.sin6_addr);
+ const auto len = sizeof(mSocketAddr_.sin6_addr)/sizeof(uint32_t);
+ for (size_t i = 0; i < len; ++i)
+ addressWords[i] |= ~maskWords[i];
+}
+
void
Ip::Address::applyClientMask(const Address &mask)
{
/// IPv6 clients use 'privacy addressing' instead.
void applyClientMask(const Address &mask);
+ /// Set bits of the stored IP address on if they are on in the given mask.
+ /// For example, supplying a /24 mask turns 127.0.0.1 into 127.0.0.255.
+ /// \sa applyMask(const Address &)
+ void turnMaskedBitsOn(const Address &mask);
+
/** Return the ASCII equivalent of the address
* Semantically equivalent to the IPv4 inet_ntoa()
* eg. 127.0.0.1 (IPv4) or ::1 (IPv6)
done; \
if test "$$failed" -eq 0; then cp $(TRUE) $@ ; else exit 1; fi
-CLEANFILES += squid-conf-tests squid-stderr.log
+CLEANFILES += \
+ squid-conf-tests \
+ squid-expected-messages \
+ squid-stderr.log \
+ squid-stderr.log.next \
+ squid-stderr.log.unmatched
--- /dev/null
+## Copyright (C) 1996-2023 The Squid Software Foundation and contributors
+##
+## Squid software is distributed under GPLv2+ license and includes
+## contributions from numerous individuals and organizations.
+## Please see the COPYING and CONTRIBUTORS files for details.
+##
+
+acl test11 dstdomain .example.com example.com
+acl test12 dstdomain example.com .example.com
+
+acl test21 dstdomain .example.com a.example.com
+acl test22 dstdomain b.example.com .example.com
+
+acl test31 dstdomain .example.com .example.com c.example.com
+
+acl test41 dstdomain .d.example.com .example.com
+acl test42 dstdomain .example.com .e.example.com
+
+acl test51 dstdomain example.com example.net .
--- /dev/null
+expect-messages <<END
+WARNING: Ignoring example.com because it is already covered by .example.com
+ advice: Remove value example.com from the ACL named test11
+
+WARNING: Ignoring earlier example.com because it is covered by .example.com
+ advice: Remove value example.com from the ACL named test12
+
+WARNING: Ignoring a.example.com because it is already covered by .example.com
+ advice: Remove value a.example.com from the ACL named test21
+
+WARNING: Ignoring earlier b.example.com because it is covered by .example.com
+ advice: Remove value b.example.com from the ACL named test22
+
+WARNING: Ignoring .example.com because it is already covered by .example.com
+ advice: Remove value .example.com from the ACL named test31
+
+WARNING: Ignoring c.example.com because it is already covered by .example.com
+ advice: Remove value c.example.com from the ACL named test31
+
+WARNING: Ignoring earlier .d.example.com because it is covered by .example.com
+ advice: Remove value .d.example.com from the ACL named test41
+
+WARNING: Ignoring .e.example.com because it is already covered by .example.com
+ advice: Remove value .e.example.com from the ACL named test42
+END
--- /dev/null
+## Copyright (C) 1996-2023 The Squid Software Foundation and contributors
+##
+## Squid software is distributed under GPLv2+ license and includes
+## contributions from numerous individuals and organizations.
+## Please see the COPYING and CONTRIBUTORS files for details.
+##
+
+acl test11 http_status 400 400
+acl test12 http_status 407 400 407
+acl test13 http_status 200 200-300
+
+acl test14 http_status 300-399
+acl test14 http_status 304
+
+acl test21 http_status 100-300 350 200-400
+acl test22 http_status 150 200-400 100-300
+
+acl test30 http_status 300 400 300-400
--- /dev/null
+expect-messages <<END
+WARNING: Ignoring 400 because it is already covered by 400
+ advice: Remove value 400 from the ACL named test11
+
+WARNING: Ignoring 407 because it is already covered by 407
+ advice: Remove value 407 from the ACL named test12
+
+WARNING: Ignoring earlier 200 because it is covered by 200-300
+ advice: Remove value 200 from the ACL named test13
+
+WARNING: Ignoring 304 because it is already covered by 300-399
+ advice: Remove value 304 from the ACL named test14
+
+WARNING: Ignoring earlier 350 because it is covered by 200-400
+ advice: Remove value 350 from the ACL named test21
+WARNING: Merging overlapping 200-400 and 100-300 into 100-400
+ advice: Replace values 200-400 and 100-300 with 100-400 in the ACL named test21
+
+WARNING: Merging overlapping 100-300 and 200-400 into 100-400
+ advice: Replace values 100-300 and 200-400 with 100-400 in the ACL named test22
+WARNING: Ignoring earlier 150 because it is covered by 100-400
+ advice: Remove value 150 from the ACL named test22
+
+WARNING: Ignoring earlier 400 because it is covered by 300-400
+ advice: Remove value 400 from the ACL named test30
+WARNING: Ignoring earlier 300 because it is covered by 300-400
+ advice: Remove value 300 from the ACL named test30
+END
--- /dev/null
+## Copyright (C) 1996-2023 The Squid Software Foundation and contributors
+##
+## Squid software is distributed under GPLv2+ license and includes
+## contributions from numerous individuals and organizations.
+## Please see the COPYING and CONTRIBUTORS files for details.
+##
+
+acl test11 src 127.0.0.1 127.0.0.0-127.0.0.255
+acl test12 src 192.168.1.0/24 192.168.0.0/16
+
+acl test13 src 127.0.0.0-127.0.0.255 127.0.0.1
+acl test14 src 127.0.0.0-127.0.0.128 127.0.0.128-127.0.0.255
+
+acl test15 src 10.0.0.1-10.0.0.128 10.0.0.0-10.0.0.1 10.0.0.128-10.0.0.255
+
+acl test25 dst 127.0.0.0-127.0.0.128/32 127.0.0.128-127.1.0.255
+
+acl test36 dst 127.0.0.1-127.0.0.128 127.0.0.0-127.1.0.0/16
+acl test37 dst 127.0.0.0-127.1.0.0/16 127.0.0.1-127.0.0.128
+
+# TODO: make configurable depending on USE_IPV6
+# acl test41 src bad::1 bad::0-bad::f
+# acl test42 src dead::0-dead::0 dead::0
+# acl test43 src bad::0/64 bad::0/128
+# acl test44 src beef::0/16 beef:bad::/64
--- /dev/null
+expect-messages <<END
+WARNING: Ignoring earlier 127.0.0.1 because it is covered by 127.0.0.0-127.0.0.255
+ advice: Remove value 127.0.0.1 from the ACL named test11
+
+WARNING: Ignoring earlier 192.168.1.0/24 because it is covered by 192.168.0.0/16
+ advice: Remove value 192.168.1.0/24 from the ACL named test12
+
+WARNING: Ignoring 127.0.0.1 because it is already covered by 127.0.0.0-127.0.0.255
+ advice: Remove value 127.0.0.1 from the ACL named test13
+
+WARNING: Merging overlapping 127.0.0.128-127.0.0.255 and 127.0.0.0-127.0.0.128 into 127.0.0.0-127.0.0.255
+ advice: Replace values 127.0.0.128-127.0.0.255 and 127.0.0.0-127.0.0.128 with 127.0.0.0-127.0.0.255 in the ACL named test14
+
+WARNING: Merging overlapping 10.0.0.0-10.0.0.1 and 10.0.0.1-10.0.0.128 into 10.0.0.0-10.0.0.128
+ advice: Replace values 10.0.0.0-10.0.0.1 and 10.0.0.1-10.0.0.128 with 10.0.0.0-10.0.0.128 in the ACL named test15
+WARNING: Merging overlapping 10.0.0.128-10.0.0.255 and 10.0.0.0-10.0.0.128 into 10.0.0.0-10.0.0.255
+ advice: Replace values 10.0.0.128-10.0.0.255 and 10.0.0.0-10.0.0.128 with 10.0.0.0-10.0.0.255 in the ACL named test15
+
+WARNING: Merging overlapping 127.0.0.128-127.1.0.255 and 127.0.0.0-127.0.0.128 into 127.0.0.0-127.1.0.255
+ advice: Replace values 127.0.0.128-127.1.0.255 and 127.0.0.0-127.0.0.128 with 127.0.0.0-127.1.0.255 in the ACL named test25
+
+WARNING: Ignoring earlier 127.0.0.1-127.0.0.128 because it is covered by 127.0.0.0-127.1.0.0/16
+ advice: Remove value 127.0.0.1-127.0.0.128 from the ACL named test36
+
+WARNING: Ignoring 127.0.0.1-127.0.0.128 because it is already covered by 127.0.0.0-127.1.0.0/16
+ advice: Remove value 127.0.0.1-127.0.0.128 from the ACL named test37
+END
+
+# TODO: skip-unless-autoconf-defines USE_IPV6 1
+# WARNING: Ignoring earlier bad::1 because it is covered by bad::-bad::f
+# advice: Remove value bad::1 from the ACL named test41
+#
+# WARNING: Ignoring dead:: because it is already covered by dead::-dead::
+# advice: Remove value dead:: from the ACL named test42
+#
+# WARNING: Ignoring bad:: because it is already covered by bad::/64
+# advice: Remove value bad:: from the ACL named test43
+#
+# WARNING: Ignoring beef:bad::/64 because it is already covered by beef::/16
+# advice: Remove value beef:bad::/64 from the ACL named test44
# If set, expect a matching stderr message
messageRegex=""
+# If set, expect stderr messages matching regexes in the named file.
+# See expectMessages() and matchEachRegex().
+messageRegexFilename=""
+
expectMessage()
{
local p1="$1"
fi
}
+# Starts expecting Squid stderr lines that match configured regular
+# expressions. Expressions are read from standard input, one extended regex
+# per line, until a line matching here-document terminator in $p1 is read.
+# Empty lines are ignored.
+expectMessages()
+{
+ local p1="$1"
+ local p2="$2"
+ local where="$3"
+
+ if test -n "$messageRegexFilename"
+ then
+ echo "$where: ERROR: Repeated message-setting instruction"
+ exit 1
+ fi
+
+ if test -z "$p1"
+ then
+ echo "$where: ERROR: Missing here-doc terminator"
+ exit 1
+ fi
+ local heredocTerminator="$p1"
+
+ if test -n "$p2"
+ then
+ echo "$where: ERROR: Bad here-doc: Unexpected input after '$terminator' terminator: $p2"
+ exit 1
+ fi
+
+ messageRegexFilename="squid-expected-messages"
+ if ! :> $messageRegexFilename
+ then
+ echo "$where: ERROR: Cannot create a temporary file named $messageRegexFilename"
+ exit 1
+ fi
+
+ local foundTerminator=0;
+ while read hereDocLine
+ do
+ lineNo=$(($lineNo+1))
+ where="$instructionsFile:$lineNo";
+
+ if test "<<$hereDocLine" = "$heredocTerminator"
+ then
+ foundTerminator=1
+ break;
+ fi
+
+ # skip empty lines; they cannot be used as regexes and they improve
+ # here-document formatting
+ if test -z "$hereDocLine"
+ then
+ continue;
+ fi
+
+ if ! printf '%s\n' "$hereDocLine" >> $messageRegexFilename
+ then
+ echo "$where: ERROR: Cannot write to a temporary file named $messageRegexFilename"
+ exit 1
+ fi
+ done
+
+ if test $foundTerminator != 1
+ then
+ echo "$where: ERROR: Input ended before here-doc terminator ($heredocTerminator)"
+ exit 1
+ fi
+}
+
+# Checks that each of the extended regexes (in the given file) matches line(s)
+# in the given Squid log file. A log line can match at most once.
+matchEachRegex()
+{
+ local regexFilename="$1"
+ local errLog="$2"
+
+ local errorLogRemaining="$errorLog.unmatched";
+ local errorLogNext="$errorLog.next";
+
+ if ! cp $errorLog $errorLogRemaining
+ then
+ echo "ERROR: Cannot create a temporary file named $errorLogRemaining"
+ exit 1
+ fi
+
+ local result=0
+ while read regex
+ do
+ if grep -q -E "$regex" $errorLogRemaining
+ then
+ # No good way to distinguish a possible lack of "grep -v" matches
+ # from grep errors because both result in non-zero grep exit code.
+ # For now, assume that error log always has some "extra" lines,
+ # guaranteeing at least one "grep -v non-empty-regex" match.
+ if ! grep -v -E "$regex" $errorLogRemaining > $errorLogNext || ! mv $errorLogNext $errorLogRemaining
+ then
+ echo "ERROR: Temporary file manipulation failure"
+ exit 1
+ fi
+ else
+ echo "ERROR: Squid did not emit an expected message to stderr"
+ echo " expected message regex: $regex"
+ result=1
+ fi
+ done < $regexFilename
+
+ if test $result != 0
+ then
+ echo "Unmatched Squid stderr lines (see $errorLogRemaining):"
+ cat $errorLogRemaining
+ fi
+
+ return $result
+}
+
instructionsFile="$configFile.instructions"
if test -e $instructionsFile
then
continue;
fi
+ if test "$instructionName" = "expect-messages"
+ then
+ expectMessages "$p1" "$p2" "$here"
+ continue;
+ fi
+
if test "$instructionName" = "skip-unless-autoconf-defines"
then
# Skip test unless the given macro is #defined in autoconf.h
updateOutcome 1
fi
+if test -n "$messageRegexFilename" && ! matchEachRegex $messageRegexFilename $errorLog
+then
+ # matchEachRegex reports errors
+ updateOutcome 1
+fi
+
if test $expectFailure = no
then
if test "$result" -ne 0
projects / thirdparty / squid.git / commitdiff
