From: Frederic Bourgeois <fredbmail@free.fr>
Date: Mon, 10 Feb 2014 11:08:58 +0000 (-0700)
Subject: Bug 3969: user credentials cache lookup for Digest authentication broken
X-Git-Tag: SQUID_3_5_0_1~380
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d8d76b36aecec5b0c5f0ba93078b8e9829790ae8;p=thirdparty%2Fsquid.git

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 52fc73a0fc..e4430800e1 100644
--- 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 d3cb095c32..aa731adfa3 100644
--- 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 6d00950c9c..4f490236d4 100644
--- 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 1b7c738a3f..c8dc1661ad 100644
--- 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);