From: Amos Jeffries Date: Sat, 17 Jan 2015 09:15:53 +0000 (-0800) Subject: Bug 3997: Excessive NTLM or Negotiate auth helper annotations X-Git-Tag: merge-candidate-3-v1~338 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c10ebce8035c25cc0d2c9b27f0720e91c4885d8e;p=thirdparty%2Fsquid.git Bug 3997: Excessive NTLM or Negotiate auth helper annotations With the transaction annotations feature added in Squid-3.4 auth helper response values get recorded as annotatiions. In the case of NTLM and Negotiate authentication the helper response contains a large credentials token which changes frequently. Also, user credentials state is cached. In the case of NTLM and Negotiate the active credentials are cached in the TCP connection state data, but also for the cache mgr helper reports make use of caching in a global username cache. When these two features are combined, the global username cache for mgr reporting accumulates all TCP connection specific token= values presented by the client on all its connections, and any changes to the token over its lifetime. The result is that for users performing either many transactions, or staying connected for long periods the memory consumption from unnecesarily stored tokens is excessive. When clients do both the machine memory can be consumed, and the CPU can reach 100% consumption just walking the annotations lists during regular operations. To fix this we drop the security credentials tokens from cached annotations list in NTLM and Negotiate. Digest is also included though its HA1 token value is static it has similar privacy issues related to storage. Also, use the new 3.5 APi for username cache key creation to build the global username cache key for NTLM/Negotiate using the TCP connection specific token so that credentials and associated tokens do not get accidentally shared between connections and the manager can accurately report users. --- diff --git a/src/auth/digest/UserRequest.cc b/src/auth/digest/UserRequest.cc index f1f08bd333..19ea3115de 100644 --- a/src/auth/digest/UserRequest.cc +++ b/src/auth/digest/UserRequest.cc @@ -329,6 +329,8 @@ Auth::Digest::UserRequest::HandleReply(void *data, const Helper::Reply &reply) // add new helper kv-pair notes to the credentials object // so that any transaction using those credentials can access them auth_user_request->user()->notes.appendNewOnly(&reply.notes); + // remove any private credentials detail which got added. + auth_user_request->user()->notes.remove("ha1"); static bool oldHelperWarningDone = false; switch (reply.result) { diff --git a/src/auth/negotiate/Config.cc b/src/auth/negotiate/Config.cc index b85678c49f..f3d8193a5d 100644 --- a/src/auth/negotiate/Config.cc +++ b/src/auth/negotiate/Config.cc @@ -252,7 +252,7 @@ authenticateNegotiateStats(StoreEntry * sentry) * Auth_user structure. */ Auth::UserRequest::Pointer -Auth::Negotiate::Config::decode(char const *, const char *aRequestRealm) +Auth::Negotiate::Config::decode(char const *proxy_auth, const char *aRequestRealm) { Auth::Negotiate::User *newUser = new Auth::Negotiate::User(Auth::Config::Find("negotiate"), aRequestRealm); Auth::UserRequest *auth_user_request = new Auth::Negotiate::UserRequest(); @@ -261,6 +261,8 @@ Auth::Negotiate::Config::decode(char const *, const char *aRequestRealm) auth_user_request->user(newUser); auth_user_request->user()->auth_type = Auth::AUTH_NEGOTIATE; + auth_user_request->user()->BuildUserKey(proxy_auth, aRequestRealm); + /* all we have to do is identify that it's Negotiate - the helper does the rest */ debugs(29, 9, HERE << "decode Negotiate authentication"); return auth_user_request; diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc index ea76a56820..4ac86dcb86 100644 --- a/src/auth/negotiate/UserRequest.cc +++ b/src/auth/negotiate/UserRequest.cc @@ -259,6 +259,8 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const Helper::Reply &reply // add new helper kv-pair notes to the credentials object // so that any transaction using those credentials can access them auth_user_request->user()->notes.appendNewOnly(&reply.notes); + // remove any private credentials detail which got added. + auth_user_request->user()->notes.remove("token"); Auth::Negotiate::UserRequest *lm_request = dynamic_cast(auth_user_request.getRaw()); assert(lm_request != NULL); @@ -311,8 +313,7 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const Helper::Reply &reply /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); - /* see if this is an existing user with a different proxy_auth - * string */ + /* see if this is an existing user */ AuthUserHashPointer *usernamehash = static_cast(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->userKey())); Auth::User::Pointer local_auth_user = lm_request->user(); while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NEGOTIATE || diff --git a/src/auth/ntlm/Config.cc b/src/auth/ntlm/Config.cc index 7dafc6db2a..dbbd479273 100644 --- a/src/auth/ntlm/Config.cc +++ b/src/auth/ntlm/Config.cc @@ -232,7 +232,7 @@ authenticateNTLMStats(StoreEntry * sentry) * Auth_user structure. */ Auth::UserRequest::Pointer -Auth::Ntlm::Config::decode(char const *, const char *aRequestRealm) +Auth::Ntlm::Config::decode(char const *proxy_auth, const char *aRequestRealm) { Auth::Ntlm::User *newUser = new Auth::Ntlm::User(Auth::Config::Find("ntlm"), aRequestRealm); Auth::UserRequest::Pointer auth_user_request = new Auth::Ntlm::UserRequest(); @@ -241,6 +241,8 @@ Auth::Ntlm::Config::decode(char const *, const char *aRequestRealm) auth_user_request->user(newUser); auth_user_request->user()->auth_type = Auth::AUTH_NTLM; + auth_user_request->user()->BuildUserKey(proxy_auth, aRequestRealm); + /* all we have to do is identify that it's NTLM - the helper does the rest */ debugs(29, 9, HERE << "decode: NTLM authentication"); return auth_user_request; diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc index 0052cf1f55..9dbba52619 100644 --- a/src/auth/ntlm/UserRequest.cc +++ b/src/auth/ntlm/UserRequest.cc @@ -254,6 +254,8 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const Helper::Reply &reply) // add new helper kv-pair notes to the credentials object // so that any transaction using those credentials can access them auth_user_request->user()->notes.appendNewOnly(&reply.notes); + // remove any private credentials detail which got added. + auth_user_request->user()->notes.remove("token"); Auth::Ntlm::UserRequest *lm_request = dynamic_cast(auth_user_request.getRaw()); assert(lm_request != NULL); @@ -305,8 +307,7 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const Helper::Reply &reply) debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel << "'"); /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); - /* see if this is an existing user with a different proxy_auth - * string */ + /* see if this is an existing user */ AuthUserHashPointer *usernamehash = static_cast(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->userKey())); Auth::User::Pointer local_auth_user = lm_request->user(); while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NTLM ||