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 3b5fe2dfdb739dab4af4cafa35ab7b99e1285ff2..efb764c506cb8d30687c6b7c491252e0f1a6b610 100644 (file)
--- 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 88c06716b486b40fd2212088d85c750d0d744a54..d08355d932b89550082ad3933224c6f2c28c784a 100644 (file)
--- 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 237b09019484f0250eaa22e3857818eead2f6c93..59655df62fdcd278fb4a6c0fe7a634450d31584a 100644 (file)
--- 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();