From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 14 Feb 2013 05:42:57 +0000 (-0700)
Subject: Fix Negotiate auth NA response handling
X-Git-Tag: SQUID_3_4_0_1~275
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=723fd4c11ce252c602e4e362ea37bbadb8ee9181;p=thirdparty%2Fsquid.git

Fix Negotiate auth NA response handling

Handle the blurb presented on NA responses as an error message.

Negotiate auth was also treating token as a mandatory field, although it is
optional along with an optional message.
---

diff --git a/src/HelperReply.cc b/src/HelperReply.cc
index 847cbf3646..379cf2b8c0 100644
--- a/src/HelperReply.cc
+++ b/src/HelperReply.cc
@@ -31,6 +31,7 @@ HelperReply::parse(char *buf, size_t len)
     }
 
     char *p = buf;
+    bool sawNA = false;
 
     // optimization: do not consider parsing result code if the response is short.
     // URL-rewriter may return relative URLs or empty response for a large portion
@@ -100,6 +101,7 @@ HelperReply::parse(char *buf, size_t len)
             // NTLM fail-closed ERR response
             result = HelperReply::Error;
             p+=3;
+            sawNA=true;
         }
 
         for (; xisspace(*p); ++p); // skip whitespace
@@ -112,10 +114,12 @@ HelperReply::parse(char *buf, size_t len)
     // NULL-terminate so the helper callback handlers do not buffer-overrun
     other_.terminate();
 
-    parseResponseKeys();
+    // Hack for backward-compatibility: Do not parse for kv-pairs on NA response
+    if (!sawNA)
+        parseResponseKeys();
 
-    // Hack for backward-compatibility: BH used to be a text message...
-    if (other().hasContent() && result == HelperReply::BrokenHelper) {
+    // Hack for backward-compatibility: BH and NA used to be a text message...
+    if (other().hasContent() && (sawNA || result == HelperReply::BrokenHelper)) {
         notes.add("message",other().content());
         modifiableOther().clean();
     }
diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc
index 8e6e95322c..44796d035b 100644
--- a/src/auth/negotiate/UserRequest.cc
+++ b/src/auth/negotiate/UserRequest.cc
@@ -329,17 +329,16 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply)
     case HelperReply::Error: {
         Note::Pointer messageNote = reply.notes.find("message");
         Note::Pointer tokenNote = reply.notes.find("token");
-        if (tokenNote == NULL) {
-            /* protocol error */
-            fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content());
-            break;
-        }
 
         /* authentication failure (wrong password, etc.) */
-        auth_user_request->denyMessage(messageNote->firstValue());
+        if (messageNote != NULL) {
+            auth_user_request->denyMessage(messageNote->firstValue());
+        else
+            auth_user_request->denyMessage("Negotiate Authentication denied with no reason given");
         auth_user_request->user()->credentials(Auth::Failed);
         safe_free(lm_request->server_blob);
-        lm_request->server_blob = xstrdup(tokenNote->firstValue());
+        if (tokenNote != NULL)
+            lm_request->server_blob = xstrdup(tokenNote->firstValue());
         lm_request->releaseAuthServer();
         debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'");
     }