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 f1f08bd333be2592f5afb80009db663b4392d9ab..19ea3115de3b0d0a5969c8cd99ef73066bc82ea7 100644 (file)
--- 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 b85678c49fd90c47409ea3cc209e8065bc88caea..f3d8193a5dc48816736c8feb3e9b8e687b89300b 100644 (file)
--- 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 ea76a5682076fadba01e16609d1066c2a763e960..4ac86dcb866b2d87d1c617b761c1de047e2df613 100644 (file)
--- 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::Negotiate::UserRequest *>(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<AuthUserHashPointer *>(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 7dafc6db2a7e5bba732922c48f35ec0db8160a72..dbbd47927375944229f8cd7094a45da3775f8244 100644 (file)
--- 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 0052cf1f55b2dcdf3f09ee396eacf552b4078836..9dbba526196fbeb07af69854e4fd35460133086c 100644 (file)
--- 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::Ntlm::UserRequest *>(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<AuthUserHashPointer *>(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 ||