From 0cd14eafa5b05f0cefb00152f48d33946e7681fb Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Sun, 5 Oct 2025 18:44:01 +0000 Subject: [PATCH] Do not forward HTCP CLR requests denied by htcp_clr_access (#2246) Before this change: * htcp_access controls TST access * htcp_clr_access controls CLR access to local cache purging * nothing controls CLR forwarding -- it is always allowed After this change: * htcp_access controls TST access * htcp_clr_access controls CLR access to local cache purging * htcp_clr_access controls CLR forwarding This bug was discovered and detailed by Joshua Rogers https://joshua.hu/ --- doc/release-notes/release-8.sgml.in | 6 ++++++ src/cf.data.pre | 22 +++++++++++++--------- src/htcp.cc | 21 +++++++-------------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/doc/release-notes/release-8.sgml.in b/doc/release-notes/release-8.sgml.in index a521cf4f8d..8ddb88821e 100644 --- a/doc/release-notes/release-8.sgml.in +++ b/doc/release-notes/release-8.sgml.in @@ -114,6 +114,12 @@ This section gives an account of those changes in three categories: connection should increase the configured limit by one to preserve previous behavior. + htcp_clr_access + +

HTCP CLR requests denied by this directive are no longer forwarded to + cache_peers that are configured to receive forwarded CLR requests. Only + HTCP CLR requests allowed by this directive are forwarded to those + cache_peers. diff --git a/src/cf.data.pre b/src/cf.data.pre index 5a6f7b90f3..76570527c2 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -2037,22 +2037,24 @@ LOC: Config.accessList.htcp DEFAULT: none DEFAULT_DOC: Deny, unless rules exist in squid.conf. DOC_START - Allowing or Denying access to the HTCP port based on defined - access lists + Controls whether HTCP TST requests received on htcp_port are allowed. htcp_access allow|deny [!]aclname ... - See also htcp_clr_access for details on access control for - cache purge (CLR) HTCP messages. + This directive does not control whether HTCP CLR requests are allowed. + Use htcp_clr_access directive for that. + + This directive does not control whether HTCP requests with other opcodes + are allowed (e.g., NOP, MON, and SET). Squid ignores those HTCP requests. NOTE: The default if no htcp_access lines are present is to - deny all traffic. This default may cause problems with peers + deny all HTCP TST traffic. This default may cause problems with peers using the htcp option. This clause only supports fast acl types. See https://wiki.squid-cache.org/SquidFaq/SquidAcl for details. -# Allow HTCP queries from local networks only +# Allow HTCP TST queries from local networks only #htcp_access allow localnet #htcp_access deny all DOC_END @@ -2064,12 +2066,14 @@ LOC: Config.accessList.htcp_clr DEFAULT: none DEFAULT_DOC: Deny, unless rules exist in squid.conf. DOC_START - Allowing or Denying access to purge content using HTCP based - on defined access lists. - See htcp_access for details on general HTCP access control. + Controls whether HTCP CLR requests received on htcp_port are allowed. + See htcp_access for controlling other HTCP messages. htcp_clr_access allow|deny [!]aclname ... + HTCP CLR requests purge matching cached entries. They may be forwarded to + specially marked cache_peers (see cache_peer HTCP options for details). + This clause only supports fast acl types. See https://wiki.squid-cache.org/SquidFaq/SquidAcl for details. diff --git a/src/htcp.cc b/src/htcp.cc index f3957899e5..da1e616b35 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -1236,7 +1236,7 @@ htcpSpecifier::checkedHit(StoreEntry *e) } static void -htcpHandleClr(htcpDataHeader * hdr, char *buf, int sz, Ip::Address &from) +htcpHandleClr(htcpDataHeader * const hdr, char * const buf, const int sz, Ip::Address &from) { /* buf[0/1] is reserved and reason */ if (sz < 2) { @@ -1246,10 +1246,9 @@ htcpHandleClr(htcpDataHeader * hdr, char *buf, int sz, Ip::Address &from) } int reason = static_cast(buf[1]) << 4; debugs(31, 2, "HTCP CLR reason: " << reason); - buf += 2; - sz -= 2; - /* buf should be a SPECIFIER */ + const auto specifierStart = buf + 2; + const auto specifierSize = sz - 2; if (sz == 0) { debugs(31, 4, "nothing to do"); @@ -1257,7 +1256,7 @@ htcpHandleClr(htcpDataHeader * hdr, char *buf, int sz, Ip::Address &from) return; } - htcpSpecifier::Pointer s(htcpUnpackSpecifier(buf, sz)); + const auto s = htcpUnpackSpecifier(specifierStart, specifierSize); if (!s) { debugs(31, 3, "htcpUnpackSpecifier failed"); @@ -1302,15 +1301,10 @@ htcpHandleClr(htcpDataHeader * hdr, char *buf, int sz, Ip::Address &from) default: break; } -} -/* - * Forward a CLR request to all peers who have requested that CLRs be - * forwarded to them. - */ -static void -htcpForwardClr(char *buf, int sz) -{ + // Forward this CLR request to all peers who have requested that CLRs be + // forwarded to them. + // TODO: Consider not forwarding requests with htcpClrStore() < 0. for (const auto &p: CurrentCachePeers()) { if (!p->options.htcp) { continue; @@ -1457,7 +1451,6 @@ htcpHandleMsg(char *buf, int sz, Ip::Address &from) break; case HTCP_CLR: htcpHandleClr(&hdr, hbuf, hsz, from); - htcpForwardClr(buf, sz); break; default: break; -- 2.47.3