On 22/01/2012 11:34 a.m., Henrik Nordström wrote:
sön 2012-01-22 klockan 00:41 +1300 skrev Amos Jeffries:
It adds a host_verify_loose directive which allows requests which fail
Host: validation to continue through processing. The default is OFF for
now to encourage safety. Enabling this does open the clients to some
minor aspects of same-origin bypass again. BUT ...
* 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.
I would say it should be on by default, or actually the directive should
be named host_verify_strict and off by default.
As you note for a client to be vulnerable with this on the client needs
to be vulnerable when used with direct connection without having it's
connections intercepted by Squid.
Having this Host validation enabled breaks valid traffic, something
which we MUST NOT do in default configuration.
It also adds 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.
Short term, add a flag which by default prevents these from being passed
to peers to avoid opening up the original CVE issue when intercepting
and then forwarding to some peers.
We have the forward.cc start() logics for dst-passthru decision to
prevent peers being looked up right now, so they wont even be an option
if the verify fails.
This version updates the hierarchical flag and cacheHierarchical()
function to fix any other code paths which use them to see if a peer is
likely. That will be reverted when the peer relay is done.
Mid term, implement tunneling over peers using CONNECT. Keep in mind
that this requires changes in pconn state to track tunnels separately
from normal peer connections.
That is still TODO. I'd like to get this version with initial loosening
rolled out since it fixes the more commons cases quickly.
Long term, a better forwarding mechanism to allow the peer to see the
traffic proper and avoid the tunnel. Needs to be enabled in cache_peer
or some discovery mechanism added (OPTIONS * ?). Here I still vote for
original IP in requested URL and Host header preserved as discussed
before as it degrades in a safe manner no matter how wrongly things gets
configured.
Regards
Henrik
=== added file 'errors/templates/ERR_CONFLICT_HOST'
--- errors/templates/ERR_CONFLICT_HOST 1970-01-01 00:00:00 +0000
+++ errors/templates/ERR_CONFLICT_HOST 2012-01-16 12:08:08 +0000
@@ -0,0 +1,43 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN"
"http://www.w3.org/TR/html4/strict.dtd";>
+<html><head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+<title>ERROR: The requested URL could not be retrieved</title>
+<style type="text/css"><!--
+ %l
+
+body
+:lang(fa) { direction: rtl; font-size: 100%; font-family: Tahoma, Roya,
sans-serif; float: right; }
+:lang(he) { direction: rtl; }
+ --></style>
+</head><body id=%c>
+<div id="titles">
+<h1>ERROR</h1>
+<h2>The requested URL could not be retrieved</h2>
+</div>
+<hr>
+
+<div id="content">
+<p>The following error was encountered while trying to retrieve the URL: <a
href="%U">%U</a></p>
+
+<blockquote id="data">
+<pre>URI Host Conflict</pre>
+</blockquote>
+
+<p>This means the domain name you are trying to access apparently no longer
exists on the machine you are requesting it from.</p>
+
+<p>Some possible problems are:</p>
+<ul>
+<li>The domain may have moved very recently. Trying again will resolve
that.</li>
+<li>The website may require you to use a local country-based version. Using
your ISP provided DNS server(s) should resolve that.</li>
+</ul>
+
+<p>Your cache administrator is <a href="mailto:%w%W";>%w</a>.</p>
+<br>
+</div>
+
+<hr>
+<div id="footer">
+<p>Generated %T by %h (%s)</p>
+<!-- %c -->
+</div>
+</body></html>
=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc 2012-01-20 18:55:04 +0000
+++ src/HttpRequest.cc 2012-01-27 19:57:26 +0000
@@ -605,6 +605,13 @@
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);
=== modified file 'src/cf.data.pre'
--- src/cf.data.pre 2012-01-27 12:45:06 +0000
+++ src/cf.data.pre 2012-01-30 01:24:19 +0000
@@ -1743,25 +1743,48 @@
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
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2012-01-20 18:55:04 +0000
+++ src/client_side_request.cc 2012-01-23 10:45:34 +0000
@@ -553,6 +553,21 @@
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 @@
clientStreamNode *node = (clientStreamNode
*)http->client_stream.tail->prev->data;
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(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 @@
} else {
// Okay no problem.
debugs(85, 3, HERE << "validate passed.");
+ http->request->flags.hostVerified = 1;
http->doCallouts();
}
safe_free(hostB);
@@ -908,6 +924,10 @@
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
=== modified file 'src/err_type.h'
--- src/err_type.h 2011-12-30 16:01:37 +0000
+++ src/err_type.h 2012-01-17 12:58:32 +0000
@@ -35,6 +35,7 @@
ERR_INVALID_URL,
ERR_ZERO_SIZE_OBJECT,
ERR_PRECONDITION_FAILED,
+ ERR_CONFLICT_HOST,
/* FTP Errors */
ERR_FTP_DISABLED,
=== modified file 'src/forward.cc'
--- src/forward.cc 2012-01-20 18:55:04 +0000
+++ src/forward.cc 2012-01-23 05:39:41 +0000
@@ -120,8 +120,9 @@
// 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;
=== modified file 'src/structs.h'
--- src/structs.h 2012-01-06 20:41:21 +0000
+++ src/structs.h 2012-01-23 08:35:25 +0000
@@ -963,7 +963,9 @@
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
@@ -973,10 +975,10 @@
}
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;
@@ -992,7 +994,8 @@
#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;
