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 7efbd0494a529cdb5bd0d1df33566c8cf098b176..703d47bfb0beb9d42e008970539dfecae4893bdd 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 b74641c7bf181eecff88843d8ed88dbe38d07175..bb2d8406568c5ada9f9872016a362250389251a6 100644 (file)
--- a/src/auth/digest/auth_digest.cc
+++ b/src/auth/digest/auth_digest.cc
@@ -476,25 +476,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()
 {
@@ -729,7 +710,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;
@@ -1076,7 +1057,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);