From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sun, 17 Apr 2011 03:35:52 +0000 (-0600)
Subject: Fixes NTLM and Negotiate auth assertion "RefCountCount() == 2"
X-Git-Tag: take06~6^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=58e9434217db9c8097319b618fba412319245e45;p=thirdparty%2Fsquid.git

Fixes NTLM and Negotiate auth assertion "RefCountCount() == 2"

It turns out the replay cache and invalid RefCount cases this was added to
protect againt are not present anyway. After some minor cleanup to remove
double-calls in Negotiate things appear to run nicely.

NOTE:
 There is still a risk that these problem cases may in future occur, but
 meanwhile we need NTLM and Negotiate to be usable and efficient.
 The bugs resulting from those can be dealt with if/when they do occur.
---

diff --git a/src/auth/User.cc b/src/auth/User.cc
index 3b5fe2dfdb..efb764c506 100644
--- a/src/auth/User.cc
+++ b/src/auth/User.cc
@@ -86,19 +86,12 @@ Auth::User::credentials(CredentialState newCreds)
  * two users _can_ be merged without invalidating all the request
  * scheme data. The scheme is also responsible for merging any user
  * related scheme data itself.
+ * The caller is responsible for altering all refcount pointers to
+ * the 'from' object. They are invalid once this method is complete.
  */
 void
 Auth::User::absorb(Auth::User::Pointer from)
 {
-
-    /* RefCount children CANNOT be merged like this. The external Auth::User::Pointer's cannot be changed. */
-
-    /* check that we only have the two references:
-     * 1) our function scope
-     * 2) the parsing function scope)
-     */
-    assert(from->RefCountCount() == 2);
-
     /*
      * XXX Incomplete: it should merge in hash references too and ask the module to merge in scheme data
      *  dlink_list proxy_auth_list;
diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc
index 88c06716b4..d08355d932 100644
--- a/src/auth/negotiate/UserRequest.cc
+++ b/src/auth/negotiate/UserRequest.cc
@@ -339,8 +339,6 @@ AuthNegotiateUserRequest::HandleReply(void *data, void *lastserver, char *reply)
         safe_free(negotiate_request->server_blob);
         negotiate_request->server_blob = xstrdup(blob);
         negotiate_request->releaseAuthServer();
-        auth_user_request->user()->credentials(Auth::Ok);
-        debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << blob << "'");
 
         /* connection is authenticated */
         debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username());
@@ -357,9 +355,9 @@ AuthNegotiateUserRequest::HandleReply(void *data, void *lastserver, char *reply)
              * Just free the temporary auth_user after merging as
              * much of it new state into the existing one as possible */
             usernamehash->user()->absorb(local_auth_user);
-            local_auth_user = usernamehash->user();
             /* from here on we are working with the original cached credentials. */
-            negotiate_request->_auth_user = local_auth_user;
+            local_auth_user = usernamehash->user();
+            auth_user_request->user(local_auth_user);
         } else {
             /* store user in hash's */
             local_auth_user->addToNameCache();
@@ -367,8 +365,8 @@ AuthNegotiateUserRequest::HandleReply(void *data, void *lastserver, char *reply)
         /* set these to now because this is either a new login from an
          * existing user or a new user */
         local_auth_user->expiretime = current_time.tv_sec;
-        negotiate_request->releaseAuthServer();
-        negotiate_request->user()->credentials(Auth::Ok);
+        auth_user_request->user()->credentials(Auth::Ok);
+        debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << blob << "'");
 
     } else if (strncasecmp(reply, "NA ", 3) == 0 && arg != NULL) {
         /* authentication failure (wrong password, etc.) */
diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc
index 237b090194..59655df62f 100644
--- a/src/auth/ntlm/UserRequest.cc
+++ b/src/auth/ntlm/UserRequest.cc
@@ -313,10 +313,11 @@ AuthNTLMUserRequest::HandleReply(void *data, void *lastserver, char *reply)
         if (usernamehash) {
             /* we can't seamlessly recheck the username due to the
              * challenge-response nature of the protocol.
-             * Just free the temporary auth_user */
+             * Just free the temporary auth_user after merging as
+             * much of it new state into the existing one as possible */
             usernamehash->user()->absorb(local_auth_user);
             local_auth_user = usernamehash->user();
-            ntlm_request->_auth_user = local_auth_user;
+            auth_user_request->user(local_auth_user);
         } else {
             /* store user in hash's */
             local_auth_user->addToNameCache();