From ca7d0e52ad8efb758fd25c98cf9d31286c04a39b Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Sat, 4 Feb 2012 23:35:38 -0700 Subject: [PATCH] Bug 3478: pt1: Host verify catching dynamic CDN hosted sites This add a bit more leniency to the Host: header validation without re-opening Squid to the cache poisoning risks involved.Resolving most issues with websites using geo-based DNS results and/or short DNS TTL for load balancing. It alters host_verify_strict directive to allow requests which fail Host: validation to continue through processing. The default remains OFF. * blocks caching on the response to protect all other network clients against one compromised client spreading infections. * forces the original (untrusted) destination IP to be used instead of any alternative Squid might find. Preventing Squid or peer DNS lookup being the point of vulnerability for the same-origin bypass. For any client to be vulnerable it must be vulnerable inside the browser agent where the original TCP connection is established. Also add a new error template ERR_CONFLICT_HOST to replace the confusing "invalid request" message with a clear explanation of the problem and some client workarounds. FUTURE WORK: * adapt processing to allow these requests to safely be passed to peers. * adapt caching to permit safe sharing between clients making identical requests to same sources. --- errors/template.list | 1 + errors/templates/ERR_CONFLICT_HOST | 43 ++++++++++++++++++++++++ src/HttpRequest.cc | 7 ++++ src/cf.data.pre | 53 +++++++++++++++++++++--------- src/client_side_request.cc | 22 ++++++++++++- src/err_type.h | 1 + src/forward.cc | 5 +-- src/structs.h | 11 ++++--- 8 files changed, 121 insertions(+), 22 deletions(-) create mode 100644 errors/templates/ERR_CONFLICT_HOST diff --git a/errors/template.list b/errors/template.list index 24b216276b..e0301c480a 100644 --- a/errors/template.list +++ b/errors/template.list @@ -5,6 +5,7 @@ ERROR_TEMPLATES= \ templates/ERR_CACHE_ACCESS_DENIED \ templates/ERR_CACHE_MGR_ACCESS_DENIED \ templates/ERR_CANNOT_FORWARD \ + templates/ERR_CONFLICT_HOST \ templates/ERR_CONNECT_FAIL \ templates/ERR_DIR_LISTING \ templates/ERR_DNS_FAIL \ diff --git a/errors/templates/ERR_CONFLICT_HOST b/errors/templates/ERR_CONFLICT_HOST new file mode 100644 index 0000000000..9e8347e1b5 --- /dev/null +++ b/errors/templates/ERR_CONFLICT_HOST @@ -0,0 +1,43 @@ + + + +ERROR: The requested URL could not be retrieved + + +
+

ERROR

+

The requested URL could not be retrieved

+
+
+ +
+

The following error was encountered while trying to retrieve the URL: %U

+ +
+
URI Host Conflict
+
+ +

This means the domain name you are trying to access apparently no longer exists on the machine you are requesting it from.

+ +

Some possible problems are:

+ + +

Your cache administrator is %w.

