This patch seeks to reduce the constant MISS situation we have from
false failures doing Host: header verify.
The concept is that for certain requests we have an additional piece
added to the cache key, HttpRequest::storeSecurityKey.
* when looking up a StoreEntry to be a HIT we check for the "normal"
URL or Store-ID based key first.
* when no StoreEntry can be found for the "normal" entry, we lookup a
key with the additional piece.
Using the original dst-IP as an optional key extension for requests
suspected of Host forgery will allow groups of clients to HIT on the
same suspected forged response if they all happen to get the same IP for
the server. Such clients would have been passed-thru to the same server
anyway and get back whatever result it provides, valid or hijacked.
The expected behaviour by Squid for the troublesome servers is the same
as if they were emitting broken Vary: header. Squid will cache a variant
for each destination IP that fails verification, with a mix of HIT/MISS,
until one suddenly passes and all traffic becomes HITs on the
non-varying entry.
NOTE: due to my sparse level of knowledge about the store API complexity
I am needing help testing that the patch attached actually does the
above behaviour.
Specifically that it does not cache one of the suspect objects in a way
that leaves it able to be HIT on without the dst-IP security key.
Amos
=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc 2014-04-27 07:59:17 +0000
+++ src/HttpRequest.cc 2014-05-11 07:29:34 +0000
@@ -262,40 +262,44 @@
#endif
// This may be too conservative for the 204 No Content case
// may eventually need cloneNullAdaptationImmune() for that.
flags = aReq->flags.cloneAdaptationImmune();
errType = aReq->errType;
errDetail = aReq->errDetail;
#if USE_AUTH
auth_user_request = aReq->auth_user_request;
extacl_user = aReq->extacl_user;
extacl_passwd = aReq->extacl_passwd;
#endif
myportname = aReq->myportname;
// main property is which connection the request was received on (if any)
clientConnectionManager = aReq->clientConnectionManager;
notes = aReq->notes;
+
+ // ensure the security context for this request is always copied
+ storeSecurityKey = aReq->storeSecurityKey;
+
return true;
}
/**
* Checks the first line of an HTTP request is valid
* currently just checks the request method is present.
*
* NP: Other errors are left for detection later in the parse.
*/
bool
HttpRequest::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len,
Http::StatusCode *error)
{
// content is long enough to possibly hold a reply
// 2 being magic size of a 1-byte request method plus space delimiter
if ( buf->contentSize() < 2 ) {
// this is ony a real error if the headers apparently complete.
if (hdr_len > 0) {
debugs(58, 3, HERE << "Too large request header (" << hdr_len << "
bytes)");
*error = Http::scInvalidHeader;
}
=== modified file 'src/HttpRequest.h'
--- src/HttpRequest.h 2014-04-27 07:59:17 +0000
+++ src/HttpRequest.h 2014-05-10 14:01:20 +0000
@@ -209,40 +209,43 @@
NotePairs::Pointer notes; ///< annotations added by the note directive and
helpers
String tag; /* Internal tag for this request */
String extacl_user; /* User name returned by extacl lookup
*/
String extacl_passwd; /* Password returned by extacl lookup */
String extacl_log; /* String to be used for access.log purposes */
String extacl_message; /* String to be used for error page purposes */
#if FOLLOW_X_FORWARDED_FOR
String x_forwarded_for_iterator; /* XXX a list of IP addresses */
#endif /* FOLLOW_X_FORWARDED_FOR */
/// A strong etag of the cached entry. Used for refreshing that entry.
String etag;
+ /// An additional context to add to the store-ID key for security.
+ String storeSecurityKey;
+
public:
bool multipartRangeRequest() const;
bool parseFirstLine(const char *start, const char *end);
int parseHeader(const char *parse_start, int len);
virtual bool expectingBody(const HttpRequestMethod& unused, int64_t&)
const;
bool bodyNibbled() const; // the request has a [partially] consumed body
int prefixLen();
void swapOut(StoreEntry * e);
void pack(Packer * p);
static void httpRequestPack(void *obj, Packer *p);
static HttpRequest * CreateFromUrlAndMethod(char * url, const
HttpRequestMethod& method);
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2014-04-27 07:59:17 +0000
+++ src/client_side_request.cc 2014-05-11 08:16:18 +0000
@@ -547,44 +547,50 @@
http->request->flags.hostVerified = true;
http->doCallouts();
return;
}
debugs(85, 3, HERE << "validate IP " << clientConn->local << "
non-match from Host: IP " << ia->in_addrs[i]);
}
}
debugs(85, 3, HERE << "FAIL: validate IP " << clientConn->local << "
possible from Host:");
hostHeaderVerifyFailed("local IP", "any domain IP");
}
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 !=
Http::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));
+/* XXX
// 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 = false; // MUST NOT cache (for now)
- // XXX: when we have updated the cache key to base on raw-IP + URI
this cacheable limit can go.
+*/
+ // add a security context (the original dst IP) to the cache key
+ // so that only clients sharing this same suspect request can get to it
+ char ipbuf[MAX_IPSTRLEN+1];
+ http->request->storeSecurityKey =
http->getConn()->clientConnection->local.toUrl(ipbuf, sizeof(ipbuf)-1);
+
http->request->flags.hierarchical = false; // 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.
http->doCallouts();
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));
debugs(85, DBG_IMPORTANT, "SECURITY ALERT: on URL: " <<
urlCanonical(http->request));
// IP address validation for Host: failed. reject the connection.
clientStreamNode *node = (clientStreamNode
*)http->client_stream.tail->prev->data;
clientReplyContext *repContext = dynamic_cast<clientReplyContext
*>(node->data.getRaw());
assert (repContext);
repContext->setReplyToError(ERR_CONFLICT_HOST, Http::scConflict,
http->request->method, NULL,
http->getConn()->clientConnection->remote,
http->request,
NULL,
=== modified file 'src/http.cc'
--- src/http.cc 2014-04-27 07:59:17 +0000
+++ src/http.cc 2014-05-11 10:15:27 +0000
@@ -239,57 +239,59 @@
default:
#if QUESTIONABLE
/*
* Any 2xx response should eject previously cached entities...
*/
if (status >= 200 && status < 300)
remove = 1;
#endif
break;
}
if (!remove && !forbidden)
return;
assert(e->mem_obj);
+ // storeGetPublicByRequest() returns best-match from a set of entries.
+ // So we loop to invalidate all the entries it can find.
+ do {
+
if (e->mem_obj->request)
pe = storeGetPublicByRequest(e->mem_obj->request);
else
pe = storeGetPublic(e->mem_obj->storeId(), e->mem_obj->method);
if (pe != NULL) {
assert(e != pe);
#if USE_HTCP
neighborsHtcpClear(e, NULL, e->mem_obj->request, e->mem_obj->method,
HTCP_CLR_INVALIDATION);
#endif
pe->release();
}
+ } while (pe != NULL);
- /** \par
- * Also remove any cached HEAD response in case the object has
- * changed.
- */
+ // also remove any cached HEAD response in case the object has changed
if (e->mem_obj->request)
pe = storeGetPublicByRequestMethod(e->mem_obj->request,
Http::METHOD_HEAD);
else
pe = storeGetPublic(e->mem_obj->storeId(), Http::METHOD_HEAD);
if (pe != NULL) {
assert(e != pe);
#if USE_HTCP
neighborsHtcpClear(e, NULL, e->mem_obj->request,
HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_INVALIDATION);
#endif
pe->release();
}
}
void
HttpStateData::processSurrogateControl(HttpReply *reply)
{
if (request->flags.accelerated && reply->surrogate_control) {
HttpHdrScTarget *sctusable =
reply->surrogate_control->getMergedTarget(Config.Accel.surrogate_id);
=== modified file 'src/store.cc'
--- src/store.cc 2014-02-21 10:46:19 +0000
+++ src/store.cc 2014-05-11 10:17:51 +0000
@@ -594,41 +594,48 @@
StoreEntry::getPublic (StoreClient *aClient, const char *uri, const
HttpRequestMethod& method)
{
assert (aClient);
StoreEntry *result = storeGetPublic (uri, method);
if (!result)
result = NullStoreEntry::getInstance();
aClient->created (result);
}
StoreEntry *
storeGetPublic(const char *uri, const HttpRequestMethod& method)
{
return Store::Root().get(storeKeyPublic(uri, method));
}
StoreEntry *
storeGetPublicByRequestMethod(HttpRequest * req, const HttpRequestMethod&
method)
{
- return Store::Root().get(storeKeyPublicByRequestMethod(req, method));
+ // prefer a general-access variant if one is available
+ StoreEntry *e = Store::Root().get(storeKeyPublicByRequestMethod(req,
method));
+
+ // use a private variant if necessary
+ if (!e && req->storeSecurityKey.size() > 0)
+ e = Store::Root().get(storeKeyPublicByRequestMethod(req, method,
true));
+
+ return e;
}
StoreEntry *
storeGetPublicByRequest(HttpRequest * req)
{
StoreEntry *e = storeGetPublicByRequestMethod(req, req->method);
if (e == NULL && req->method == Http::METHOD_HEAD)
/* We can generate a HEAD reply from a cached GET object */
e = storeGetPublicByRequestMethod(req, Http::METHOD_GET);
return e;
}
static int
getKeyCounter(void)
{
static int key_counter = 0;
if (++key_counter < 0)
@@ -752,51 +759,51 @@
if (vary.size() > 0) {
/* Again, we own this structure layout */
rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf());
vary.clean();
}
#endif
pe->replaceHttpReply(rep, false); // no write until key is public
pe->timestampsSet();
pe->makePublic();
pe->startWriting(); // after makePublic()
pe->complete();
pe->unlock("StoreEntry::setPublicKey+Vary");
}
- newkey = storeKeyPublicByRequest(mem_obj->request);
+ newkey = storeKeyPublicByRequest(mem_obj->request, true);
} else
newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method);
if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) {
debugs(20, 3, "Making old " << *e2 << " private.");
e2->setPrivateKey();
e2->release();
if (mem_obj->request)
- newkey = storeKeyPublicByRequest(mem_obj->request);
+ newkey = storeKeyPublicByRequest(mem_obj->request, true);
else
newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method);
}
if (key)
hashDelete();
EBIT_CLR(flags, KEY_PRIVATE);
hashInsert(newkey);
if (swap_filen > -1)
storeDirSwapLog(this, SWAP_LOG_ADD);
}
StoreEntry *
storeCreatePureEntry(const char *url, const char *log_url, const RequestFlags
&flags, const HttpRequestMethod& method)
{
StoreEntry *e = NULL;
debugs(20, 3, "storeCreateEntry: '" << url << "'");
=== modified file 'src/store_key_md5.cc'
--- src/store_key_md5.cc 2013-06-27 21:04:01 +0000
+++ src/store_key_md5.cc 2014-05-11 10:02:58 +0000
@@ -115,61 +115,66 @@
SquidMD5Update(&M, (unsigned char *) &method, sizeof(method));
SquidMD5Update(&M, (unsigned char *) url, strlen(url));
SquidMD5Final(digest, &M);
return digest;
}
const cache_key *
storeKeyPublic(const char *url, const HttpRequestMethod& method)
{
static cache_key digest[SQUID_MD5_DIGEST_LENGTH];
unsigned char m = (unsigned char) method.id();
SquidMD5_CTX M;
SquidMD5Init(&M);
SquidMD5Update(&M, &m, sizeof(m));
SquidMD5Update(&M, (unsigned char *) url, strlen(url));
SquidMD5Final(digest, &M);
return digest;
}
const cache_key *
-storeKeyPublicByRequest(HttpRequest * request)
+storeKeyPublicByRequest(HttpRequest * request, bool useSecurityContext)
{
- return storeKeyPublicByRequestMethod(request, request->method);
+ return storeKeyPublicByRequestMethod(request, request->method,
useSecurityContext);
}
const cache_key *
-storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod&
method)
+storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod&
method, bool useSecurityContext)
{
static cache_key digest[SQUID_MD5_DIGEST_LENGTH];
unsigned char m = (unsigned char) method.id();
const char *url = request->storeId(); /* storeId returns the right
storeID\canonical URL for the md5 calc */
SquidMD5_CTX M;
SquidMD5Init(&M);
SquidMD5Update(&M, &m, sizeof(m));
SquidMD5Update(&M, (unsigned char *) url, strlen(url));
if (request->vary_headers) {
SquidMD5Update(&M, (unsigned char *) request->vary_headers,
strlen(request->vary_headers));
debugs(20, 3, "updating public key by vary headers: " <<
request->vary_headers << " for: " << url);
}
+ if (useSecurityContext && request->storeSecurityKey.size() > 0) {
+ SquidMD5Update(&M, (unsigned char *)
request->storeSecurityKey.rawBuf(), request->storeSecurityKey.size());
+ debugs(20, 3, "updating public key by security context: " <<
request->storeSecurityKey << " for: " << url);
+ }
+
SquidMD5Final(digest, &M);
return digest;
}
cache_key *
storeKeyDup(const cache_key * key)
{
cache_key *dup = (cache_key *)memAllocate(MEM_MD5_DIGEST);
memcpy(dup, key, SQUID_MD5_DIGEST_LENGTH);
return dup;
}
cache_key *
storeKeyCopy(cache_key * dst, const cache_key * src)
{
memcpy(dst, src, SQUID_MD5_DIGEST_LENGTH);
return dst;
}
=== modified file 'src/store_key_md5.h'
--- src/store_key_md5.h 2012-09-21 14:57:30 +0000
+++ src/store_key_md5.h 2014-05-11 08:54:29 +0000
@@ -28,31 +28,31 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
*
*/
#ifndef SQUID_STORE_KEY_MD5_H_
#define SQUID_STORE_KEY_MD5_H_
#include "hash.h"
#include "typedefs.h"
class HttpRequestMethod;
class HttpRequest;
cache_key *storeKeyDup(const cache_key *);
cache_key *storeKeyCopy(cache_key *, const cache_key *);
void storeKeyFree(const cache_key *);
const cache_key *storeKeyScan(const char *);
const char *storeKeyText(const cache_key *);
const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&);
-const cache_key *storeKeyPublicByRequest(HttpRequest *);
-const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const
HttpRequestMethod&);
+const cache_key *storeKeyPublicByRequest(HttpRequest *, bool
useSecurityContext = false);
+const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const
HttpRequestMethod&, bool useSecurityContext = false);
const cache_key *storeKeyPrivate(const char *, const HttpRequestMethod&, int);
int storeKeyHashBuckets(int);
int storeKeyNull(const cache_key *);
void storeKeyInit(void);
extern HASHHASH storeKeyHashHash;
extern HASHCMP storeKeyHashCmp;
#endif /* SQUID_STORE_KEY_MD5_H_ */
