=== modified file 'lib/MemPoolChunked.cc' --- lib/MemPoolChunked.cc 2012-09-01 14:38:36 +0000 +++ lib/MemPoolChunked.cc 2013-01-12 10:01:46 +0000 @@ -141,7 +141,11 @@ next = NULL; pool = aPool; - objCache = xcalloc(1, pool->chunk_size); + if (pool->doZeroOnPush) + objCache = xcalloc(1, pool->chunk_size); + else + objCache = xmalloc(pool->chunk_size); + freeList = objCache; void **Free = (void **)freeList; === modified file 'lib/MemPoolMalloc.cc' --- lib/MemPoolMalloc.cc 2012-09-01 14:38:36 +0000 +++ lib/MemPoolMalloc.cc 2013-01-11 07:16:57 +0000 @@ -56,7 +56,10 @@ memMeterDec(meter.idle); ++saved_calls; } else { - obj = xcalloc(1, obj_size); + if (doZeroOnPush) + obj = xcalloc(1, obj_size); + else + obj = xmalloc(obj_size); memMeterInc(meter.alloc); } memMeterInc(meter.inuse); === modified file 'src/auth/Acl.cc' --- src/auth/Acl.cc 2012-09-19 17:16:56 +0000 +++ src/auth/Acl.cc 2013-01-11 05:02:04 +0000 @@ -26,7 +26,7 @@ return ACCESS_DENIED; } else if (request->flags.sslBumped) { debugs(28, 5, "SslBumped request: It is an encapsulated request do not authenticate"); - checklist->auth_user_request = checklist->conn() != NULL ? checklist->conn()->auth_user_request : request->auth_user_request; + checklist->auth_user_request = checklist->conn() != NULL ? checklist->conn()->getConnectionAuth() : request->auth_user_request; if (checklist->auth_user_request != NULL) return ACCESS_ALLOWED; else === modified file 'src/auth/AclProxyAuth.cc' --- src/auth/AclProxyAuth.cc 2012-08-14 11:53:07 +0000 +++ src/auth/AclProxyAuth.cc 2013-01-11 05:02:04 +0000 @@ -163,7 +163,7 @@ checklist->auth_user_request = NULL; if (checklist->conn() != NULL) { - checklist->conn()->auth_user_request = NULL; + checklist->conn()->setConnectionAuth(NULL, "proxy_auth ACL failure"); } } === modified file 'src/auth/UserRequest.cc' --- src/auth/UserRequest.cc 2012-10-29 04:59:58 +0000 +++ src/auth/UserRequest.cc 2013-01-11 05:02:05 +0000 @@ -221,7 +221,7 @@ {} void -Auth::UserRequest::onConnectionClose(ConnStateData *) +Auth::UserRequest::releaseAuthServer() {} const char * @@ -253,7 +253,7 @@ else if (request != NULL && request->auth_user_request != NULL) return request->auth_user_request; else if (conn != NULL) - return conn->auth_user_request; + return conn->getConnectionAuth(); else return NULL; } @@ -303,7 +303,7 @@ /* connection auth we must reset on auth errors */ if (conn != NULL) { - conn->auth_user_request = NULL; + conn->setConnectionAuth(NULL, "HTTP request missing credentials"); } *auth_user_request = NULL; @@ -315,13 +315,13 @@ * No check for function required in the if: its compulsory for conn based * auth modules */ - if (proxy_auth && conn != NULL && conn->auth_user_request != NULL && - authenticateUserAuthenticated(conn->auth_user_request) && - conn->auth_user_request->connLastHeader() != NULL && - strcmp(proxy_auth, conn->auth_user_request->connLastHeader())) { + if (proxy_auth && conn != NULL && conn->getConnectionAuth() != NULL && + authenticateUserAuthenticated(conn->getConnectionAuth()) && + conn->getConnectionAuth()->connLastHeader() != NULL && + strcmp(proxy_auth, conn->getConnectionAuth()->connLastHeader())) { debugs(29, 2, "WARNING: DUPLICATE AUTH - authentication header on already authenticated connection!. AU " << - conn->auth_user_request << ", Current user '" << - conn->auth_user_request->username() << "' proxy_auth " << + conn->getConnectionAuth() << ", Current user '" << + conn->getConnectionAuth()->username() << "' proxy_auth " << proxy_auth); /* remove this request struct - the link is already authed and it can't be to reauth. */ @@ -330,7 +330,7 @@ * authenticateAuthenticate */ assert(*auth_user_request == NULL); - conn->auth_user_request = NULL; + conn->setConnectionAuth(NULL, "changed credentials token"); } /* we have a proxy auth header and as far as we know this connection has @@ -342,20 +342,20 @@ debugs(29, 9, HERE << "This is a new checklist test on:" << conn->clientConnection); } - if (proxy_auth && request->auth_user_request == NULL && conn != NULL && conn->auth_user_request != NULL) { + if (proxy_auth && request->auth_user_request == NULL && conn != NULL && conn->getConnectionAuth() != NULL) { Auth::Config * scheme = Auth::Config::Find(proxy_auth); - if (conn->auth_user_request->user() == NULL || conn->auth_user_request->user()->config != scheme) { + if (conn->getConnectionAuth()->user() == NULL || conn->getConnectionAuth()->user()->config != scheme) { debugs(29, DBG_IMPORTANT, "WARNING: Unexpected change of authentication scheme from '" << - conn->auth_user_request->user()->config->type() << + conn->getConnectionAuth()->user()->config->type() << "' to '" << proxy_auth << "' (client " << src_addr << ")"); - conn->auth_user_request = NULL; + conn->setConnectionAuth(NULL, "changed auth scheme"); } } - if (request->auth_user_request == NULL && (conn == NULL || conn->auth_user_request == NULL)) { + if (request->auth_user_request == NULL && (conn == NULL || conn->getConnectionAuth() == NULL)) { /* beginning of a new request check */ debugs(29, 4, HERE << "No connection authentication type"); @@ -378,14 +378,14 @@ *auth_user_request = request->auth_user_request; } else { assert (conn != NULL); - if (conn->auth_user_request != NULL) { - *auth_user_request = conn->auth_user_request; + if (conn->getConnectionAuth() != NULL) { + *auth_user_request = conn->getConnectionAuth(); } else { /* failed connection based authentication */ debugs(29, 4, HERE << "Auth user request " << *auth_user_request << " conn-auth user request " << - conn->auth_user_request << " conn type " << - conn->auth_user_request->user()->auth_type << " authentication failed."); + conn->getConnectionAuth() << " conn type " << + conn->getConnectionAuth()->user()->auth_type << " authentication failed."); *auth_user_request = NULL; return AUTH_ACL_CHALLENGE; === modified file 'src/auth/UserRequest.h' --- src/auth/UserRequest.h 2012-11-04 12:27:49 +0000 +++ src/auth/UserRequest.h 2013-01-11 05:02:05 +0000 @@ -145,7 +145,7 @@ /* add the [Proxy-]Authentication-Info trailer */ virtual void addAuthenticationInfoTrailer(HttpReply * rep, int accel); - virtual void onConnectionClose(ConnStateData *); + virtual void releaseAuthServer(); /** * Called when squid is ready to put the request on hold and wait for a callback from the auth module === modified file 'src/auth/digest/User.cc' --- src/auth/digest/User.cc 2012-09-23 09:04:21 +0000 +++ src/auth/digest/User.cc 2013-01-11 05:35:50 +0000 @@ -9,7 +9,9 @@ Auth::Digest::User::User(Auth::Config *aConfig) : Auth::User(aConfig), HA1created(0) -{} +{ + memset(HA1, 0, sizeof(HA1)); +} Auth::Digest::User::~User() { === modified file 'src/auth/negotiate/UserRequest.cc' --- src/auth/negotiate/UserRequest.cc 2012-11-27 21:19:46 +0000 +++ src/auth/negotiate/UserRequest.cc 2013-01-11 05:02:08 +0000 @@ -129,27 +129,6 @@ debugs(29, 6, HERE << "No Negotiate auth server to release."); } -/* clear any connection related authentication details */ -void -Auth::Negotiate::UserRequest::onConnectionClose(ConnStateData *conn) -{ - assert(conn != NULL); - - debugs(29, 8, HERE << "closing connection '" << conn << "' (this is '" << this << "')"); - - if (conn->auth_user_request == NULL) { - debugs(29, 8, HERE << "no auth_user_request"); - return; - } - - releaseAuthServer(); - - /* unlock the connection based lock */ - debugs(29, 9, HERE << "Unlocking auth_user from the connection '" << conn << "'."); - - conn->auth_user_request = NULL; -} - void Auth::Negotiate::UserRequest::authenticate(HttpRequest * aRequest, ConnStateData * conn, http_hdr_type type) { @@ -199,8 +178,8 @@ user()->credentials(Auth::Pending); safe_free(client_blob); client_blob=xstrdup(blob); - assert(conn->auth_user_request == NULL); - conn->auth_user_request = this; + assert(conn->getConnectionAuth() == NULL); + conn->setConnectionAuth(this, "new Negotiate handshake request"); request = aRequest; HTTPMSGLOCK(request); break; === modified file 'src/auth/negotiate/UserRequest.h' --- src/auth/negotiate/UserRequest.h 2012-11-04 12:27:49 +0000 +++ src/auth/negotiate/UserRequest.h 2013-01-11 05:02:08 +0000 @@ -26,7 +26,6 @@ virtual int authenticated() const; virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type); virtual Direction module_direction(); - virtual void onConnectionClose(ConnStateData *); virtual void module_start(AUTHCB *, void *); virtual void addAuthenticationInfoHeader(HttpReply * rep, int accel); === modified file 'src/auth/ntlm/UserRequest.cc' --- src/auth/ntlm/UserRequest.cc 2012-11-27 21:19:46 +0000 +++ src/auth/ntlm/UserRequest.cc 2013-01-11 05:02:08 +0000 @@ -123,26 +123,6 @@ } void -Auth::Ntlm::UserRequest::onConnectionClose(ConnStateData *conn) -{ - assert(conn != NULL); - - debugs(29, 8, HERE << "closing connection '" << conn << "' (this is '" << this << "')"); - - if (conn->auth_user_request == NULL) { - debugs(29, 8, HERE << "no auth_user_request"); - return; - } - - releaseAuthServer(); - - /* unlock the connection based lock */ - debugs(29, 9, HERE << "Unlocking auth_user from the connection '" << conn << "'."); - - conn->auth_user_request = NULL; -} - -void Auth::Ntlm::UserRequest::authenticate(HttpRequest * aRequest, ConnStateData * conn, http_hdr_type type) { assert(this); @@ -192,8 +172,8 @@ user()->credentials(Auth::Pending); safe_free(client_blob); client_blob=xstrdup(blob); - assert(conn->auth_user_request == NULL); - conn->auth_user_request = this; + assert(conn->getConnectionAuth() == NULL); + conn->setConnectionAuth(this, "new NTLM handshake request"); request = aRequest; HTTPMSGLOCK(request); break; === modified file 'src/auth/ntlm/UserRequest.h' --- src/auth/ntlm/UserRequest.h 2012-11-04 12:27:49 +0000 +++ src/auth/ntlm/UserRequest.h 2013-01-11 05:02:08 +0000 @@ -26,14 +26,13 @@ virtual int authenticated() const; virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type); virtual Auth::Direction module_direction(); - virtual void onConnectionClose(ConnStateData *); virtual void module_start(AUTHCB *, void *); virtual const char * connLastHeader(); /* we need to store the helper server between requests */ helper_stateful_server *authserver; - void releaseAuthServer(void); ///< Release authserver NTLM helpers properly when finished or abandoning. + virtual void releaseAuthServer(); ///< Release authserver NTLM helpers properly when finished or abandoning. /* our current blob to pass to the client */ char *server_blob; === modified file 'src/client_side.cc' --- src/client_side.cc 2013-01-11 10:02:21 +0000 +++ src/client_side.cc 2013-01-12 09:53:06 +0000 @@ -789,6 +789,82 @@ deleteThis("ConnStateData::connStateClosed"); } +#if USE_AUTH +void +ConnStateData::setConnectionAuth(const Auth::UserRequest::Pointer &aur, const char *by) +{ + if (credentialsState == NULL) { + debugs(33, 2, HERE << "Adding connection-auth to " << clientConnection << " from " << by); + credentialsState = aur; + return; + } + + // clobered with self-pointer + // NP: something nasty is going on in Squid, but harmless. + if (aur == credentialsState) { + debugs(33, 2, HERE << "WARNING: Ignoring duplicate connection-auth for " << clientConnection << " from " << by); + return; + } + + /* + * Connection-auth relies on a single set of credentials being preserved + * for all requests on a connection once they have been setup. + * There are several things which need to happen to preserve security + * when connection-auth credentials change unexpectedly or are unset. + * + * 1) auth helper released from any active state + * + * They can only be reserved by a handshake process which this + * connection can now never complete. + * This prevents helpers hanging when their connections close. + * + * 2) pinning is expected to be removed and server conn closed + * + * The upstream link is authenticated with the same credentials. + * Expecting the same level of consistency we should have received. + * This prevents upstream being faced with multiple or missing + * credentials after authentication. + * + * 3) the connection needs to close. + * + * This prevents attackers injecting requests into a connection, + * or gateways wrongly multiplexing users into a single connection. + * + * When credentials are missing closure needs to follow an auth + * challenge for best recovery by the client. + * + * When credentials change there is nothing we can do but abort as + * fast as possible. If TCP RST can be sent instead of HTTP response + * that would be the best-case action. + */ + + // clobbered with nul-pointer + if (aur == NULL) { + debugs(33, 2, HERE << "WARNING: Closing " << clientConnection << " due to connection-auth erase from " << by); + credentialsState->releaseAuthServer(); + credentialsState = NULL; + unpinConnection(); + // XXX: need to test whether the connection re-auth challenge is sent. If not, how to trigger it from here. + // NP: the current situation seems to fix challenge loops in Safari without visible issues in others. + deleteThis("connection-auth removed"); + return; + } + + // clobbered with alternative credentials + if (aur != credentialsState) { + debugs(33, 2, HERE << "ERROR: Closing " << clientConnection << " due to change of connection-auth from " << by); + credentialsState->releaseAuthServer(); + credentialsState = NULL; + unpinConnection(); + clientConnection->close(); // XXX: need to send TCP RST packet if we can. + deleteThis("connection-auth change attempted"); + return; + } + + /* NOT REACHABLE */ +} +#endif + // cleans up before destructor is called void ConnStateData::swanSong() @@ -798,10 +874,13 @@ clientdbEstablished(clientConnection->remote, -1); /* decrement */ assert(areAllContextsForThisConnection()); freeAllContexts(); + #if USE_AUTH - if (auth_user_request != NULL) { - debugs(33, 4, "ConnStateData::swanSong: freeing auth_user_request '" << auth_user_request << "' (this is '" << this << "')"); - auth_user_request->onConnectionClose(this); + if (getConnectionAuth() != NULL) { + debugs(33, 4, "ConnStateData::swanSong: freeing connection-auth credentials '" << getConnectionAuth() << "' (this is '" << this << "')"); + // avoid setConnectionAuth() because of its side-effects, but ensure the helpers are released properly + credentialsState->releaseAuthServer(); + credentialsState = NULL; } #endif @@ -2676,8 +2755,8 @@ !conn->port->allow_direct : 0; #if USE_AUTH if (request->flags.sslBumped) { - if (conn->auth_user_request != NULL) - request->auth_user_request = conn->auth_user_request; + if (conn->getConnectionAuth() != NULL) + request->auth_user_request = conn->getConnectionAuth(); } #endif === modified file 'src/client_side.h' --- src/client_side.h 2012-11-04 12:27:49 +0000 +++ src/client_side.h 2013-01-11 05:02:13 +0000 @@ -238,11 +238,28 @@ int64_t mayNeedToReadMoreBody() const; #if USE_AUTH + +private: + /// state of some credentials that can be used to perform authentication on this connection + Auth::UserRequest::Pointer credentialsState; +public: /** * note this is ONLY connection based because NTLM and Negotiate is against HTTP spec. * the user details for connection based authentication */ - Auth::UserRequest::Pointer auth_user_request; + Auth::UserRequest::Pointer getConnectionAuth() const { return credentialsState; }; + + /** + * Set the connection-based credentials to use from now until connection closure. + * + * NP: once set the credentials are fixed until closure because for any change to be needed + * since something invalid has happened: + * - NTLM/Negotiate auth was violated by the per-request headers missing a revalidation token + * - NTLM/Negotiate auth was violated by the per-request headers being for another user + * - SSL-Bump CONNECT tunnel with persistent credentials has ended + */ + void setConnectionAuth(const Auth::UserRequest::Pointer &aur, const char *cause); + #endif /** === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-12-28 15:23:21 +0000 +++ src/client_side_request.cc 2013-01-11 05:02:13 +0000 @@ -613,8 +613,8 @@ http->request, NULL, #if USE_AUTH - http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? - http->getConn()->auth_user_request : http->request->auth_user_request); + http->getConn() != NULL && http->getConn()->getConnectionAuth() != NULL ? + http->getConn()->getConnectionAuth() : http->request->auth_user_request); #else NULL); #endif @@ -782,8 +782,8 @@ #if USE_AUTH char const *proxy_auth_msg = ""; - if (http->getConn() != NULL && http->getConn()->auth_user_request != NULL) - proxy_auth_msg = http->getConn()->auth_user_request->denyMessage(""); + if (http->getConn() != NULL && http->getConn()->getConnectionAuth() != NULL) + proxy_auth_msg = http->getConn()->getConnectionAuth()->denyMessage(""); else if (http->request->auth_user_request != NULL) proxy_auth_msg = http->request->auth_user_request->denyMessage(""); #endif @@ -846,8 +846,8 @@ #if USE_AUTH error->auth_user_request = - http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? - http->getConn()->auth_user_request : http->request->auth_user_request; + http->getConn() != NULL && http->getConn()->getConnectionAuth() != NULL ? + http->getConn()->getConnectionAuth() : http->request->auth_user_request; #endif readNextRequest = true; @@ -1497,7 +1497,7 @@ #if USE_AUTH // Preserve authentication info for the ssl-bumped request if (request->auth_user_request != NULL) - getConn()->auth_user_request = request->auth_user_request; + getConn()->setConnectionAuth(request->auth_user_request, "SSL-bumped CONNECT"); #endif assert(sslBumpNeeded()); @@ -1964,7 +1964,7 @@ ); #if USE_AUTH calloutContext->error->auth_user_request = - c != NULL && c->auth_user_request != NULL ? c->auth_user_request : request->auth_user_request; + c != NULL && c->getConnectionAuth() != NULL ? c->getConnectionAuth() : request->auth_user_request; #endif calloutContext->error->detailError(errDetail); calloutContext->readNextRequest = true; === modified file 'src/redirect.cc' --- src/redirect.cc 2012-11-30 11:08:47 +0000 +++ src/redirect.cc 2013-01-11 05:02:45 +0000 @@ -279,8 +279,8 @@ http->request, NULL, #if USE_AUTH - http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? - http->getConn()->auth_user_request : http->request->auth_user_request); + http->getConn() != NULL && http->getConn()->getConnectionAuth() != NULL ? + http->getConn()->getConnectionAuth() : http->request->auth_user_request); #else NULL); #endif === modified file 'src/tests/Stub.list' --- src/tests/Stub.list 2012-11-02 00:13:41 +0000 +++ src/tests/Stub.list 2013-01-11 04:59:49 +0000 @@ -17,6 +17,7 @@ tests/stub_cache_manager.cc \ tests/stub_cbdata.cc \ tests/stub_client_db.cc \ + tests/stub_client_side.cc \ tests/stub_client_side_request.cc \ tests/stub_comm.cc \ tests/stub_debug.cc \