Drop cache_object protocol support (#1250)

Removing this non-standard protocol (already mentioned as deprecated
in Squid sources) helps eliminate duplication and simplifies the
existing error-prone forwarding logic (causing CVEs).

Separate commits replace cache_object URLs sent by squidclient and
cachemgr.cgi tools with URLs using an http scheme.

diff --git a/doc/release-notes/release-7.sgml.in b/doc/release-notes/release-7.sgml.in
index faaf6e6249aac5b4dd6a759a8cef2077338e3c5b..d29658a56bd1b99ad7ce36cb2d2fce5c0d661bb1 100644 (file)
--- a/doc/release-notes/release-7.sgml.in
+++ b/doc/release-notes/release-7.sgml.in
@@ -30,6 +30,7 @@ The Squid-@SQUID_RELEASE@ change history can be <url url="https://github.com/squ
 
 <p>The most important of these new features are:
 <itemize>
+       <item>removed support for the cache_object URI scheme
        <item>Cache Manager changes
 </itemize>
 
@@ -40,6 +41,9 @@ The Squid-@SQUID_RELEASE@ change history can be <url url="https://github.com/squ
 
 <p>
 <descrip>
+       <tag>cache_object URI</tag>
+       <p>Cache manager is no longer accessible by URIs with cache_object scheme.
+
        <tag>non_peers</tag>
        <p>Removed the <em>mgr:non_peers</em> report. Squid still ignores
        unexpected ICP responses but no longer remembers the details that comprised

diff --git a/src/FwdState.cc b/src/FwdState.cc
index d92cacaff83dd40ae37579b7108719087fb93cc1..f8991db0cf0f4294fe0ae997a7969c9ddfcc09e1 100644 (file)
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -346,8 +346,7 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht
      * be allowed.  yuck, I know.
      */
 
-    if ( Config.accessList.miss && !request->client_addr.isNoAddr() &&
-            !request->flags.internal && request->url.getScheme() != AnyP::PROTO_CACHE_OBJECT) {
+    if ( Config.accessList.miss && !request->client_addr.isNoAddr() && !request->flags.internal) {
         /**
          * Check if this host is allowed to fetch MISSES from us (miss_access).
          * Intentionally replace the src_addr automatically selected by the checklist code
@@ -396,11 +395,6 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht
 
     switch (request->url.getScheme()) {
 
-    case AnyP::PROTO_CACHE_OBJECT:
-        debugs(17, 2, "calling CacheManager due to request scheme " << request->url.getScheme());
-        CacheManager::GetInstance()->start(clientConn, request, entry, al);
-        return;
-
     case AnyP::PROTO_URN:
         urnStart(request, entry, al);
         return;
@@ -1281,8 +1275,6 @@ FwdState::dispatch()
                 Ftp::StartGateway(this);
             break;
 
-        case AnyP::PROTO_CACHE_OBJECT:
-
         case AnyP::PROTO_URN:
             fatal_dump("Should never get here");
             break;

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 7c484fb850d96df4e4133ade51003776bf9a618e..8c1f4a25ae121c32487a7d922a96bf050b8262af 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -560,9 +560,6 @@ HttpRequest::maybeCacheable()
             return false;
         break;
 
-    case AnyP::PROTO_CACHE_OBJECT:
-        return false;
-
     //case AnyP::PROTO_FTP:
     default:
         break;

diff --git a/src/adaptation/ecap/Host.cc b/src/adaptation/ecap/Host.cc
index 64303b5b5b782da96351688968127d9c38465bb6..defa5a1f2696eb25219f1b90a0e6cd7d1f464567 100644 (file)
--- a/src/adaptation/ecap/Host.cc
+++ b/src/adaptation/ecap/Host.cc
@@ -21,7 +21,6 @@
 #include "MasterXaction.h"
 
 const libecap::Name Adaptation::Ecap::protocolInternal("internal", libecap::Name::NextId());
-const libecap::Name Adaptation::Ecap::protocolCacheObj("cache_object", libecap::Name::NextId());
 const libecap::Name Adaptation::Ecap::protocolIcp("ICP", libecap::Name::NextId());
 #if USE_HTCP
 const libecap::Name Adaptation::Ecap::protocolHtcp("Htcp", libecap::Name::NextId());
@@ -52,7 +51,6 @@ Adaptation::Ecap::Host::Host()
     libecap::protocolWais.assignHostId(AnyP::PROTO_WAIS);
     libecap::protocolUrn.assignHostId(AnyP::PROTO_URN);
     libecap::protocolWhois.assignHostId(AnyP::PROTO_WHOIS);
-    protocolCacheObj.assignHostId(AnyP::PROTO_CACHE_OBJECT);
     protocolIcp.assignHostId(AnyP::PROTO_ICP);
 #if USE_HTCP
     protocolHtcp.assignHostId(AnyP::PROTO_HTCP);

diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc
index 4387bce9fc2b6f4aa4faecedb29015ccc83d9e5e..1c4bdfd298500c1fe42bc8fba4f062a2b5de2dc8 100644 (file)
--- a/src/adaptation/ecap/MessageRep.cc
+++ b/src/adaptation/ecap/MessageRep.cc
@@ -152,8 +152,6 @@ Adaptation::Ecap::FirstLineRep::protocol() const
     case AnyP::PROTO_HTCP:
         return protocolHtcp;
 #endif
-    case AnyP::PROTO_CACHE_OBJECT:
-        return protocolCacheObj;
     case AnyP::PROTO_ICY:
         return protocolIcy;
     case AnyP::PROTO_COAP:

diff --git a/src/anyp/ProtocolType.h b/src/anyp/ProtocolType.h
index 6e5ef8f431e16b09cdc692f183ef65d219525463..eef8268ba203146bc2aaa602b440ddddaa549cbf 100644 (file)
--- a/src/anyp/ProtocolType.h
+++ b/src/anyp/ProtocolType.h
@@ -28,7 +28,6 @@ typedef enum {
     PROTO_COAP,
     PROTO_COAPS,
     PROTO_WAIS,
-    PROTO_CACHE_OBJECT,
     PROTO_ICP,
 #if USE_HTCP
     PROTO_HTCP,

diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc
index e004470c4180fc8483d2be362f4cb785edec2cb6..3eed2366abd4ffe8f0fd818a9fdd1aea9d0bad30 100644 (file)
--- a/src/anyp/Uri.cc
+++ b/src/anyp/Uri.cc
@@ -194,15 +194,8 @@ uriParseScheme(Parser::Tokenizer &tok)
      * Scheme names consist of a sequence of characters beginning with a
      * letter and followed by any combination of letters, digits, plus
      * ("+"), period ("."), or hyphen ("-").
-     *
-     * The underscore ("_") required to match "cache_object://" squid
-     * special URI scheme.
      */
-    static const auto schemeChars =
-#if USE_HTTP_VIOLATIONS
-        CharacterSet("special", "_") +
-#endif
-        CharacterSet("scheme", "+.-") + CharacterSet::ALPHA + CharacterSet::DIGIT;
+    static const auto schemeChars = CharacterSet("scheme", "+.-") + CharacterSet::ALPHA + CharacterSet::DIGIT;
 
     SBuf str;
     if (tok.prefix(str, schemeChars, 16) && tok.skip(':') && CharacterSet::ALPHA[str.at(0)]) {
@@ -946,7 +939,6 @@ urlCheckRequest(const HttpRequest * r)
 
     case AnyP::PROTO_URN:
     case AnyP::PROTO_HTTP:
-    case AnyP::PROTO_CACHE_OBJECT:
         return true;
 
     case AnyP::PROTO_FTP:

diff --git a/src/anyp/UriScheme.cc b/src/anyp/UriScheme.cc
index 4a13f707123e5ed14c5bd9e632274c5f6e5d3035..60a0425bb44600e03111abaa1c6093fe9830ea2c 100644 (file)
--- a/src/anyp/UriScheme.cc
+++ b/src/anyp/UriScheme.cc
@@ -90,9 +90,6 @@ AnyP::UriScheme::defaultPort() const
     case AnyP::PROTO_WAIS:
         return 210;
 
-    case AnyP::PROTO_CACHE_OBJECT:
-        return CACHE_HTTP_PORT;
-
     case AnyP::PROTO_WHOIS:
         return 43;
 

diff --git a/src/cache_manager.cc b/src/cache_manager.cc
index 317386014492515929d6e7a7368787f662a062ed..dfaefed5ca57cc8e88e1e47415fbceaa84e9d576 100644 (file)
--- a/src/cache_manager.cc
+++ b/src/cache_manager.cc
@@ -152,29 +152,15 @@ CacheManager::createRequestedAction(const Mgr::ActionParams &params)
     return cmd->profile->creator->create(cmd);
 }
 
-static const CharacterSet &
-MgrFieldChars(const AnyP::ProtocolType &protocol)
-{
-    // Deprecated cache_object:// scheme used '@' to delimit passwords
-    if (protocol == AnyP::PROTO_CACHE_OBJECT) {
-        static const CharacterSet fieldChars = CharacterSet("cache-object-field", "@?#").complement();
-        return fieldChars;
-    }
-
-    static const CharacterSet actionChars = CharacterSet("mgr-field", "?#").complement();
-    return actionChars;
-}
-
 /**
- * define whether the URL is a cache-manager URL and parse the action
- * requested by the user. Checks via CacheManager::ActionProtection() that the
- * item is accessible by the user.
+ * Parses the action requested by the user and checks via
+ * CacheManager::ActionProtection() that the item is accessible by the user.
  *
  * Syntax:
  *
- *  scheme "://" authority [ '/squid-internal-mgr' ] path-absolute [ '@' unreserved ] '?' query-string
+ * [ scheme "://" authority ] '/squid-internal-mgr' path-absolute [ "?" query ] [ "#" fragment ]
  *
- * see RFC 3986 for definitions of scheme, authority, path-absolute, query-string
+ * see RFC 3986 for definitions of scheme, authority, path-absolute, query
  *
  * \returns Mgr::Command object with action to perform and parameters it might use
  */
@@ -184,23 +170,17 @@ CacheManager::ParseUrl(const AnyP::Uri &uri)
     Parser::Tokenizer tok(uri.path());
 
     static const SBuf internalMagicPrefix("/squid-internal-mgr/");
-    if (!tok.skip(internalMagicPrefix) && !tok.skip('/'))
-        throw TextException("invalid URL path", Here());
+    Assure(tok.skip(internalMagicPrefix));
 
     Mgr::Command::Pointer cmd = new Mgr::Command();
     cmd->params.httpUri = SBufToString(uri.absolute());
 
-    const auto &fieldChars = MgrFieldChars(uri.getScheme());
+    static const auto fieldChars = CharacterSet("mgr-field", "?#").complement();
 
     SBuf action;
     if (!tok.prefix(action, fieldChars)) {
-        if (uri.getScheme() == AnyP::PROTO_CACHE_OBJECT) {
-            static const SBuf menuReport("menu");
-            action = menuReport;
-        } else {
-            static const SBuf indexReport("index");
-            action = indexReport;
-        }
+        static const SBuf indexReport("index");
+        action = indexReport;
     }
     cmd->params.actionName = SBufToString(action);
 
@@ -213,12 +193,6 @@ CacheManager::ParseUrl(const AnyP::Uri &uri)
         throw TextException(ToSBuf("action '", action, "' is ", prot), Here());
     cmd->profile = profile;
 
-    SBuf passwd;
-    if (uri.getScheme() == AnyP::PROTO_CACHE_OBJECT && tok.skip('@')) {
-        (void)tok.prefix(passwd, fieldChars);
-        cmd->params.password = SBufToString(passwd);
-    }
-
     // TODO: fix when AnyP::Uri::parse() separates path?query#fragment
     SBuf params;
     if (tok.skip('?')) {
@@ -230,8 +204,7 @@ CacheManager::ParseUrl(const AnyP::Uri &uri)
         throw TextException("invalid characters in URL", Here());
     // else ignore #fragment (if any)
 
-    debugs(16, 3, "MGR request: host=" << uri.host() << ", action=" << action <<
-           ", password=" << passwd << ", params=" << params);
+    debugs(16, 3, "MGR request: host=" << uri.host() << ", action=" << action << ", params=" << params);
 
     return cmd;
 }

diff --git a/src/cf.data.pre b/src/cf.data.pre
index bf319815ecfe1771e1dc764a7ed5511663b64512..f32ef3faf65ab4c5fb7fa50fb5200e61acb61b40 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1068,7 +1068,7 @@ DEFAULT: ssl::certUntrusted ssl_error X509_V_ERR_INVALID_CA X509_V_ERR_SELF_SIGN
 DEFAULT: ssl::certSelfSigned ssl_error X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT
 ENDIF
 DEFAULT: all src all
-DEFAULT: manager url_regex -i ^cache_object:// +i ^[^:]+://[^/]+/squid-internal-mgr/
+DEFAULT: manager url_regex +i ^[^:]+://[^/]+/squid-internal-mgr/
 DEFAULT: localhost src 127.0.0.1/32 ::1
 DEFAULT: to_localhost dst 127.0.0.0/8 0.0.0.0/32 ::1/128 ::/128
 DEFAULT: to_linklocal dst 169.254.0.0/16 fe80::/10

diff --git a/src/client_side.cc b/src/client_side.cc
index de5c5512a377786df5191541f92f4c21c5b8447a..2d60f2c1fe813bf7f958b1797fbaae2dbafca106 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1120,10 +1120,6 @@ prepareAcceleratedURL(ConnStateData * conn, const Http1::RequestParserPointer &h
 
     /* BUG: Squid cannot deal with '*' URLs (RFC2616 5.1.2) */
 
-    static const SBuf cache_object("cache_object://");
-    if (hp->requestUri().startsWith(cache_object))
-        return nullptr; /* already in good shape */
-
     // XXX: re-use proper URL parser for this
     SBuf url = hp->requestUri(); // use full provided URI if we abort
     do { // use a loop so we can break out of it
@@ -1661,9 +1657,6 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp,
 
     request->flags.internal = http->flags.internal;
 
-    if (request->url.getScheme() == AnyP::PROTO_CACHE_OBJECT)
-        request->flags.disableCacheUse("cache_object URL scheme");
-
     if (!isFtp) {
         // XXX: for non-HTTP messages instantiate a different Http::Message child type
         // for now Squid only supports HTTP requests

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 0643dbca6152e8aae0518388ffb726a52dacc956..ce9cb1af33bb6e6d8fc232ea00b968e2c37f69a1 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -913,9 +913,6 @@ clientHierarchical(ClientHttpRequest * http)
     if (request->url.getScheme() == AnyP::PROTO_HTTP)
         return method.respMaybeCacheable();
 
-    if (request->url.getScheme() == AnyP::PROTO_CACHE_OBJECT)
-        return 0;
-
     return 1;
 }
 

diff --git a/src/tests/testCacheManager.cc b/src/tests/testCacheManager.cc
index 853bdcdb5eeeb63fec060292a6718d70767b47e4..c92124810257f8e4d4262600da2d76f1280fb0ab 100644 (file)
--- a/src/tests/testCacheManager.cc
+++ b/src/tests/testCacheManager.cc
@@ -102,7 +102,6 @@ TestCacheManager::testParseUrl()
     CPPUNIT_ASSERT(mgr != nullptr);
 
     std::vector<AnyP::ProtocolType> validSchemes = {
-        AnyP::PROTO_CACHE_OBJECT,
         AnyP::PROTO_HTTP,
         AnyP::PROTO_HTTPS,
         AnyP::PROTO_FTP
@@ -182,8 +181,8 @@ TestCacheManager::testParseUrl()
 
         for (const auto *magic : magicPrefixes) {
 
-            // all schemes except cache_object require magic path prefix bytes
-            if (scheme != AnyP::PROTO_CACHE_OBJECT && strlen(magic) <= 2)
+            // all schemes require magic path prefix bytes
+            if (strlen(magic) <= 2)
                 continue;
 
             /* Check the parser accepts all the valid cases */

diff --git a/src/tests/testUriScheme.cc b/src/tests/testUriScheme.cc
index 0b2f54d8252dc49060bf63cd8a9042ece2b5af0f..856fa9688dcb62589afc87583614f2ac62019d8e 100644 (file)
--- a/src/tests/testUriScheme.cc
+++ b/src/tests/testUriScheme.cc
@@ -98,7 +98,7 @@ TestUriScheme::testConstructprotocol_t()
     AnyP::UriScheme lhs_none(AnyP::PROTO_NONE), rhs_none(AnyP::PROTO_NONE);
     CPPUNIT_ASSERT_EQUAL(lhs_none, rhs_none);
 
-    AnyP::UriScheme lhs_cacheobj(AnyP::PROTO_CACHE_OBJECT), rhs_cacheobj(AnyP::PROTO_CACHE_OBJECT);
+    AnyP::UriScheme lhs_cacheobj(AnyP::PROTO_HTTP), rhs_cacheobj(AnyP::PROTO_HTTP);
     CPPUNIT_ASSERT_EQUAL(lhs_cacheobj, rhs_cacheobj);
     CPPUNIT_ASSERT(lhs_none != rhs_cacheobj);
 }
@@ -125,7 +125,7 @@ TestUriScheme::testEqualprotocol_t()
     CPPUNIT_ASSERT(AnyP::UriScheme() == AnyP::PROTO_NONE);
     CPPUNIT_ASSERT(not (AnyP::UriScheme(AnyP::PROTO_WAIS) == AnyP::PROTO_HTTP));
     CPPUNIT_ASSERT(AnyP::PROTO_HTTP == AnyP::UriScheme(AnyP::PROTO_HTTP));
-    CPPUNIT_ASSERT(not (AnyP::PROTO_CACHE_OBJECT == AnyP::UriScheme(AnyP::PROTO_HTTP)));
+    CPPUNIT_ASSERT(not (AnyP::PROTO_HTTPS == AnyP::UriScheme(AnyP::PROTO_HTTP)));
 }
 
 /*