Bug 3969: user credentials cache lookup for Digest authentication broken

Changes to the username credentials cache were made in Basic auth but
the matching changes were not duplicated to Digest auth. Since the
lookup is identical move it to generic Auth::Config.

Also fixes assertion auth_digest.cc:759:
    "(nonce->user == NULL) || (nonce->user == user)"

diff --git a/src/auth/Config.cc b/src/auth/Config.cc
index d396b42704a7687e7ea3944e8954a9b690d98ba8..040ee205500fd384dc75cb7a354ee652f078f8d2 100644 (file)
--- a/src/auth/Config.cc
+++ b/src/auth/Config.cc
@@ -32,6 +32,7 @@
 
 #include "squid.h"
 #include "auth/Config.h"
+#include "auth/Gadgets.h"
 #include "auth/UserRequest.h"
 #include "Debug.h"
 #include "globals.h"
@@ -76,3 +77,22 @@ Auth::Config::Find(const char *proxy_auth)
 void
 Auth::Config::registerWithCacheManager(void)
 {}
+
+Auth::User::Pointer
+Auth::Config::findUserInCache(const char *nameKey, Auth::Type authType)
+{
+    AuthUserHashPointer *usernamehash;
+    debugs(29, 9, "Looking for user '" << nameKey << "'");
+
+    if (nameKey && (usernamehash = static_cast<AuthUserHashPointer *>(hash_lookup(proxy_auth_username_cache, nameKey)))) {
+        while (usernamehash) {
+            if ((usernamehash->user()->auth_type == authType) &&
+                    !strcmp(nameKey, (char const *)usernamehash->key))
+                return usernamehash->user();
+
+            usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
+        }
+    }
+
+    return NULL;
+}

diff --git a/src/auth/Config.h b/src/auth/Config.h
index ec29ed06dc25919de3706eb38924622139628ef9..315bf6d521c9f72af59d9dfda00cd08d9934521d 100644 (file)
--- a/src/auth/Config.h
+++ b/src/auth/Config.h
@@ -122,6 +122,9 @@ public:
     /** add headers as needed when challenging for auth */
     virtual void fixHeader(UserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *) = 0;
 
+    /// Find any existing user credentials in the authentication cache by name and type.
+    virtual Auth::User::Pointer findUserInCache(const char *nameKey, Auth::Type type);
+
     /** prepare to handle requests */
     virtual void init(Config *) = 0;
 

diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 71002efeab24d379d7840f7249c30a16aea28c55..e592ae67f9673b34af3012b389115faff181277e 100644 (file)
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -195,25 +195,6 @@ authenticateBasicStats(StoreEntry * sentry)
     helperStats(sentry, basicauthenticators, "Basic Authenticator Statistics");
 }
 
-static Auth::User::Pointer
-authBasicAuthUserFindUsername(const char *username)
-{
-    AuthUserHashPointer *usernamehash;
-    debugs(29, 9, HERE << "Looking for user '" << username << "'");
-
-    if (username && (usernamehash = static_cast<AuthUserHashPointer *>(hash_lookup(proxy_auth_username_cache, username)))) {
-        while (usernamehash) {
-            if ((usernamehash->user()->auth_type == Auth::AUTH_BASIC) &&
-                    !strcmp(username, (char const *)usernamehash->key))
-                return usernamehash->user();
-
-            usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
-        }
-    }
-
-    return NULL;
-}
-
 char *
 Auth::Basic::Config::decodeCleartext(const char *httpAuthHeader)
 {
@@ -310,7 +291,7 @@ Auth::Basic::Config::decode(char const *proxy_auth)
     /* now lookup and see if we have a matching auth_user structure in memory. */
     Auth::User::Pointer auth_user;
 
-    if ((auth_user = authBasicAuthUserFindUsername(lb->username())) == NULL) {
+    if ((auth_user = findUserInCache(lb->username(), Auth::AUTH_BASIC)) == NULL) {
         /* the user doesn't exist in the username cache yet */
         /* save the credentials */
         debugs(29, 9, HERE << "Creating new user '" << lb->username() << "'");

diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc
index 669bde7b4127daa918e9f15486a5908bd4c54df8..60abc1f83120b106e08b6ff0c39690eeec17e8e5 100644 (file)
--- a/src/auth/digest/auth_digest.cc
+++ b/src/auth/digest/auth_digest.cc
@@ -475,25 +475,6 @@ authDigestNoncePurge(digest_nonce_h * nonce)
     authDigestNonceUnlink(nonce);
 }
 
-/* USER related functions */
-static Auth::User::Pointer
-authDigestUserFindUsername(const char *username)
-{
-    AuthUserHashPointer *usernamehash;
-    debugs(29, 9, HERE << "Looking for user '" << username << "'");
-
-    if (username && (usernamehash = static_cast < AuthUserHashPointer * >(hash_lookup(proxy_auth_username_cache, username)))) {
-        while ((usernamehash->user()->auth_type != Auth::AUTH_DIGEST) && (usernamehash->next))
-            usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
-
-        if (usernamehash->user()->auth_type == Auth::AUTH_DIGEST) {
-            return usernamehash->user();
-        }
-    }
-
-    return NULL;
-}
-
 void
 Auth::Digest::Config::rotateHelpers()
 {
@@ -728,7 +709,7 @@ authDigestUserLinkNonce(Auth::Digest::User * user, digest_nonce_h * nonce)
 {
     dlink_node *node;
 
-    if (!user || !nonce)
+    if (!user || !nonce || !nonce->user)
         return;
 
     Auth::Digest::User *digest_user = user;
@@ -1075,7 +1056,7 @@ Auth::Digest::Config::decode(char const *proxy_auth)
 
     Auth::User::Pointer auth_user;
 
-    if ((auth_user = authDigestUserFindUsername(username)) == NULL) {
+    if ((auth_user = findUserInCache(username, Auth::AUTH_DIGEST)) == NULL) {
         /* the user doesn't exist in the username cache yet */
         debugs(29, 9, HERE << "Creating new digest user '" << username << "'");
         digest_user = new Auth::Digest::User(this);