+
+
+ +
+ + diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 1bdbde357f..b464e65ece 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -605,6 +605,13 @@ HttpRequest::CreateFromUrl(char * url) bool HttpRequest::cacheable() const { + // Intercepted request with Host: header which cannot be trusted. + // Because it failed verification, or someone bypassed the security tests + // we cannot cache the reponse for sharing between clients. + // TODO: update cache to store for particular clients only (going to same Host: and destination IP) + if (!flags.hostVerified && (flags.intercepted || flags.spoof_client_ip)) + return false; + if (protocol == AnyP::PROTO_HTTP) return httpCachable(method); diff --git a/src/cf.data.pre b/src/cf.data.pre index 14d34453bb..425c6dfd0c 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1741,25 +1741,48 @@ LOC: Config.onoff.hostStrictVerify DOC_START Regardless of this option setting, when dealing with intercepted traffic, Squid always verifies that the destination IP address matches - the Host header domain or IP (called 'authority form URL'). Squid - responds with an HTTP 409 (Conflict) error page and logs a security - warning if there is no match. - - When set to ON, Squid verifies that the destination IP address matches - the Host header for forward-proxy and reverse-proxy traffic as well. For - those traffic types, Squid also enables the following checks, comparing - the corresponding Host header and Request-URI components: - - * The host names (domain or IP) must be identical, - but valueless or missing Host header disables all checks. - For the two host names to match, both must be either IP or FQDN. - - * Port numbers must be identical, - but if a port is missing, the scheme-default port is assumed. + the Host header domain or IP (called 'authority form URL'). This enforcement is performed to satisfy a MUST-level requirement in RFC 2616 section 14.23: "The Host field value MUST represent the naming authority of the origin server or gateway given by the original URL". + + When set to ON: + Squid always responds with an HTTP 409 (Conflict) error + page and logs a security warning if there is no match. + + Squid verifies that the destination IP address matches + the Host header for forward-proxy and reverse-proxy traffic + as well. For those traffic types, Squid also enables the + following checks, comparing the corresponding Host header + and Request-URI components: + + * The host names (domain or IP) must be identical, + but valueless or missing Host header disables all checks. + For the two host names to match, both must be either IP + or FQDN. + + * Port numbers must be identical, but if a port is missing + the scheme-default port is assumed. + + + When set to OFF (the default): + Squid allows suspicious requests to continue but logs a + security warning and blocks caching of the response. + + * Forward-proxy traffic is not checked at all. + + * Reverse-proxy traffic is not checked at all. + + * Intercepted traffic which passes verification is handled + normally. + + For now it also forces suspicious requests to go DIRECT to the + original destination, overriding client_dst_passthru for + intercepted requests which fail Host: verification. + + For now suspicious intercepted CONNECT requests are always + responded to with an HTTP 409 (Conflict) error page. DOC_END NAME: client_dst_passthru diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 6cff12df8b..0c3113b0e9 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -553,6 +553,21 @@ ClientRequestContext::hostHeaderIpVerify(const ipcache_addrs* ia, const DnsLooku void ClientRequestContext::hostHeaderVerifyFailed(const char *A, const char *B) { + // IP address validation for Host: failed. Admin wants to ignore them. + // NP: we do not yet handle CONNECT tunnels well, so ignore for them + if (!Config.onoff.hostStrictVerify && http->request->method != METHOD_CONNECT) { + debugs(85, 3, "SECURITY ALERT: Host header forgery detected on " << http->getConn()->clientConnection << + " (" << A << " does not match " << B << ") on URL: " << urlCanonical(http->request)); + + // NP: it is tempting to use 'flags.nocache' but that is all about READing cache data. + // The problems here are about WRITE for new cache content, which means flags.cachable + http->request->flags.cachable = 0; // MUST NOT cache (for now) + // XXX: when we have updated the cache key to base on raw-IP + URI this cacheable limit can go. + http->request->flags.hierarchical = 0; // MUST NOT pass to peers (for now) + // XXX: when we have sorted out the best way to relay requests properly to peers this hierarchical limit can go. + return; + } + debugs(85, DBG_IMPORTANT, "SECURITY ALERT: Host header forgery detected on " << http->getConn()->clientConnection << " (" << A << " does not match " << B << ")"); debugs(85, DBG_IMPORTANT, "SECURITY ALERT: By user agent: " << http->request->header.getStr(HDR_USER_AGENT)); @@ -562,7 +577,7 @@ ClientRequestContext::hostHeaderVerifyFailed(const char *A, const char *B) clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data; clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); - repContext->setReplyToError(ERR_INVALID_REQ, HTTP_CONFLICT, + repContext->setReplyToError(ERR_CONFLICT_HOST, HTTP_CONFLICT, http->request->method, NULL, http->getConn()->clientConnection->remote, http->request, @@ -657,6 +672,7 @@ ClientRequestContext::hostHeaderVerify() } else { // Okay no problem. debugs(85, 3, HERE << "validate passed."); + http->request->flags.hostVerified = 1; http->doCallouts(); } safe_free(hostB); @@ -882,6 +898,10 @@ clientHierarchical(ClientHttpRequest * http) HttpRequestMethod method = request->method; const wordlist *p = NULL; + // intercepted requests MUST NOT (yet) be sent to peers unless verified + if (!request->flags.hostVerified && (request->flags.intercepted || request->flags.spoof_client_ip)) + return 0; + /* * IMS needs a private key, so we can use the hierarchy for IMS only if our * neighbors support private keys diff --git a/src/err_type.h b/src/err_type.h index a8c28ae7ea..9c5c91043b 100644 --- a/src/err_type.h +++ b/src/err_type.h @@ -35,6 +35,7 @@ typedef enum { ERR_INVALID_URL, ERR_ZERO_SIZE_OBJECT, ERR_PRECONDITION_FAILED, + ERR_CONFLICT_HOST, /* FTP Errors */ ERR_FTP_DISABLED, diff --git a/src/forward.cc b/src/forward.cc index 168e2c53e7..b1ef8a3bbe 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -120,8 +120,9 @@ void FwdState::start(Pointer aSelf) // Bug 3243: CVE 2009-0801 // Bypass of browser same-origin access control in intercepted communication // To resolve this we must force DIRECT and only to the original client destination. - if (Config.onoff.client_dst_passthru && request && !request->flags.redirected && - (request->flags.intercepted || request->flags.spoof_client_ip)) { + const bool isIntercepted = request && !request->flags.redirected && (request->flags.intercepted || request->flags.spoof_client_ip); + const bool useOriginalDst = Config.onoff.client_dst_passthru || (request && !request->flags.hostVerified); + if (isIntercepted && useOriginalDst) { Comm::ConnectionPointer p = new Comm::Connection(); p->remote = clientConn->local; p->peerType = ORIGINAL_DST; diff --git a/src/structs.h b/src/structs.h index a807ca8486..07716eae47 100644 --- a/src/structs.h +++ b/src/structs.h @@ -978,7 +978,9 @@ struct _iostats { struct request_flags { - request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),fail_on_validation_err(0),stale_if_hit(0),accelerated(0),ignore_cc(0),intercepted(0),spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),sslBumped(0),destinationIPLookedUp_(0) { + request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),fail_on_validation_err(0),stale_if_hit(0),accelerated(0),ignore_cc(0),intercepted(0), + hostVerified(0), + spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),sslBumped(0),destinationIPLookedUp_(0) { #if USE_HTTP_VIOLATIONS nocache_hack = 0; #endif @@ -988,10 +990,10 @@ struct request_flags { } unsigned int range:1; - unsigned int nocache:1; + unsigned int nocache:1; ///< whether the response to this request may be READ from cache unsigned int ims:1; unsigned int auth:1; - unsigned int cachable:1; + unsigned int cachable:1; ///< whether the response to thie request may be stored in the cache unsigned int hierarchical:1; unsigned int loopdetect:1; unsigned int proxy_keepalive:1; @@ -1007,7 +1009,8 @@ unsigned int proxying: #endif unsigned int accelerated:1; unsigned int ignore_cc:1; - unsigned int intercepted:1; /**< transparently intercepted request */ + unsigned int intercepted:1; ///< intercepted request + unsigned int hostVerified:1; ///< whether the Host: header passed verification unsigned int spoof_client_ip:1; /**< spoof client ip if possible */ unsigned int internal:1; unsigned int internalclient:1; -- 2.47.2