Bug #874: rough port of the http_reply_header_size functionality

diff --git a/src/HttpReply.cc b/src/HttpReply.cc
index 8cce8fff76ec1f384915bebd59b669a5effaf2a1..ad557ca99542bf45973467aeae3b8c2bec7dd01b 100644 (file)
--- a/src/HttpReply.cc
+++ b/src/HttpReply.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpReply.cc,v 1.69 2004/12/20 16:30:32 robertc Exp $
+ * $Id: HttpReply.cc,v 1.70 2004/12/21 17:52:53 robertc Exp $
  *
  * DEBUG: section 58    HTTP Reply (Response)
  * AUTHOR: Alex Rousskov
@@ -151,16 +151,17 @@ httpReplyParse(HttpReply * rep, const char *buf, ssize_t end)
      * becuase somebody may feed a non NULL-terminated buffer to
      * us.
      */
-    char *headers = (char *)memAllocate(MEM_4K_BUF);
+    MemBuf mb = MemBufNull;
     int success;
-    size_t s = XMIN(end + 1, (ssize_t)4096);
     /* reset current state, because we are not used in incremental fashion */
     httpReplyReset(rep);
     /* put a string terminator.  s is how many bytes to touch in
      * 'buf' including the terminating NULL. */
-    xstrncpy(headers, buf, s);
-    success = httpReplyParseStep(rep, headers, 0);
-    memFree(headers, MEM_4K_BUF);
+    memBufDefInit(&mb);
+    memBufAppend(&mb, buf, end);
+    memBufAppend(&mb, "\0", 1);
+    success = httpReplyParseStep(rep, mb.buf, 0);
+    memBufClean(&mb);
     return success == 1;
 }
 

diff --git a/src/MemBuf.cc b/src/MemBuf.cc
index 72065ffdba0acbdb7aa4565a966cf702ff4b812b..e806e1a1b50d1d1f17865eb9944c90223429b552 100644 (file)
--- a/src/MemBuf.cc
+++ b/src/MemBuf.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: MemBuf.cc,v 1.36 2004/06/06 15:09:55 hno Exp $
+ * $Id: MemBuf.cc,v 1.37 2004/12/21 17:52:53 robertc Exp $
  *
  * DEBUG: section 59    auto-growing Memory Buffer with printf
  * AUTHOR: Alex Rousskov
@@ -161,7 +161,7 @@ memBufClean(MemBuf * mb)
 
     memFreeBuf(mb->capacity, mb->buf);
     mb->buf = NULL;
-    mb->size = mb->capacity = 0;
+    mb->size = mb->capacity = mb->max_capacity = 0;
 }
 
 /* cleans the buffer without changing its capacity

diff --git a/src/cf.data.pre b/src/cf.data.pre
index bd0f6261e330d39129791f3463c0eb15f54c0daa..9c89c36ce3d60855fdabc6a0f524c4fb27c3b5bc 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1,6 +1,6 @@
 
 #
-# $Id: cf.data.pre,v 1.366 2004/12/20 16:30:34 robertc Exp $
+# $Id: cf.data.pre,v 1.367 2004/12/21 17:52:53 robertc Exp $
 #
 #
 # SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -2789,6 +2789,19 @@ DOC_START
        matching line.
 DOC_END
 
+NAME: reply_header_max_size
+COMMENT: (KB)
+TYPE: b_size_t
+DEFAULT: 20 KB
+LOC: Config.maxReplyHeaderSize
+DOC_START
+       This specifies the maximum size for HTTP headers in a reply.
+       Reply headers are usually relatively small (about 512 bytes).
+       Placing a limit on the reply header size will catch certain
+       bugs (for example with persistent connections) and possibly
+       buffer-overflow or denial-of-service attacks.
+DOC_END
+
 NAME: reply_body_max_size
 COMMENT: size [acl acl...]
 TYPE: acl_b_size_t

diff --git a/src/enums.h b/src/enums.h
index 0ae29b85ee665518d2777d66ae5e2f5e78275918..3bb386969f74ce27e7718e5d810d3e8e82a4424f 100644 (file)
--- a/src/enums.h
+++ b/src/enums.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: enums.h,v 1.238 2004/12/20 16:30:35 robertc Exp $
+ * $Id: enums.h,v 1.239 2004/12/21 17:52:53 robertc Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -444,7 +444,8 @@ typedef enum {
     HTTP_GATEWAY_TIMEOUT = 504,
     HTTP_HTTP_VERSION_NOT_SUPPORTED = 505,
     HTTP_INSUFFICIENT_STORAGE = 507,   /* RFC2518 section 10.6 */
