21377893 problem in UTILITY/SQUID
authorRich Burridge <rich.burridge@oracle.com>
Wed, 13 Apr 2016 10:14:18 -0700
changeset 5756 8233953c0160
parent 5755 041717cfc591
child 5757 9c6a2daa1337
21377893 problem in UTILITY/SQUID 23088298 problem in UTILITY/SQUID 23088308 problem in UTILITY/SQUID
components/squid/patches/CVE-2015-5400.patch
components/squid/patches/CVE-2016-3947.patch
components/squid/patches/CVE-2016-3948.patch
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/squid/patches/CVE-2015-5400.patch	Wed Apr 13 10:14:18 2016 -0700
@@ -0,0 +1,255 @@
+Fix for CVE-2015-5400. See:
+
+  http://www.squid-cache.org/Advisories/SQUID-2015_2.txt
+
+for more details. Based on the squid version 3.5.X patch at:
+
+  http://www.squid-cache.org/Versions/v3/3.5/changesets/squid-3.5-13856.patch
+
+--- squid-3.5.5/src/tunnel.cc.orig	2016-04-12 11:11:11.546364864 -0700
++++ squid-3.5.5/src/tunnel.cc	2016-04-12 11:12:40.789271952 -0700
[email protected]@ -110,6 +110,10 @@
+                  (request->flags.interceptTproxy || request->flags.intercepted));
+     }
+ 
++    /// Sends "502 Bad Gateway" error response to the client,
++    /// if it is waiting for Squid CONNECT response, closing connections.
++    void informUserOfPeerError(const char *errMsg);
++
+     class Connection
+     {
+ 
[email protected]@ -128,12 +132,13 @@
+ 
+         void error(int const xerrno);
+         int debugLevelForError(int const xerrno) const;
+-        /// handles a non-I/O error associated with this Connection
+-        void logicError(const char *errMsg);
+         void closeIfOpen();
+         void dataSent (size_t amount);
++        /// writes 'b' buffer, setting the 'writer' member to 'callback'.
++        void write(const char *b, int size, AsyncCall::Pointer &callback, FREE * free_func);
+         int len;
+         char *buf;
++        AsyncCall::Pointer writer; ///< pending Comm::Write callback
+         int64_t *size_ptr;      /* pointer to size in an ConnStateData for logging */
+ 
+         Comm::ConnectionPointer conn;    ///< The currently connected connection.
[email protected]@ -223,6 +228,7 @@
+     TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+     debugs(26, 3, HERE << tunnelState->server.conn);
+     tunnelState->server.conn = NULL;
++    tunnelState->server.writer = NULL;
+ 
+     if (tunnelState->request != NULL)
+         tunnelState->request->hier.stopPeerClock(false);
[email protected]@ -232,7 +238,7 @@
+         return;
+     }
+ 
+-    if (!tunnelState->server.len) {
++    if (!tunnelState->client.writer) {
+         tunnelState->client.conn->close();
+         return;
+     }
[email protected]@ -244,13 +250,14 @@
+     TunnelStateData *tunnelState = (TunnelStateData *)params.data;
+     debugs(26, 3, HERE << tunnelState->client.conn);
+     tunnelState->client.conn = NULL;
++    tunnelState->client.writer = NULL;
+ 
+     if (tunnelState->noConnections()) {
+         delete tunnelState;
+         return;
+     }
+ 
+-    if (!tunnelState->client.len) {
++    if (!tunnelState->server.writer) {
+         tunnelState->server.conn->close();
+         return;
+     }
[email protected]@ -381,6 +388,23 @@
+         handleConnectResponse(len);
+ }
+ 
++void
++TunnelStateData::informUserOfPeerError(const char *errMsg)
++{
++    server.len = 0;
++    if (!clientExpectsConnectResponse()) {
++        // closing the connection is the best we can do here
++        debugs(50, 3, server.conn << " closing on error: " << errMsg);
++        server.conn->close();
++        return;
++    }
++    ErrorState *err  = new ErrorState(ERR_CONNECT_FAIL, Http::scBadGateway, request.getRaw());
++    err->callback = tunnelErrorComplete;
++    err->callback_data = this;
++    *status_ptr = Http::scBadGateway;
++    errorSend(http->getConn()->clientConnection, err);
++}
++
+ /* Read from client side and queue it for writing to the server */
+ void
+ TunnelStateData::ReadConnectResponseDone(const Comm::ConnectionPointer &, char *buf, size_t len, Comm::Flag errcode, int xerrno, void *data)
[email protected]@ -412,7 +436,7 @@
+     const bool parsed = rep.parse(connectRespBuf, eof, &parseErr);
+     if (!parsed) {
+         if (parseErr > 0) { // unrecoverable parsing error
+-            server.logicError("malformed CONNECT response from peer");
++            informUserOfPeerError("malformed CONNECT response from peer");
+             return;
+         }
+ 
[email protected]@ -421,7 +445,7 @@
+         assert(!parseErr);
+ 
+         if (!connectRespBuf->hasSpace()) {
+-            server.logicError("huge CONNECT response from peer");
++            informUserOfPeerError("huge CONNECT response from peer");
+             return;
+         }
+ 
[email protected]@ -435,7 +459,8 @@
+ 
+     // bail if we did not get an HTTP 200 (Connection Established) response
+     if (rep.sline.status() != Http::scOkay) {
+-        server.logicError("unsupported CONNECT response status code");
++        // if we ever decide to reuse the peer connection, we must extract the error response first
++        informUserOfPeerError("unsupported CONNECT response status code");
+         return;
+     }
+ 
[email protected]@ -454,13 +479,6 @@
+ }
+ 
+ void
+-TunnelStateData::Connection::logicError(const char *errMsg)
+-{
+-    debugs(50, 3, conn << " closing on error: " << errMsg);
+-    conn->close();
+-}
+-
+-void
+ TunnelStateData::Connection::error(int const xerrno)
+ {
+     /* XXX fixme xstrerror and xerrno... */
[email protected]@ -556,7 +574,7 @@
+     debugs(26, 3, HERE << "Schedule Write");
+     AsyncCall::Pointer call = commCbCall(5,5, "TunnelBlindCopyWriteHandler",
+                                          CommIoCbPtrFun(completion, this));
+-    Comm::Write(to.conn, from.buf, len, call, NULL);
++    to.write(from.buf, len, call, NULL);
+ }
+ 
+ /* Writes data from the client buffer to the server side */
[email protected]@ -565,6 +583,7 @@
+ {
+     TunnelStateData *tunnelState = (TunnelStateData *)data;
+     assert (cbdataReferenceValid (tunnelState));
++    tunnelState->server.writer = NULL;
+ 
+     tunnelState->writeServerDone(buf, len, flag, xerrno);
+ }
[email protected]@ -614,6 +633,7 @@
+ {
+     TunnelStateData *tunnelState = (TunnelStateData *)data;
+     assert (cbdataReferenceValid (tunnelState));
++    tunnelState->client.writer = NULL;
+ 
+     tunnelState->writeClientDone(buf, len, flag, xerrno);
+ }
[email protected]@ -631,7 +651,14 @@
+ }
+ 
+ void
+-TunnelStateData::writeClientDone(char *buf, size_t len, Comm::Flag flag, int xerrno)
++TunnelStateData::Connection::write(const char *b, int size, AsyncCall::Pointer &callback, FREE * free_func)
++{
++    writer = callback;
++    Comm::Write(conn, b, size, callback, free_func);
++}
++
++void
++TunnelStateData::writeClientDone(char *, size_t len, Comm::Flag flag, int xerrno)
+ {
+     debugs(26, 3, HERE << client.conn << ", " << len << " bytes written, flag=" << flag);
+ 
[email protected]@ -789,6 +816,7 @@
+ {
+     TunnelStateData *tunnelState = (TunnelStateData *)data;
+     debugs(26, 3, HERE << conn << ", flag=" << flag);
++    tunnelState->client.writer = NULL;
+ 
+     if (flag != Comm::OK) {
+         *tunnelState->status_ptr = Http::scInternalServerError;
[email protected]@ -805,6 +833,7 @@
+ {
+     TunnelStateData *tunnelState = (TunnelStateData *)data;
+     debugs(26, 3, conn << ", flag=" << flag);
++    tunnelState->server.writer = NULL;
+     assert(tunnelState->waitingForConnectRequest());
+ 
+     if (flag != Comm::OK) {
[email protected]@ -845,7 +874,7 @@
+     else {
+         AsyncCall::Pointer call = commCbCall(5,5, "tunnelConnectedWriteDone",
+                                              CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
+-        Comm::Write(tunnelState->client.conn, conn_established, strlen(conn_established), call, NULL);
++        tunnelState->client.write(conn_established, strlen(conn_established), call, NULL);
+     }
+ }
+ 
[email protected]@ -1064,29 +1093,21 @@
+     debugs(11, 2, "Tunnel Server REQUEST: " << tunnelState->server.conn << ":\n----------\n" <<
+            Raw("tunnelRelayConnectRequest", mb.content(), mb.contentSize()) << "\n----------");
+ 
+-    if (tunnelState->clientExpectsConnectResponse()) {
+-        // hack: blindly tunnel peer response (to our CONNECT request) to the client as ours.
+-        AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectedWriteDone",
+-                                       CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
+-        Comm::Write(srv, &mb, writeCall);
+-    } else {
+-        // we have to eat the connect response from the peer (so that the client
+-        // does not see it) and only then start shoveling data to the client
+-        AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectReqWriteDone",
+-                                       CommIoCbPtrFun(tunnelConnectReqWriteDone,
+-                                               tunnelState));
+-        Comm::Write(srv, &mb, writeCall);
+-        tunnelState->connectReqWriting = true;
+-
+-        tunnelState->connectRespBuf = new MemBuf;
+-        // SQUID_TCP_SO_RCVBUF: we should not accumulate more than regular I/O buffer
+-        // can hold since any CONNECT response leftovers have to fit into server.buf.
+-        // 2*SQUID_TCP_SO_RCVBUF: HttpMsg::parse() zero-terminates, which uses space.
+-        tunnelState->connectRespBuf->init(SQUID_TCP_SO_RCVBUF, 2*SQUID_TCP_SO_RCVBUF);
+-        tunnelState->readConnectResponse();
++    AsyncCall::Pointer writeCall = commCbCall(5,5, "tunnelConnectReqWriteDone",
++                                   CommIoCbPtrFun(tunnelConnectReqWriteDone,
++                                           tunnelState));
++
++    tunnelState->server.write(mb.buf, mb.size, writeCall, mb.freeFunc());
++    tunnelState->connectReqWriting = true;
++
++    tunnelState->connectRespBuf = new MemBuf;
++    // SQUID_TCP_SO_RCVBUF: we should not accumulate more than regular I/O buffer
++    // can hold since any CONNECT response leftovers have to fit into server.buf.
++    // 2*SQUID_TCP_SO_RCVBUF: HttpMsg::parse() zero-terminates, which uses space.
++    tunnelState->connectRespBuf->init(SQUID_TCP_SO_RCVBUF, 2*SQUID_TCP_SO_RCVBUF);
++    tunnelState->readConnectResponse();
+ 
+-        assert(tunnelState->waitingForConnectExchange());
+-    }
++    assert(tunnelState->waitingForConnectExchange());
+ 
+     AsyncCall::Pointer timeoutCall = commCbCall(5, 4, "tunnelTimeout",
+                                      CommTimeoutCbPtrFun(tunnelTimeout, tunnelState));
[email protected]@ -1219,7 +1240,7 @@
+ 
+     AsyncCall::Pointer call = commCbCall(5,5, "tunnelConnectedWriteDone",
+                                          CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
+-    Comm::Write(tunnelState->client.conn, buf.content(), buf.contentSize(), call, NULL);
++    tunnelState->client.write(buf.content(), buf.contentSize(), call, NULL);
+ }
+ #endif //USE_OPENSSL
+ 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/squid/patches/CVE-2016-3947.patch	Wed Apr 13 10:14:18 2016 -0700
@@ -0,0 +1,36 @@
+Fix for CVE-2016-3947. See:
+
+  https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2016-3947
+
+for more details. Based on the squid 3.5.X patch at:
+
+  http://www.squid-cache.org/Versions/v3/3.5/changesets/squid-3.5-14015.patch
+
+--- squid-3.5.5/src/icmp/Icmp6.cc.orig	2016-04-12 11:19:40.947624766 -0700
++++ squid-3.5.5/src/icmp/Icmp6.cc	2016-04-12 11:20:00.180868789 -0700
[email protected]@ -256,7 +256,7 @@
+     #define ip6_hops    // HOPS!!!  (can it be true??)
+ 
+         ip = (struct ip6_hdr *) pkt;
+-        pkt += sizeof(ip6_hdr);
++        NP: echo size needs to +sizeof(ip6_hdr);
+ 
+     debugs(42, DBG_CRITICAL, HERE << "ip6_nxt=" << ip->ip6_nxt <<
+             ", ip6_plen=" << ip->ip6_plen <<
[email protected]@ -267,7 +267,6 @@
+     */
+ 
+     icmp6header = (struct icmp6_hdr *) pkt;
+-    pkt += sizeof(icmp6_hdr);
+ 
+     if (icmp6header->icmp6_type != ICMP6_ECHO_REPLY) {
+ 
[email protected]@ -292,7 +291,7 @@
+         return;
+     }
+ 
+-    echo = (icmpEchoData *) pkt;
++    echo = (icmpEchoData *) (pkt + sizeof(icmp6_hdr));
+ 
+     preply.opcode = echo->opcode;
+ 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/squid/patches/CVE-2016-3948.patch	Wed Apr 13 10:14:18 2016 -0700
@@ -0,0 +1,416 @@
+Fix for CVE-2016-3948. See:
+
+  http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2016-3948
+
+for more details. Based on the squid version 3.5.X patch at:
+
+  http://www.squid-cache.org/Versions/v3/3.5/changesets/squid-3.5-14016.patch
+ 
+--- squid-3.5.5/src/HttpRequest.cc.orig	2016-04-12 11:32:45.105453713 -0700
++++ squid-3.5.5/src/HttpRequest.cc	2016-04-12 11:34:54.820733440 -0700
[email protected]@ -92,7 +92,7 @@
+     peer_login = NULL;      // not allocated/deallocated by this class
+     peer_domain = NULL;     // not allocated/deallocated by this class
+     peer_host = NULL;
+-    vary_headers = NULL;
++    vary_headers = SBuf();
+     myportname = null_string;
+     tag = null_string;
+ #if USE_AUTH
[email protected]@ -124,9 +124,7 @@
+     auth_user_request = NULL;
+ #endif
+     safe_free(canonical);
+-
+-    safe_free(vary_headers);
+-
++    vary_headers.clear();
+     url.clear();
+     urlpath.clean();
+ 
[email protected]@ -203,7 +201,7 @@
+ 
+     copy->lastmod = lastmod;
+     copy->etag = etag;
+-    copy->vary_headers = vary_headers ? xstrdup(vary_headers) : NULL;
++    copy->vary_headers = vary_headers;
+     // XXX: what to do with copy->peer_domain?
+ 
+     copy->tag = tag;
+--- squid-3.5.5/src/HttpRequest.h.orig	2016-04-12 11:32:45.107463536 -0700
++++ squid-3.5.5/src/HttpRequest.h	2016-04-12 11:35:03.221096498 -0700
[email protected]@ -179,7 +179,8 @@
+ 
+     time_t lastmod;     /* Used on refreshes */
+ 
+-    const char *vary_headers;   /* Used when varying entities are detected. Changes how the store key is calculated */
++    /// The variant second-stage cache key. Generated from Vary header pattern for this request.
++    SBuf vary_headers;
+ 
+     char *peer_domain;      /* Configured peer forceddomain */
+ 
+--- squid-3.5.5/src/MemObject.cc.orig	2016-04-12 11:32:45.109364872 -0700
++++ squid-3.5.5/src/MemObject.cc	2016-04-12 11:35:07.861297412 -0700
[email protected]@ -136,8 +136,6 @@
+     HTTPMSGUNLOCK(request);
+ 
+     ctx_exit(ctx);              /* must exit before we free mem->url */
+-
+-    safe_free(vary_headers);
+ }
+ 
+ void
[email protected]@ -221,8 +219,8 @@
+ MemObject::stat(MemBuf * mb) const
+ {
+     mb->Printf("\t" SQUIDSBUFPH " %s\n", SQUIDSBUFPRINT(method.image()), logUri());
+-    if (vary_headers)
+-        mb->Printf("\tvary_headers: %s\n", vary_headers);
++    if (!vary_headers.isEmpty())
++        mb->Printf("\tvary_headers: " SQUIDSBUFPH "\n", SQUIDSBUFPRINT(vary_headers));
+     mb->Printf("\tinmem_lo: %" PRId64 "\n", inmem_lo);
+     mb->Printf("\tinmem_hi: %" PRId64 "\n", data_hdr.endOffset());
+     mb->Printf("\tswapout: %" PRId64 " bytes queued\n",
+--- squid-3.5.5/src/MemObject.h.orig	2016-04-12 11:32:45.111075168 -0700
++++ squid-3.5.5/src/MemObject.h	2016-04-12 11:35:10.909852018 -0700
[email protected]@ -13,6 +13,7 @@
+ #include "dlink.h"
+ #include "HttpRequestMethod.h"
+ #include "RemovalPolicy.h"
++#include "SBuf.h"
+ #include "stmem.h"
+ #include "StoreIOBuffer.h"
+ #include "StoreIOState.h"
[email protected]@ -167,7 +168,7 @@
+     unsigned int chksum;
+ #endif
+ 
+-    const char *vary_headers;
++    SBuf vary_headers;
+ 
+     void delayRead(DeferredRead const &);
+     void kickReads();
+--- squid-3.5.5/src/MemStore.cc.orig	2016-04-12 11:32:45.112589100 -0700
++++ squid-3.5.5/src/MemStore.cc	2016-04-12 11:35:13.878529885 -0700
[email protected]@ -443,7 +443,7 @@
+ 
+     assert(e.mem_obj);
+ 
+-    if (e.mem_obj->vary_headers) {
++    if (!e.mem_obj->vary_headers.isEmpty()) {
+         // XXX: We must store/load SerialisedMetaData to cache Vary in RAM
+         debugs(20, 5, "Vary not yet supported: " << e.mem_obj->vary_headers);
+         return false;
+--- squid-3.5.5/src/StoreMetaVary.cc.orig	2016-04-12 11:32:45.114054766 -0700
++++ squid-3.5.5/src/StoreMetaVary.cc	2016-04-12 11:35:17.350257452 -0700
[email protected]@ -18,14 +18,14 @@
+ {
+     assert (getType() == STORE_META_VARY_HEADERS);
+ 
+-    if (!e->mem_obj->vary_headers) {
++    if (e->mem_obj->vary_headers.isEmpty()) {
+         /* XXX separate this mutator from the query */
+         /* Assume the object is OK.. remember the vary request headers */
+-        e->mem_obj->vary_headers = xstrdup((char *)value);
++        e->mem_obj->vary_headers.assign(static_cast<const char *>(value), length);
+         return true;
+     }
+ 
+-    if (strcmp(e->mem_obj->vary_headers, (char *)value) != 0)
++    if (e->mem_obj->vary_headers.cmp(static_cast<const char *>(value), length) != 0)
+         return false;
+ 
+     return true;
+--- squid-3.5.5/src/client_side.cc.orig	2016-04-12 11:30:22.544117692 -0700
++++ squid-3.5.5/src/client_side.cc	2016-04-12 11:35:20.775116943 -0700
[email protected]@ -4627,20 +4627,19 @@
+ int
+ varyEvaluateMatch(StoreEntry * entry, HttpRequest * request)
+ {
+-    const char *vary = request->vary_headers;
++    SBuf vary(request->vary_headers);
+     int has_vary = entry->getReply()->header.has(HDR_VARY);
+ #if X_ACCELERATOR_VARY
+-
+     has_vary |=
+         entry->getReply()->header.has(HDR_X_ACCELERATOR_VARY);
+ #endif
+ 
+-    if (!has_vary || !entry->mem_obj->vary_headers) {
+-        if (vary) {
++    if (!has_vary || entry->mem_obj->vary_headers.isEmpty()) {
++        if (!vary.isEmpty()) {
+             /* Oops... something odd is going on here.. */
+             debugs(33, DBG_IMPORTANT, "varyEvaluateMatch: Oops. Not a Vary object on second attempt, '" <<
+                    entry->mem_obj->urlXXX() << "' '" << vary << "'");
+-            safe_free(request->vary_headers);
++            request->vary_headers.clear();
+             return VARY_CANCEL;
+         }
+ 
[email protected]@ -4654,8 +4653,8 @@
+          */
+         vary = httpMakeVaryMark(request, entry->getReply());
+ 
+-        if (vary) {
+-            request->vary_headers = xstrdup(vary);
++        if (!vary.isEmpty()) {
++            request->vary_headers = vary;
+             return VARY_OTHER;
+         } else {
+             /* Ouch.. we cannot handle this kind of variance */
[email protected]@ -4663,18 +4662,18 @@
+             return VARY_CANCEL;
+         }
+     } else {
+-        if (!vary) {
++        if (vary.isEmpty()) {
+             vary = httpMakeVaryMark(request, entry->getReply());
+ 
+-            if (vary)
+-                request->vary_headers = xstrdup(vary);
++            if (!vary.isEmpty())
++                request->vary_headers = vary;
+         }
+ 
+-        if (!vary) {
++        if (vary.isEmpty()) {
+             /* Ouch.. we cannot handle this kind of variance */
+             /* XXX This cannot really happen, but just to be complete */
+             return VARY_CANCEL;
+-        } else if (strcmp(vary, entry->mem_obj->vary_headers) == 0) {
++        } else if (vary.cmp(entry->mem_obj->vary_headers) == 0) {
+             return VARY_MATCH;
+         } else {
+             /* Oops.. we have already been here and still haven't
+--- squid-3.5.5/src/client_side_reply.cc.orig	2016-04-12 11:30:24.187260295 -0700
++++ squid-3.5.5/src/client_side_reply.cc	2016-04-12 11:35:25.478369424 -0700
[email protected]@ -988,9 +988,8 @@
+     }
+ 
+     /* And for Vary, release the base URI if none of the headers was included in the request */
+-
+-    if (http->request->vary_headers
+-            && !strstr(http->request->vary_headers, "=")) {
++    if (!http->request->vary_headers.isEmpty()
++            && http->request->vary_headers.find('=') != SBuf::npos) {
+         StoreEntry *entry = storeGetPublic(urlCanonical(http->request), Http::METHOD_GET);
+ 
+         if (entry) {
+--- squid-3.5.5/src/http.cc.orig	2016-04-12 11:30:25.907567935 -0700
++++ squid-3.5.5/src/http.cc	2016-04-12 11:35:29.150761600 -0700
[email protected]@ -573,9 +573,9 @@
+ /*
+  * For Vary, store the relevant request headers as
+  * virtual headers in the reply
+- * Returns false if the variance cannot be stored
++ * Returns an empty SBuf if the variance cannot be stored
+  */
+-const char *
++SBuf
+ httpMakeVaryMark(HttpRequest * request, HttpReply const * reply)
+ {
+     String vary, hdr;
[email protected]@ -583,9 +583,9 @@
+     const char *item;
+     const char *value;
+     int ilen;
+-    static String vstr;
++    SBuf vstr;
++    static const SBuf asterisk("*");
+ 
+-    vstr.clean();
+     vary = reply->header.getList(HDR_VARY);
+ 
+     while (strListGetItem(&vary, ',', &item, &ilen, &pos)) {
[email protected]@ -596,11 +596,13 @@
+         if (strcmp(name, "*") == 0) {
+             /* Can not handle "Vary: *" withtout ETag support */
+             safe_free(name);
+-            vstr.clean();
++            vstr.clear();
+             break;
+         }
+ 
+-        strListAdd(&vstr, name, ',');
++        if (!vstr.isEmpty())
++            vstr.append(", ", 2);
++        vstr.append(name);
+         hdr = request->header.getByName(name);
+         safe_free(name);
+         value = hdr.termedBuf();
[email protected]@ -625,7 +627,17 @@
+         char *name = (char *)xmalloc(ilen + 1);
+         xstrncpy(name, item, ilen + 1);
+         Tolower(name);
+-        strListAdd(&vstr, name, ',');
++
++        if (strcmp(name, "*") == 0) {
++            /* Can not handle "Vary: *" withtout ETag support */
++            safe_free(name);
++            vstr.clear();
++            break;
++        }
++
++        if (!vstr.isEmpty())
++            vstr.append(", ", 2);
++        vstr.append(name);
+         hdr = request->header.getByName(name);
+         safe_free(name);
+         value = hdr.termedBuf();
[email protected]@ -643,8 +655,8 @@
+     vary.clean();
+ #endif
+ 
+-    debugs(11, 3, "httpMakeVaryMark: " << vstr);
+-    return vstr.termedBuf();
++    debugs(11, 3, vstr);
++    return vstr;
+ }
+ 
+ void
[email protected]@ -917,15 +929,15 @@
+             || rep->header.has(HDR_X_ACCELERATOR_VARY)
+ #endif
+        ) {
+-        const char *vary = httpMakeVaryMark(request, rep);
++        const SBuf vary(httpMakeVaryMark(request, rep));
+ 
+-        if (!vary) {
++        if (vary.isEmpty()) {
+             entry->makePrivate();
+             if (!fwd->reforwardableStatus(rep->sline.status()))
+                 EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
+             varyFailure = true;
+         } else {
+-            entry->mem_obj->vary_headers = xstrdup(vary);
++            entry->mem_obj->vary_headers = vary;
+         }
+     }
+ 
+--- squid-3.5.5/src/http.h.orig	2016-04-12 11:30:26.674812741 -0700
++++ squid-3.5.5/src/http.h	2016-04-12 11:35:34.119393224 -0700
[email protected]@ -12,6 +12,7 @@
+ #include "clients/Client.h"
+ #include "comm.h"
+ #include "HttpStateFlags.h"
++#include "SBuf.h"
+ 
+ class ChunkedCodingParser;
+ class FwdState;
[email protected]@ -116,7 +117,7 @@
+ 
+ int httpCachable(const HttpRequestMethod&);
+ void httpStart(FwdState *);
+-const char *httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
++SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
+ 
+ #endif /* SQUID_HTTP_H */
+ 
+--- squid-3.5.5/src/store.cc.orig	2016-04-12 11:30:28.336891365 -0700
++++ squid-3.5.5/src/store.cc	2016-04-12 11:35:38.983063089 -0700
[email protected]@ -685,31 +685,27 @@
+     if (mem_obj->request) {
+         HttpRequest *request = mem_obj->request;
+ 
+-        if (!mem_obj->vary_headers) {
++        if (mem_obj->vary_headers.isEmpty()) {
+             /* First handle the case where the object no longer varies */
+-            safe_free(request->vary_headers);
++            request->vary_headers.clear();
+         } else {
+-            if (request->vary_headers && strcmp(request->vary_headers, mem_obj->vary_headers) != 0) {
++            if (!request->vary_headers.isEmpty() && request->vary_headers.cmp(mem_obj->vary_headers) != 0) {
+                 /* Oops.. the variance has changed. Kill the base object
+                  * to record the new variance key
+                  */
+-                safe_free(request->vary_headers);       /* free old "bad" variance key */
++                request->vary_headers.clear();       /* free old "bad" variance key */
+                 if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method))
+                     pe->release();
+             }
+ 
+             /* Make sure the request knows the variance status */
+-            if (!request->vary_headers) {
+-                const char *vary = httpMakeVaryMark(request, mem_obj->getReply());
+-
+-                if (vary)
+-                    request->vary_headers = xstrdup(vary);
+-            }
++            if (request->vary_headers.isEmpty())
++                request->vary_headers = httpMakeVaryMark(request, mem_obj->getReply());
+         }
+ 
+         // TODO: storeGetPublic() calls below may create unlocked entries.
+         // We should add/use storeHas() API or lock/unlock those entries.
+-        if (mem_obj->vary_headers && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {
++        if (!mem_obj->vary_headers.isEmpty() && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {
+             /* Create "vary" base object */
+             String vary;
+             StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method);
+--- squid-3.5.5/src/store_key_md5.cc.orig	2016-04-12 11:32:45.123264698 -0700
++++ squid-3.5.5/src/store_key_md5.cc	2016-04-12 11:35:42.319388524 -0700
[email protected]@ -125,8 +125,8 @@
+     SquidMD5Update(&M, &m, sizeof(m));
+     SquidMD5Update(&M, (unsigned char *) url, strlen(url));
+ 
+-    if (request->vary_headers) {
+-        SquidMD5Update(&M, (unsigned char *) request->vary_headers, strlen(request->vary_headers));
++    if (!request->vary_headers.isEmpty()) {
++        SquidMD5Update(&M, request->vary_headers.rawContent(), request->vary_headers.length());
+         debugs(20, 3, "updating public key by vary headers: " << request->vary_headers << " for: " << url);
+     }
+ 
+--- squid-3.5.5/src/store_swapmeta.cc.orig	2016-04-12 11:32:45.124659332 -0700
++++ squid-3.5.5/src/store_swapmeta.cc	2016-04-12 11:35:45.927230596 -0700
[email protected]@ -40,7 +40,6 @@
+     tlv *TLV = NULL;        /* we'll return this */
+     tlv **T = &TLV;
+     const char *url;
+-    const char *vary;
+     assert(e->mem_obj != NULL);
+     const int64_t objsize = e->mem_obj->expectedReplySize();
+     assert(e->swap_status == SWAPOUT_WRITING);
[email protected]@ -87,10 +86,12 @@
+     }
+ 
+     T = StoreMeta::Add(T, t);
+-    vary = e->mem_obj->vary_headers;
++    SBuf vary(e->mem_obj->vary_headers);
+ 
+-    if (vary) {
+-        t =StoreMeta::Factory(STORE_META_VARY_HEADERS, strlen(vary) + 1, vary);
++    if (!vary.isEmpty()) {
++        // TODO: do we still need +1 here? StoreMetaVary::checkConsistency
++        //       no longer relies on nul-termination, but other things might.
++        t = StoreMeta::Factory(STORE_META_VARY_HEADERS, vary.length() + 1, vary.c_str());
+ 
+         if (!t) {
+             storeSwapTLVFree(TLV);
+--- squid-3.5.5/src/tests/stub_MemObject.cc.orig	2016-04-12 11:34:21.726285840 -0700
++++ squid-3.5.5/src/tests/stub_MemObject.cc	2016-04-12 11:35:49.383455312 -0700
[email protected]@ -38,7 +38,6 @@
+     id(0),
+     object_sz(-1),
+     swap_hdr_sz(0),
+-    vary_headers(NULL),
+     _reply(NULL)
+ {
+     memset(&clients, 0, sizeof(clients));
+--- squid-3.5.5/src/tests/stub_http.cc.orig	2016-04-12 11:34:21.728074407 -0700
++++ squid-3.5.5/src/tests/stub_http.cc	2016-04-12 11:35:52.784226579 -0700
[email protected]@ -7,12 +7,12 @@
+  */
+ 
+ #include "squid.h"
+-
+ #include "HttpReply.h"
+ #include "HttpRequest.h"
++#include "SBuf.h"
+ 
+ #define STUB_API "http.cc"
+ #include "tests/STUB.h"
+ 
+-const char * httpMakeVaryMark(HttpRequest * request, HttpReply const * reply) STUB_RETVAL(NULL)
++SBuf httpMakeVaryMark(HttpRequest *, HttpReply const *) STUB_RETVAL(SBuf())
+