Author: Christos Tsantilas <chtsanti@users.sourceforge.net>

Bug 2206: no Proxy-Authenticate header in 407 responses

Proper handling of Hop-By-Hop headers according to RFC2616
(drop them!) special handling given to Auth headers on pass-thru cases.

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index a0bab13f85c9af70625b62f1e9aa1f7a585dd209..96bada8eddb7fa3078c2988c0d0913004dddabeb 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -236,10 +236,11 @@ static http_hdr_type RequestHeadersArr[] =
         HDR_USER_AGENT, HDR_X_FORWARDED_FOR, HDR_SURROGATE_CAPABILITY
     };
 
+static HttpHeaderMask HopByHopHeadersMask;
 static http_hdr_type HopByHopHeadersArr[] =
     {
-        HDR_CONNECTION, HDR_KEEP_ALIVE, HDR_PROXY_AUTHENTICATE, HDR_PROXY_AUTHORIZATION,
-        HDR_TE, HDR_TRAILERS, HDR_TRANSFER_ENCODING, HDR_UPGRADE
+        HDR_CONNECTION, HDR_KEEP_ALIVE, /*HDR_PROXY_AUTHENTICATE,*/ HDR_PROXY_AUTHORIZATION,
+        HDR_TE, HDR_TRAILERS, HDR_TRANSFER_ENCODING, HDR_UPGRADE, HDR_PROXY_CONNECTION
     };
 
 /* header accounting */
@@ -304,6 +305,8 @@ httpHeaderInitModule(void)
 
     httpHeaderCalcMask(&RequestHeadersMask, EntityHeadersArr, countof(EntityHeadersArr));
 
+    httpHeaderCalcMask(&HopByHopHeadersMask, HopByHopHeadersArr, countof(HopByHopHeadersArr));
+
     /* init header stats */
     assert(HttpHeaderStatCount == hoReply + 1);
 
@@ -1773,11 +1776,16 @@ HttpHeader::removeHopByHopEntries()
 {
     removeConnectionHeaderEntries();
     
-    int count = countof(HopByHopHeadersArr);
-    
-    for (int i=0; i<count; i++)
-        delById(HopByHopHeadersArr[i]);    
-    
+    const HttpHeaderEntry *e;
+    HttpHeaderPos pos = HttpHeaderInitPos;
+    int headers_deleted = 0;
+    while ((e = getEntry(&pos))) {
+       int id = e->id;
+       if(CBIT_TEST(HopByHopHeadersMask, id)){
+           delAt(pos, headers_deleted);
+           CBIT_CLR(mask, id);
+       }
+    }
 }
 
 void