-    HTTP_INVALID_HEADER = 600  /* Squid header parsing error */
+    HTTP_INVALID_HEADER = 600, /* Squid header parsing error */
+    HTTP_HEADER_TOO_LARGE = 601        /* Header too large to process */
 } http_status;
 
 /*

diff --git a/src/http.cc b/src/http.cc
index 273fa26f660ed3a144874c5d0f5db1fa79932f34..3b817e51573b068dfcced3c1e266713e8398401e 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.cc,v 1.440 2004/12/20 16:30:35 robertc Exp $
+ * $Id: http.cc,v 1.441 2004/12/21 17:52:53 robertc Exp $
  *
  * DEBUG: section 11    Hypertext Transfer Protocol (HTTP)
  * AUTHOR: Harvest Derived
@@ -81,9 +81,8 @@ httpStateFree(int fd, void *data)
 
     storeUnlockObject(httpState->entry);
 
-    if (httpState->reply_hdr) {
-        memFree(httpState->reply_hdr, MEM_8K_BUF);
-        httpState->reply_hdr = NULL;
+    if (!memBufIsNull(&httpState->reply_hdr)) {
+        memBufClean(&httpState->reply_hdr);
     }
 
     requestUnlink(httpState->request);
@@ -523,6 +522,8 @@ HttpStateData::cacheableReply()
     case HTTP_PROXY_AUTHENTICATION_REQUIRED:
 
     case HTTP_INVALID_HEADER:  /* Squid header parsing error */
+
+    case HTTP_HEADER_TOO_LARGE:
         return 0;
 
     default:                   /* Unknown status code */
@@ -614,93 +615,105 @@ httpMakeVaryMark(HttpRequest * request, HttpReply const * reply)
     return vstr.buf();
 }
 
