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 52fc73a0fcfa94519b5177e8762cd7862e542bdd..e4430800e12aad3e5389a8068c00652301bb6f45 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 "cache_cf.h"
 #include "ConfigParser.h"
@@ -129,3 +130,22 @@ Auth::Config::done()
     keyExtras = NULL;
     keyExtrasLine.clean();
 }
+
+Auth::User::Pointer
+Auth::Config::findUserInCache(const char *nameKey, Auth::Type type)
+{
+    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 == type) &&
+                    !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 d3cb095c329d09b9287b33a3b6906c5c942c2acc..aa731adfa3e5d5eea410632312de8b59ac09ba2e 100644 (file)
--- a/src/auth/Config.h
+++ b/src/auth/Config.h
@@ -128,6 +128,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 6d00950c9c82887c740028b2ef5adcb6890d890c..4f490236d4f2e5c570e46fbf71686a7e0def538a 100644 (file)
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -197,25 +197,6 @@ authenticateBasicStats(StoreEntry * sentry)
     helperStats(sentry, basicauthenticators, "Basic Authenticator Statistics");
 }
 
-static Auth::User::Pointer
-authBasicAuthUserFindUsername(const char *userkey)
-{
-    AuthUserHashPointer *usernamehash;
-    debugs(29, 9, "Looking for user '" << userkey << "'");
-
-    if (userkey && (usernamehash = static_cast<AuthUserHashPointer *>(hash_lookup(proxy_auth_username_cache, userkey)))) {
-        while (usernamehash) {
-            if ((usernamehash->user()->auth_type == Auth::AUTH_BASIC) &&
-                    !strcmp(userkey, (char const *)usernamehash->key))
-                return usernamehash->user();
-
-            usernamehash = static_cast<AuthUserHashPointer *>(usernamehash->next);
-        }
-    }
-
-    return NULL;
-}
-
 char *
 Auth::Basic::Config::decodeCleartext(const char *httpAuthHeader)
 {
@@ -311,7 +292,7 @@ Auth::Basic::Config::decode(char const *proxy_auth, const char *aRequestRealm)
     /* now lookup and see if we have a matching auth_user structure in memory. */
     Auth::User::Pointer auth_user;
 
-    if ((auth_user = authBasicAuthUserFindUsername(lb->userKey())) == NULL) {
+    if ((auth_user = findUserInCache(lb->userKey(), 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 1b7c738a3f7b68645aefaeecea7d934e93532df5..c8dc1661ad93f019c0176f0e3a65409753881def 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 *userkey)
-{
-    AuthUserHashPointer *usernamehash;
-    debugs(29, 9, "Looking for user '" << userkey << "'");
-
-    if ((usernamehash = static_cast < AuthUserHashPointer * >(hash_lookup(proxy_auth_username_cache, userkey)))) {
-        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()
 {
@@ -730,7 +711,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;
@@ -1078,7 +1059,7 @@ Auth::Digest::Config::decode(char const *proxy_auth, const char *aRequestRealm)
     Auth::User::Pointer auth_user;
 
     SBuf key = Auth::User::BuildUserKey(username, aRequestRealm);
-    if (key.isEmpty() || (auth_user = authDigestUserFindUsername(key.c_str())) == NULL) {
+    if (key.isEmpty() || (auth_user = findUserInCache(key.c_str(), 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, aRequestRealm);