---------------------
PatchSet 11280 
Date: 2007/02/27 22:43:52
Author: hno
Branch: SQUID_2_6
Tag: (none) 
Log:
MFC: Fix Negotiate authentication again. got broken in 2.6.STABLE7 (fix for Bug #1792)

The core to this problem was the AUTHENTICATE_STATE_FINISHED and it's odd
and non-obvious use and requirements. This patch eleminates this state by
moving the "user is now authenticated" logics to where the helper reply
is processed like one may expect.

This split design was quite likely a leftover from the NTLM challenge cache
used in Squid-2.5.

Merged changes:
2007/02/27 22:39:50 hno +58 -74   Fix Negotiate authentication again. got broken in 2.6.STABLE7 (fix for Bug #1792)

Members: 
	src/auth/negotiate/auth_negotiate.c:1.7->1.7.2.1 
	src/auth/negotiate/auth_negotiate.h:1.1->1.1.2.1 
	src/auth/ntlm/auth_ntlm.c:1.37->1.37.2.1 
	src/auth/ntlm/auth_ntlm.h:1.10->1.10.2.1 

Index: squid/src/auth/negotiate/auth_negotiate.c
===================================================================
RCS file: /cvsroot/squid/squid/src/auth/negotiate/auth_negotiate.c,v
retrieving revision 1.7
retrieving revision 1.7.2.1
diff -u -r1.7 -r1.7.2.1
--- squid/src/auth/negotiate/auth_negotiate.c	20 Jan 2007 21:13:28 -0000	1.7
+++ squid/src/auth/negotiate/auth_negotiate.c	27 Feb 2007 22:43:52 -0000	1.7.2.1
@@ -1,6 +1,6 @@
 
 /*
- * $Id: auth_negotiate.c,v 1.7 2007/01/20 21:13:28 hno Exp $
+ * $Id: auth_negotiate.c,v 1.7.2.1 2007/02/27 22:43:52 hno Exp $
  *
  * DEBUG: section 29    Negotiate Authenticator
  * AUTHOR: Robert Collins
@@ -85,6 +85,7 @@
 static auth_negotiate_config *negotiateConfig = NULL;
 
 static void authenticateNegotiateReleaseServer(negotiate_request_t * negotiate_request);
+static int authenticateNegotiatecmpUsername(negotiate_user_t * u1, negotiate_user_t * u2);
 /*
  *
  * Private Functions
@@ -104,12 +105,10 @@
 	helperStatefulFree(negotiateauthenticators);
     negotiateauthenticators = NULL;
     if (negotiate_request_pool) {
-	assert(memPoolInUseCount(negotiate_request_pool) == 0);
 	memPoolDestroy(negotiate_request_pool);
 	negotiate_request_pool = NULL;
     }
     if (negotiate_user_pool) {
-	assert(memPoolInUseCount(negotiate_user_pool) == 0);
 	memPoolDestroy(negotiate_user_pool);
 	negotiate_user_pool = NULL;
     }
@@ -215,8 +214,7 @@
 	 * Negotiate when the client sends a second request on an Negotiate
 	 * connection before the authenticate challenge is sent. With
 	 * this patch, the client may fail to authenticate, but squid's
-	 * state will be preserved.  Caveats: this should be a post-parse
-	 * test, but that can wait for the modular parser to be integrated.
+	 * state will be preserved.
 	 */
 	if (negotiateConfig->authenticate && Config.onoff.pipeline_prefetch != 0) {
 	    debug(29, 1) ("pipeline prefetching incompatile with Negotiate authentication. Disabling pipeline_prefetch\n");
@@ -287,7 +285,6 @@
 	return 1;
     case AUTHENTICATE_STATE_FAILED:
 	return -2;
-    case AUTHENTICATE_STATE_FINISHED:	/* do nothing.. */
     case AUTHENTICATE_STATE_DONE:	/* do nothing.. */
 	return 0;
     case AUTHENTICATE_STATE_INITIAL:
@@ -306,8 +303,6 @@
 authenticateNegotiateFixErrorHeader(auth_user_request_t * auth_user_request, HttpReply * rep, http_hdr_type type, request_t * request)
 {
     negotiate_request_t *negotiate_request;
-    if (!request->flags.proxy_keepalive)
-	return;
     if (!negotiateConfig->authenticate)
 	return;
     /* New request, no user details */
@@ -339,7 +334,6 @@
 	safe_free(negotiate_request->server_blob);
 	request->flags.must_keepalive = 1;
 	break;
-    case AUTHENTICATE_STATE_FINISHED:
     case AUTHENTICATE_STATE_DONE:
 	/* Special case when authentication finished, but not allowed by ACL */
 	if (negotiate_request->server_blob) {
@@ -488,6 +482,7 @@
 	auth_user_request->message = xstrdup("Authentication in progress");
 	debug(29, 4) ("authenticateNegotiateHandleReply: Need to challenge the client with a server blob '%s'\n", blob);
     } else if (strncasecmp(reply, "AF ", 3) == 0 && arg != NULL) {
+	auth_user_hash_pointer *usernamehash;
 	/* we're finished, release the helper */
 	if (arg)
 	    *arg++ = '\0';
@@ -498,8 +493,31 @@
 	safe_free(negotiate_request->server_blob);
 	negotiate_request->server_blob = xstrdup(blob);
 	debug(29, 4) ("authenticateNegotiateHandleReply: Successfully validated user via Negotiate. Username '%s'\n", arg);
+	/* this connection is authenticated */
+	debug(29, 4) ("authenticated user %s\n", negotiate_user->username);
+	/* see if this is an existing user with a different proxy_auth 
+	 * string */
+	usernamehash = hash_lookup(proxy_auth_username_cache, negotiate_user->username);
+	if (usernamehash) {
+	    while (usernamehash && (usernamehash->auth_user->auth_type != auth_user->auth_type || authenticateNegotiatecmpUsername(usernamehash->auth_user->scheme_data, negotiate_user) != 0))
+		usernamehash = usernamehash->next;
+	}
+	if (usernamehash) {
+	    /* we can't seamlessly recheck the username due to the 
+	     * challenge nature of the protocol. Just free the 
+	     * temporary auth_user */
+	    authenticateAuthUserMerge(auth_user, usernamehash->auth_user);
+	    auth_user = usernamehash->auth_user;
+	    auth_user_request->auth_user = auth_user;
+	} else {
+	    /* store user in hash's */
+	    authenticateUserNameCacheAdd(auth_user);
+	}
+	/* set these to now because this is either a new login from an 
+	 * existing user or a new user */
+	auth_user->expiretime = current_time.tv_sec;
 	authenticateNegotiateReleaseServer(negotiate_request);
-	negotiate_request->auth_state = AUTHENTICATE_STATE_FINISHED;
+	negotiate_request->auth_state = AUTHENTICATE_STATE_DONE;
     } else if (strncasecmp(reply, "NA ", 3) == 0 && arg != NULL) {
 	if (arg)
 	    *arg++ = '\0';
@@ -573,10 +591,11 @@
     r->data = data;
     r->auth_user_request = auth_user_request;
     authenticateAuthUserRequestLock(r->auth_user_request);
-    if (negotiate_request->auth_state == AUTHENTICATE_STATE_INITIAL)
+    if (negotiate_request->auth_state == AUTHENTICATE_STATE_INITIAL) {
 	snprintf(buf, 8192, "YR %s\n", sent_string);
-    else
+    } else {
 	snprintf(buf, 8192, "KK %s\n", sent_string);
+    }
     negotiate_request->waiting = 1;
     safe_free(negotiate_request->client_blob);
     helperStatefulSubmit(negotiateauthenticators, buf, authenticateNegotiateHandleReply, r, negotiate_request->authserver);
@@ -663,7 +682,6 @@
 authenticateNegotiateAuthenticateUser(auth_user_request_t * auth_user_request, request_t * request, ConnStateData * conn, http_hdr_type type)
 {
     const char *proxy_auth, *blob;
-    auth_user_hash_pointer *usernamehash;
     auth_user_t *auth_user;
     negotiate_request_t *negotiate_request;
     negotiate_user_t *negotiate_user;
@@ -730,33 +748,6 @@
 	negotiate_request->client_blob = xstrdup(blob);
 	return;
 	break;
-    case AUTHENTICATE_STATE_FINISHED:
-	/* this connection is authenticated */
-	debug(29, 4) ("authenticated user %s\n", negotiate_user->username);
-	/* see if this is an existing user with a different proxy_auth 
-	 * string */
-	usernamehash = hash_lookup(proxy_auth_username_cache, negotiate_user->username);
-	if (usernamehash) {
-	    while (usernamehash && (usernamehash->auth_user->auth_type != auth_user->auth_type || authenticateNegotiatecmpUsername(usernamehash->auth_user->scheme_data, negotiate_user) != 0))
-		usernamehash = usernamehash->next;
-	}
-	if (usernamehash) {
-	    /* we can't seamlessly recheck the username due to the 
-	     * challenge nature of the protocol. Just free the 
-	     * temporary auth_user */
-	    authenticateAuthUserMerge(auth_user, usernamehash->auth_user);
-	    auth_user = usernamehash->auth_user;
-	    auth_user_request->auth_user = auth_user;
-	} else {
-	    /* store user in hash's */
-	    authenticateUserNameCacheAdd(auth_user);
-	}
-	/* set these to now because this is either a new login from an 
-	 * existing user or a new user */
-	auth_user->expiretime = current_time.tv_sec;
-	authenticateNegotiateReleaseServer(negotiate_request);
-	negotiate_request->auth_state = AUTHENTICATE_STATE_DONE;
-	return;
     case AUTHENTICATE_STATE_DONE:
 	fatal("authenticateNegotiateAuthenticateUser: unexpect auth state DONE! Report a bug to the squid developers.\n");
 	break;
Index: squid/src/auth/negotiate/auth_negotiate.h
===================================================================
RCS file: /cvsroot/squid/squid/src/auth/negotiate/auth_negotiate.h,v
retrieving revision 1.1
retrieving revision 1.1.2.1
diff -u -r1.1 -r1.1.2.1
--- squid/src/auth/negotiate/auth_negotiate.h	15 May 2006 22:06:49 -0000	1.1
+++ squid/src/auth/negotiate/auth_negotiate.h	27 Feb 2007 22:43:52 -0000	1.1.2.1
@@ -12,7 +12,6 @@
     AUTHENTICATE_STATE_NONE,
     AUTHENTICATE_STATE_INITIAL,
     AUTHENTICATE_STATE_NEGOTIATE,
-    AUTHENTICATE_STATE_FINISHED,
     AUTHENTICATE_STATE_DONE,
     AUTHENTICATE_STATE_FAILED
 } auth_state_t;			/* connection level auth state */
Index: squid/src/auth/ntlm/auth_ntlm.c
===================================================================
RCS file: /cvsroot/squid/squid/src/auth/ntlm/auth_ntlm.c,v
retrieving revision 1.37
retrieving revision 1.37.2.1
diff -u -r1.37 -r1.37.2.1
--- squid/src/auth/ntlm/auth_ntlm.c	20 Jan 2007 21:13:28 -0000	1.37
+++ squid/src/auth/ntlm/auth_ntlm.c	27 Feb 2007 22:43:52 -0000	1.37.2.1
@@ -1,6 +1,6 @@
 
 /*
- * $Id: auth_ntlm.c,v 1.37 2007/01/20 21:13:28 hno Exp $
+ * $Id: auth_ntlm.c,v 1.37.2.1 2007/02/27 22:43:52 hno Exp $
  *
  * DEBUG: section 29    NTLM Authenticator
  * AUTHOR: Robert Collins
@@ -84,6 +84,7 @@
 static auth_ntlm_config *ntlmConfig = NULL;
 
 static void authenticateNTLMReleaseServer(ntlm_request_t * ntlm_request);
+static int authenticateNTLMcmpUsername(ntlm_user_t * u1, ntlm_user_t * u2);
 /*
  *
  * Private Functions
@@ -282,7 +283,6 @@
 	return 1;
     case AUTHENTICATE_STATE_FAILED:
 	return -2;
-    case AUTHENTICATE_STATE_FINISHED:	/* do nothing.. */
     case AUTHENTICATE_STATE_DONE:	/* do nothing.. */
 	return 0;
     case AUTHENTICATE_STATE_INITIAL:
@@ -332,7 +332,6 @@
 	safe_free(ntlm_request->server_blob);
 	request->flags.must_keepalive = 1;
 	break;
-    case AUTHENTICATE_STATE_FINISHED:
     case AUTHENTICATE_STATE_DONE:
 	/* Special case when authentication finished, but not allowed by ACL */
 	debug(29, 9) ("authenticateNTLMFixErrorHeader: Sending type:%d header: 'NTLM'\n", type);
@@ -450,14 +449,38 @@
 	auth_user_request->message = xstrdup("Authentication in progress");
 	debug(29, 4) ("authenticateNTLMHandleReply: Need to challenge the client with a server blob '%s'\n", blob);
     } else if (strncasecmp(reply, "AF ", 3) == 0) {
+	auth_user_hash_pointer *usernamehash;
 	/* we're finished, release the helper */
 	safe_free(ntlm_user->username);
 	ntlm_user->username = xstrdup(blob);
 	safe_free(auth_user_request->message);
 	auth_user_request->message = xstrdup("Login successful");
 	debug(29, 4) ("authenticateNTLMHandleReply: Successfully validated user via NTLM. Username '%s'\n", blob);
+	/* this connection is authenticated */
+	debug(29, 4) ("authenticated user %s\n", ntlm_user->username);
+	/* see if this is an existing user with a different proxy_auth 
+	 * string */
+	usernamehash = hash_lookup(proxy_auth_username_cache, ntlm_user->username);
+	if (usernamehash) {
+	    while (usernamehash && (usernamehash->auth_user->auth_type != auth_user->auth_type || authenticateNTLMcmpUsername(usernamehash->auth_user->scheme_data, ntlm_user) != 0))
+		usernamehash = usernamehash->next;
+	}
+	if (usernamehash) {
+	    /* we can't seamlessly recheck the username due to the 
+	     * challenge nature of the protocol. Just free the 
+	     * temporary auth_user */
+	    authenticateAuthUserMerge(auth_user, usernamehash->auth_user);
+	    auth_user = usernamehash->auth_user;
+	    auth_user_request->auth_user = auth_user;
+	} else {
+	    /* store user in hash's */
+	    authenticateUserNameCacheAdd(auth_user);
+	}
+	/* set these to now because this is either a new login from an 
+	 * existing user or a new user */
+	auth_user->expiretime = current_time.tv_sec;
 	authenticateNTLMReleaseServer(ntlm_request);
-	ntlm_request->auth_state = AUTHENTICATE_STATE_FINISHED;
+	ntlm_request->auth_state = AUTHENTICATE_STATE_DONE;
     } else if (strncasecmp(reply, "NA ", 3) == 0) {
 	safe_free(auth_user_request->message);
 	auth_user_request->message = xstrdup(blob);
@@ -618,7 +641,6 @@
 authenticateNTLMAuthenticateUser(auth_user_request_t * auth_user_request, request_t * request, ConnStateData * conn, http_hdr_type type)
 {
     const char *proxy_auth, *blob;
-    auth_user_hash_pointer *usernamehash;
     auth_user_t *auth_user;
     ntlm_request_t *ntlm_request;
     ntlm_user_t *ntlm_user;
@@ -685,33 +707,6 @@
 	ntlm_request->client_blob = xstrdup(blob);
 	return;
 	break;
-    case AUTHENTICATE_STATE_FINISHED:
-	/* this connection is authenticated */
-	debug(29, 4) ("authenticated user %s\n", ntlm_user->username);
-	/* see if this is an existing user with a different proxy_auth 
-	 * string */
-	usernamehash = hash_lookup(proxy_auth_username_cache, ntlm_user->username);
-	if (usernamehash) {
-	    while (usernamehash && (usernamehash->auth_user->auth_type != auth_user->auth_type || authenticateNTLMcmpUsername(usernamehash->auth_user->scheme_data, ntlm_user) != 0))
-		usernamehash = usernamehash->next;
-	}
-	if (usernamehash) {
-	    /* we can't seamlessly recheck the username due to the 
-	     * challenge nature of the protocol. Just free the 
-	     * temporary auth_user */
-	    authenticateAuthUserMerge(auth_user, usernamehash->auth_user);
-	    auth_user = usernamehash->auth_user;
-	    auth_user_request->auth_user = auth_user;
-	} else {
-	    /* store user in hash's */
-	    authenticateUserNameCacheAdd(auth_user);
-	}
-	/* set these to now because this is either a new login from an 
-	 * existing user or a new user */
-	auth_user->expiretime = current_time.tv_sec;
-	authenticateNTLMReleaseServer(ntlm_request);
-	ntlm_request->auth_state = AUTHENTICATE_STATE_DONE;
-	return;
     case AUTHENTICATE_STATE_DONE:
 	fatal("authenticateNTLMAuthenticateUser: unexpect auth state DONE! Report a bug to the squid developers.\n");
 	break;
Index: squid/src/auth/ntlm/auth_ntlm.h
===================================================================
RCS file: /cvsroot/squid/squid/src/auth/ntlm/auth_ntlm.h,v
retrieving revision 1.10
retrieving revision 1.10.2.1
diff -u -r1.10 -r1.10.2.1
--- squid/src/auth/ntlm/auth_ntlm.h	11 Jun 2006 00:27:36 -0000	1.10
+++ squid/src/auth/ntlm/auth_ntlm.h	27 Feb 2007 22:43:52 -0000	1.10.2.1
@@ -12,7 +12,6 @@
     AUTHENTICATE_STATE_NONE,
     AUTHENTICATE_STATE_INITIAL,
     AUTHENTICATE_STATE_NEGOTIATE,
-    AUTHENTICATE_STATE_FINISHED,
     AUTHENTICATE_STATE_DONE,
     AUTHENTICATE_STATE_FAILED
 } auth_state_t;			/* connection level auth state */