-/* rewrite this later using new interfaces @?@ */
+void
+HttpStateData::failReply(HttpReply *reply, http_status const & status)
+{
+    reply->sline.version = HttpVersion(1, 0);
+    reply->sline.status = status;
+    storeEntryReplaceObject (entry, reply);
+
+    if (eof == 1) {
+        fwdComplete(fwd);
+        comm_close(fd);
+    }
+}
+
+/* rewrite this later using new interfaces @?@
+ * This creates the error page itself.. its likely
+ * that the forward ported reply header max size patch
+ * generates non http conformant error pages - in which
+ * case the errors where should be 'BAD_GATEWAY' etc
+ */
 void
 HttpStateData::processReplyHeader(const char *buf, int size)
 {
-    char *t = NULL;
-    int room;
     size_t hdr_len;
+    size_t hdr_size = headersEnd(buf, size);
     /* Creates a blank header. If this routine is made incremental, this will
      * not do 
      */
     HttpReply *reply = httpReplyCreate();
-    Ctx ctx;
     debug(11, 3) ("httpProcessReplyHeader: key '%s'\n",
                   entry->getMD5Text());
 
-    if (reply_hdr == NULL)
-        reply_hdr = (char *)memAllocate(MEM_8K_BUF);
+    if (memBufIsNull(&reply_hdr))
+        memBufDefInit(&reply_hdr);
 
     assert(reply_hdr_state == 0);
 
-    hdr_len = reply_hdr_size;
+    if (hdr_size)
+        memBufAppend(&reply_hdr, buf, hdr_size);
+    else
+        memBufAppend(&reply_hdr, buf, size);
 
-    room = 8191 - hdr_len;
+    hdr_len = reply_hdr.size;
 
-    xmemcpy(reply_hdr + hdr_len, buf, room < size ? room : size);
+    if (hdr_len > 4 && strncmp(reply_hdr.buf, "HTTP/", 5)) {
+        debugs(11, 3, "httpProcessReplyHeader: Non-HTTP-compliant header: '" <<  reply_hdr.buf << "'\n");
+        reply_hdr_state += 2;
+        memBufClean(&reply_hdr);
+        failReply (reply, HTTP_INVALID_HEADER);
+        return;
+    }
 
-    hdr_len += room < size ? room : size;
+    if (hdr_size != hdr_len)
+        hdr_size = headersEnd(reply_hdr.buf, hdr_len);
 
-    reply_hdr[hdr_len] = '\0';
+    if (hdr_size)
+        hdr_len = hdr_size;
 
-    reply_hdr_size = hdr_len;
+    if (hdr_len > Config.maxReplyHeaderSize) {
+        debugs(11, 1, "httpProcessReplyHeader: Too large reply header\n");
 
-    if (hdr_len > 4 && strncmp(reply_hdr, "HTTP/", 5)) {
-        debug(11, 3) ("httpProcessReplyHeader: Non-HTTP-compliant header: '%s'\n", reply_hdr);
-        reply_hdr_state += 2;
-        reply->sline.version = HttpVersion(1, 0);
-        reply->sline.status = HTTP_INVALID_HEADER;
-        storeEntryReplaceObject (entry, reply);
+        if (!memBufIsNull(&reply_hdr))
+            memBufClean(&reply_hdr);
 
-        if (eof == 1) {
-            fwdComplete(fwd);
-            comm_close(fd);
-        }
+        failReply (reply, HTTP_HEADER_TOO_LARGE);
 
         return;
     }
 
-    t = reply_hdr + hdr_len;
     /* headers can be incomplete only if object still arriving */
-
-    if (!eof) {
-        size_t k = headersEnd(reply_hdr, 8192);
-
-        if (0 == k) {
-            if (eof == 1) {
-                fwdComplete(fwd);
-                comm_close(fd);
-            }
-
+    if (!hdr_size) {
+        if (eof)
+            hdr_size = hdr_len;
+        else
             return;            /* headers not complete */
-        }
-
-        t = reply_hdr + k;
     }
 
-    *t = '\0';
+    /* Cut away any excess body data (only needed for debug?) */
+    memBufAppend(&reply_hdr, "\0", 1);
+
+    reply_hdr.buf[hdr_size] = '\0';
+
     reply_hdr_state++;
+
     assert(reply_hdr_state == 1);
-    ctx = ctx_enter(entry->mem_obj->url);
+
+    Ctx ctx = ctx_enter(entry->mem_obj->url);
+
     reply_hdr_state++;
+
     debug(11, 9) ("GOT HTTP REPLY HDR:\n---------\n%s\n----------\n",
-                  reply_hdr);
+                  reply_hdr.buf);
+
     /* Parse headers into reply structure */
     /* what happens if we fail to parse here? */
-    httpReplyParse(reply, reply_hdr, hdr_len);
+    httpReplyParse(reply, reply_hdr.buf, hdr_size);
 
     if (reply->sline.status >= HTTP_INVALID_HEADER) {
-        debug(11, 3) ("httpProcessReplyHeader: Non-HTTP-compliant header: '%s'\n", reply_hdr);
-        reply->sline.version = HttpVersion(1, 0);
-        reply->sline.status = HTTP_INVALID_HEADER;
-        storeEntryReplaceObject (entry, reply);
-
-        if (eof == 1) {
-            fwdComplete(fwd);
-            comm_close(fd);
-        }
-
+        debugs(11, 3, "httpProcessReplyHeader: Non-HTTP-compliant header: '" << reply_hdr.buf << "'\n");
+        failReply (reply, HTTP_INVALID_HEADER);
         return;
     }
 
@@ -934,7 +947,7 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
         IOStats.Http.read_hist[bin]++;
     }
 
-    if (!reply_hdr && flag == COMM_OK && len > 0) {
+    if (!memBufIsNull(&reply_hdr) && flag == COMM_OK && len > 0) {
         /* Skip whitespace */
 
         while (len > 0 && xisspace(*buf))
@@ -956,7 +969,7 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
             do_next_read = 1;
         } else if (entry->isEmpty()) {
             ErrorState *err;
-            err = errorCon(ERR_READ_ERROR, HTTP_INTERNAL_SERVER_ERROR);
+            err = errorCon(ERR_READ_ERROR, HTTP_BAD_GATEWAY);
             err->request = requestLink((HttpRequest *) request);
             err->xerrno = errno;
             fwdFail(fwd, err);
@@ -968,7 +981,7 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
         }
     } else if (flag == COMM_OK && len == 0 && entry->isEmpty()) {
         ErrorState *err;
-        err = errorCon(ERR_ZERO_SIZE_OBJECT, HTTP_SERVICE_UNAVAILABLE);
+        err = errorCon(ERR_ZERO_SIZE_OBJECT, HTTP_BAD_GATEWAY);
         err->xerrno = errno;
         err->request = requestLink((HttpRequest *) request);
         fwdFail(fwd, err);
@@ -995,7 +1008,17 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
             fwdFail(fwd, err);
             do_next_read = 0;
         } else {
-            fwdComplete(fwd);
+            if (entry->mem_obj->getReply()->sline.status == HTTP_HEADER_TOO_LARGE) {
+                ErrorState *err;
+                storeEntryReset(entry);
+                err = errorCon(ERR_TOO_BIG, HTTP_BAD_GATEWAY);
+                err->request = requestLink((HttpRequest *) request);
+                fwdFail(fwd, err);
+                fwd->flags.dont_retry = 1;
+            } else {
+                fwdComplete(fwd);
+            }
+
             do_next_read = 0;
             comm_close(fd);
         }
@@ -1163,7 +1186,7 @@ HttpStateData::SendComplete(int fd, char *bufnotused, size_t size, comm_err_t er
         return;
 
     if (errflag) {
-        err = errorCon(ERR_WRITE_ERROR, HTTP_INTERNAL_SERVER_ERROR);
+        err = errorCon(ERR_WRITE_ERROR, HTTP_BAD_GATEWAY);
         err->xerrno = errno;
         err->request = requestLink(httpState->orig_request);
         errorAppendEntry(entry, err);
@@ -1796,7 +1819,7 @@ httpSendRequestEntity(int fd, char *bufnotused, size_t size, comm_err_t errflag,
         return;
 
     if (errflag) {
-        err = errorCon(ERR_WRITE_ERROR, HTTP_INTERNAL_SERVER_ERROR);
+        err = errorCon(ERR_WRITE_ERROR, HTTP_BAD_GATEWAY);
         err->xerrno = errno;
         err->request = requestLink(httpState->orig_request);
         errorAppendEntry(entry, err);

diff --git a/src/http.h b/src/http.h
index 177c58b00fa0610e2a8c685ed44942febd297bc7..9f04690d9f2268b1652b89a001ceb9e5964eeca5 100644 (file)
--- a/src/http.h
+++ b/src/http.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.h,v 1.9 2003/08/10 11:00:43 robertc Exp $
+ * $Id: http.h,v 1.10 2004/12/21 17:52:53 robertc Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -42,7 +42,8 @@ class HttpStateData
 
 public:
     static CWCB SendComplete;
-    void processReplyHeader(const char *, int);
+    /* should be private */
+    void processReplyHeader(const char *buf, int size);
     void processReplyData(const char *, size_t);
     IOCB readReply;
     void maybeReadData();
@@ -50,8 +51,7 @@ public:
 
     StoreEntry *entry;
     HttpRequest *request;
-    char *reply_hdr;
-    size_t reply_hdr_size;
+    MemBuf reply_hdr;
     int reply_hdr_state;
     peer *_peer;               /* peer request made to */
     int eof;                   /* reached end-of-object? */
@@ -75,6 +75,7 @@ private:
     };
     ConnectionStatus statusIfComplete() const;
     ConnectionStatus persistentConnStatus() const;
+    void failReply (HttpReply *reply, http_status const &status);
 };
 
 #endif /* SQUID_HTTP_H */

diff --git a/src/protos.h b/src/protos.h
index 14280aabccabfb65a1ef1d04d888477f360eaf96..f2fd66f7ca5b1973e84182b6abfd7d6331a249c3 100644 (file)
--- a/src/protos.h
+++ b/src/protos.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: protos.h,v 1.501 2004/12/20 16:30:36 robertc Exp $
+ * $Id: protos.h,v 1.502 2004/12/21 17:52:53 robertc Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -285,7 +285,6 @@ SQUIDCEXTERN void whoisStart(FwdState *);
 /* http.c */
 SQUIDCEXTERN int httpCachable(method_t);
 SQUIDCEXTERN void httpStart(FwdState *);
-SQUIDCEXTERN void httpParseReplyHeaders(const char *, HttpReply *);
 SQUIDCEXTERN mb_size_t httpBuildRequestPrefix(HttpRequest * request,
         HttpRequest * orig_request,
         StoreEntry * entry,

diff --git a/src/structs.h b/src/structs.h
index af521c1c3850f08b491f7ed9d88f8b4d37524b93..7806912a2e31b99a49f9a89a6c99496d246f0a07 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: structs.h,v 1.498 2004/12/20 16:30:36 robertc Exp $
+ * $Id: structs.h,v 1.499 2004/12/21 17:52:53 robertc Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -284,6 +284,7 @@ struct _SquidConfig
     Timeout;
     size_t maxRequestHeaderSize;
     size_t maxRequestBodySize;
+    size_t maxReplyHeaderSize;
     acl_size_t *ReplyBodySize;
 
     struct