diff --git a/src/ICAP/ICAPModXact.cc b/src/ICAP/ICAPModXact.cc
index 8da4103401d14dae546cb7b50ee844b442d2e930..96cccea8751ce60ef601dcba5780ab8bcfae9783 100644 (file)
--- a/src/ICAP/ICAPModXact.cc
+++ b/src/ICAP/ICAPModXact.cc
@@ -1178,6 +1178,7 @@ void ICAPModXact::encapsulateHead(MemBuf &icapBuf, const char *section, MemBuf &
     // end cloning
         
     // remove all hop-by-hop headers from the clone
+    headClone->header.delById(HDR_PROXY_AUTHENTICATE);
     headClone->header.removeHopByHopEntries();
 
     // pack polished HTTP header

diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc
index bef4388dce73f0a1849a4a4efc7262d8678ed023..c9956f2ddd43e9dd357818fceb042940a355514c 100644 (file)
--- a/src/auth/ntlm/auth_ntlm.cc
+++ b/src/auth/ntlm/auth_ntlm.cc
@@ -279,7 +279,6 @@ AuthNTLMConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, ht
 
         if (!keep_alive) {
             /* drop the connection */
-            rep->header.delByName("keep-alive");
             request->flags.proxy_keepalive = 0;
         }
     } else {
@@ -292,7 +291,6 @@ AuthNTLMConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, ht
         case AUTHENTICATE_STATE_FAILED:
             /* here it makes sense to drop the connection, as auth is
              * tied to it, even if MAYBE the client could handle it - Kinkie */
-            rep->header.delByName("keep-alive");
             request->flags.proxy_keepalive = 0;
             /* fall through */
 

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 5a1cb7d61e4b0973f602263c3003e51ac6d94935..c97354cf349875ed391c6a9b4159b03ded23b603 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1176,19 +1176,14 @@ clientReplyContext::buildReplyHeader()
     hdr->delById(HDR_ETAG);
 #endif
 
-    // TODO: Should ESIInclude.cc that calls removeConnectionHeaderEntries
-    // also delete HDR_PROXY_CONNECTION and HDR_KEEP_ALIVE like we do below?
-
-    // XXX: Should HDR_PROXY_CONNECTION by studied instead of HDR_CONNECTION?
-    // httpHeaderHasConnDir does that but we do not. Is this is a bug?
-    hdr->delById(HDR_PROXY_CONNECTION);
-    /* here: Keep-Alive is a field-name, not a connection directive! */
-    hdr->delById(HDR_KEEP_ALIVE);
-    /* remove Set-Cookie if a hit */
-
     if (is_hit)
         hdr->delById(HDR_SET_COOKIE);
 
+    // if there is not configured a peer proxy with login=PASS option enabled 
+    // remove the Proxy-Authenticate header
+    if ( !(request->peer_login && strcmp(request->peer_login,"PASS") ==0))
+       reply->header.delById(HDR_PROXY_AUTHENTICATE);
+
     reply->header.removeHopByHopEntries();
 
     //    if (request->range)
@@ -1247,8 +1242,9 @@ clientReplyContext::buildReplyHeader()
     }
 
     /* Filter unproxyable authentication types */
+
     if (http->logType != LOG_TCP_DENIED &&
-            (hdr->has(HDR_WWW_AUTHENTICATE) || hdr->has(HDR_PROXY_AUTHENTICATE))) {
+           (hdr->has(HDR_WWW_AUTHENTICATE) || hdr->has(HDR_PROXY_AUTHENTICATE))) {
         HttpHeaderPos pos = HttpHeaderInitPos;
         HttpHeaderEntry *e;
 
@@ -1270,7 +1266,19 @@ clientReplyContext::buildReplyHeader()
     }
 
     /* Handle authentication headers */
-    if (request->auth_user_request)
+    if(http->logType == LOG_TCP_DENIED &&
+       ( reply->sline.status == HTTP_PROXY_AUTHENTICATION_REQUIRED || 
+        reply->sline.status == HTTP_UNAUTHORIZED) 
+       ){
+       /* Add authentication header */
+       /*! \todo alter errorstate to be accel on|off aware. The 0 on the next line
+        * depends on authenticate behaviour: all schemes to date send no extra
+        * data on 407/401 responses, and do not check the accel state on 401/407
+        * responses
+        */
+       authenticateFixHeader(reply, request->auth_user_request, request, 0, 1);
+    }
+    else if (request->auth_user_request)
         authenticateFixHeader(reply, request->auth_user_request, request,
                               http->flags.accel, 0);
 

diff --git a/src/errorpage.cc b/src/errorpage.cc
index e5fd72a5ca64fd9d8c58e1eb4878ac8d2067470f..149b417d4933e47a7b4e8997653fe10a7d03949f 100644 (file)
--- a/src/errorpage.cc
+++ b/src/errorpage.cc
@@ -376,13 +376,6 @@ errorAppendEntry(StoreEntry * entry, ErrorState * err)
     entry->lock();
     entry->buffer();
     rep = errorBuildReply(err);
-    /* Add authentication header */
-    /*! \todo alter errorstate to be accel on|off aware. The 0 on the next line
-     * depends on authenticate behaviour: all schemes to date send no extra
-     * data on 407/401 responses, and do not check the accel state on 401/407
-     * responses 
-     */
-    authenticateFixHeader(rep, err->auth_user_request, err->request, 0, 1);
     entry->replaceHttpReply(rep);
     EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
     entry->flush();