Signed-off-by: Matthias Fischer matthias.fischer@ipfire.org --- lfs/squid | 2 + src/patches/squid/squid-3.5-14082.patch | 414 ++++++++++++++++++++++++++++++++ src/patches/squid/squid-3.5-14083.patch | 48 ++++ 3 files changed, 464 insertions(+) create mode 100644 src/patches/squid/squid-3.5-14082.patch create mode 100644 src/patches/squid/squid-3.5-14083.patch

diff --git a/lfs/squid b/lfs/squid index ae8e1e9..9677c2b 100644 --- a/lfs/squid +++ b/lfs/squid @@ -70,6 +70,8 @@ $(subst %,%_MD5,$(objects)) : $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) @$(PREBUILD) @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar xaf $(DIR_DL)/$(DL_FILE) + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14082.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14083.patch cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid-3.5.21-fix-max-file-descriptors.patch

cd $(DIR_APP) && autoreconf -vfi diff --git a/src/patches/squid/squid-3.5-14082.patch b/src/patches/squid/squid-3.5-14082.patch new file mode 100644 index 0000000..cef06c8 --- /dev/null +++ b/src/patches/squid/squid-3.5-14082.patch @@ -0,0 +1,414 @@ +------------------------------------------------------------ +revno: 14082 +revision-id: squid3@treenet.co.nz-20160916115328-0fpefy6pbtfq40cd +parent: squid3@treenet.co.nz-20160908185514-e3zlew29xywz6c0o +author: Eduard Bagdasaryan eduard.bagdasaryan@measurement-factory.com, Alex Rousskov rousskov@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Fri 2016-09-16 23:53:28 +1200 +message: + Fix logged request size (%http::>st) and other size-related %codes. + + The %[http::]>st logformat code should log the actual number of + [dechunked] bytes received from the client. However, for requests with + Content-Length, Squid was logging the sum of the request header size and + the Content-Length header field value instead. The logged value was + wrong when the client sent fewer than Content-Length body bytes. + + Also: + + * Fixed %http::>h and %http::<h format codes for icap_log. The old code + was focusing on preserving the "request header" (i.e. not "response + header") meaning of the %http::>h logformat code, but since ICAP + services deal with (and log) both HTTP requests and responses, that + focus on the HTTP message kind actually complicates ICAP logging + configuration and will eventually require introduction of new %http + logformat codes that would be meaningless in access.log context. + + - In ICAP context, %http::>h should log to-be-adapted HTTP message + headers embedded in an ICAP request (HTTP request headers in REQMOD; + HTTP response headers in RESPMOD). Before this change, %http::>h + logged constant/"FYI" HTTP request headers in RESPMOD. + + - Similarly, %http::<h should log adapted HTTP message headers + embedded in an ICAP response (HTTP request headers in regular + REQMOD; HTTP response headers in RESPMOD and during request + satisfaction in REQMOD). Before this change, %http::<h logged "-" in + REQMOD. + + In other words, in ICAP logging context, the ">" and "<" characters + should indicate ICAP message kind (where the being-logged HTTP message + is embedded), not HTTP message kind, even for %http codes. + + TODO: %http::>h code logs "-" in RESPMOD because AccessLogEntry does + not store virgin HTTP response headers. + + * Correctly initialize ICAP ALE HTTP fields related to HTTP message + sizes for icap_log, including %http::>st, %http::<st, %http::>sh, and + %http::>sh format codes. + + * Initialize HttpMsg::hdr_sz in HTTP header parsing code. Among other + uses, this field is needed to initialize HTTP fields inside ICAP ALE. + + * Synced default icap_log format documentation with the current code. +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160916115328-0fpefy6pbtfq40cd +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 45a920a9aa3c53cab1b2bfb8af66bcd558fa96d5 +# timestamp: 2016-09-16 12:51:02 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160908185514-\ +# e3zlew29xywz6c0o +# +# Begin patch +=== modified file 'src/adaptation/icap/ModXact.cc' +--- src/adaptation/icap/ModXact.cc 2016-01-01 00:14:27 +0000 ++++ src/adaptation/icap/ModXact.cc 2016-09-16 11:53:28 +0000 +@@ -1259,31 +1259,32 @@ + + void Adaptation::Icap::ModXact::finalizeLogInfo() + { +- HttpRequest * request_ = NULL; +- HttpRequest * adapted_request_ = NULL; +- HttpReply * reply_ = NULL; +- request_ = (virgin.cause? virgin.cause: dynamic_cast<HttpRequest*>(virgin.header)); ++ HttpRequest *adapted_request_ = NULL; ++ HttpReply *adapted_reply_ = NULL; ++ HttpRequest *virgin_request_ = (virgin.cause ? virgin.cause : dynamic_cast<HttpRequest*>(virgin.header)); + if (!(adapted_request_ = dynamic_cast<HttpRequest*>(adapted.header))) { +- adapted_request_ = request_; +- reply_ = dynamic_cast<HttpReply*>(adapted.header); ++ // if the request was not adapted, use virgin request to simplify ++ // the code further below ++ adapted_request_ = virgin_request_; ++ adapted_reply_ = dynamic_cast<HttpReply*>(adapted.header); + } + +- Adaptation::Icap::History::Pointer h = (request_ ? request_->icapHistory() : NULL); ++ Adaptation::Icap::History::Pointer h = (virgin_request_ ? virgin_request_->icapHistory() : NULL); + Must(h != NULL); // ICAPXaction::maybeLog calls only if there is a log + al.icp.opcode = ICP_INVALID; + al.url = h->log_uri.termedBuf(); + const Adaptation::Icap::ServiceRep &s = service(); + al.icap.reqMethod = s.cfg().method; + +- al.cache.caddr = request_->client_addr; ++ al.cache.caddr = virgin_request_->client_addr; + +- al.request = request_; ++ al.request = virgin_request_; + HTTPMSGLOCK(al.request); + al.adapted_request = adapted_request_; + HTTPMSGLOCK(al.adapted_request); + +- if (reply_) { +- al.reply = reply_; ++ if (adapted_reply_) { ++ al.reply = adapted_reply_; + HTTPMSGLOCK(al.reply); + } else + al.reply = NULL; +@@ -1296,25 +1297,29 @@ + al.cache.ssluser = h->ssluser.termedBuf(); + #endif + al.cache.code = h->logType; +- // XXX: should use icap-specific counters instead ? +- al.http.clientRequestSz.payloadData = h->req_sz; ++ ++ const HttpMsg *virgin_msg = dynamic_cast<HttpReply*>(virgin.header); ++ if (!virgin_msg) ++ virgin_msg = virgin_request_; ++ assert(virgin_msg != virgin.cause); ++ al.http.clientRequestSz.header = virgin_msg->hdr_sz; ++ al.http.clientRequestSz.payloadData = virgin_msg->body_pipe->producedSize(); + + // leave al.icap.bodyBytesRead negative if no body + if (replyHttpHeaderSize >= 0 || replyHttpBodySize >= 0) { + const int64_t zero = 0; // to make max() argument types the same +- al.icap.bodyBytesRead = +- max(zero, replyHttpHeaderSize) + max(zero, replyHttpBodySize); ++ const uint64_t headerSize = max(zero, replyHttpHeaderSize); ++ const uint64_t bodySize = max(zero, replyHttpBodySize); ++ al.icap.bodyBytesRead = headerSize + bodySize; ++ al.http.clientReplySz.header = headerSize; ++ al.http.clientReplySz.payloadData = bodySize; + } + +- if (reply_) { +- al.http.code = reply_->sline.status(); +- al.http.content_type = reply_->content_type.termedBuf(); +- if (replyHttpBodySize >= 0) { +- // XXX: should use icap-specific counters instead ? +- al.http.clientReplySz.payloadData = replyHttpBodySize; +- al.http.clientReplySz.header = reply_->hdr_sz; ++ if (adapted_reply_) { ++ al.http.code = adapted_reply_->sline.status(); ++ al.http.content_type = adapted_reply_->content_type.termedBuf(); ++ if (replyHttpBodySize >= 0) + al.cache.highOffset = replyHttpBodySize; +- } + //don't set al.cache.objectSize because it hasn't exist yet + + Packer p; +@@ -1323,7 +1328,7 @@ + mb.init(); + packerToMemInit(&p, &mb); + +- reply_->header.packInto(&p); ++ adapted_reply_->header.packInto(&p); + al.headers.reply = xstrdup(mb.buf); + + packerClean(&p); + +=== modified file 'src/cf.data.pre' +--- src/cf.data.pre 2016-05-06 06:36:08 +0000 ++++ src/cf.data.pre 2016-09-16 11:53:28 +0000 +@@ -4427,13 +4427,35 @@ + ICAP transaction log lines will correspond to a single access + log line. + +- ICAP log uses logformat codes that make sense for an ICAP +- transaction. Header-related codes are applied to the HTTP header +- embedded in an ICAP server response, with the following caveats: +- For REQMOD, there is no HTTP response header unless the ICAP +- server performed request satisfaction. For RESPMOD, the HTTP +- request header is the header sent to the ICAP server. For +- OPTIONS, there are no HTTP headers. ++ ICAP log supports many access.log logformat %codes. In ICAP context, ++ HTTP message-related %codes are applied to the HTTP message embedded ++ in an ICAP message. Logformat "%http::>..." codes are used for HTTP ++ messages embedded in ICAP requests while "%http::<..." codes are used ++ for HTTP messages embedded in ICAP responses. For example: ++ ++ http::>h To-be-adapted HTTP message headers sent by Squid to ++ the ICAP service. For REQMOD transactions, these are ++ HTTP request headers. For RESPMOD, these are HTTP ++ response headers, but Squid currently cannot log them ++ (i.e., %http::>h will expand to "-" for RESPMOD). ++ ++ http::<h Adapted HTTP message headers sent by the ICAP ++ service to Squid (i.e., HTTP request headers in regular ++ REQMOD; HTTP response headers in RESPMOD and during ++ request satisfaction in REQMOD). ++ ++ ICAP OPTIONS transactions do not embed HTTP messages. ++ ++ Several logformat codes below deal with ICAP message bodies. An ICAP ++ message body, if any, typically includes a complete HTTP message ++ (required HTTP headers plus optional HTTP message body). When ++ computing HTTP message body size for these logformat codes, Squid ++ either includes or excludes chunked encoding overheads; see ++ code-specific documentation for details. ++ ++ For Secure ICAP services, all size-related information is currently ++ computed before/after TLS encryption/decryption, as if TLS was not ++ in use at all. + + The following format codes are also available for ICAP logs: + +@@ -4447,19 +4469,16 @@ + icap::rm ICAP request method (REQMOD, RESPMOD, or + OPTIONS). Similar to existing rm. + +- icap::>st Bytes sent to the ICAP server (TCP payload +- only; i.e., what Squid writes to the socket). +- +- icap::<st Bytes received from the ICAP server (TCP +- payload only; i.e., what Squid reads from +- the socket). +- +- icap::<bs Number of message body bytes received from the +- ICAP server. ICAP message body, if any, usually +- includes encapsulated HTTP message headers and +- possibly encapsulated HTTP message body. The +- HTTP body part is dechunked before its size is +- computed. ++ icap::>st The total size of the ICAP request sent to the ICAP ++ server (ICAP headers + ICAP body), including chunking ++ metadata (if any). ++ ++ icap::<st The total size of the ICAP response received from the ++ ICAP server (ICAP headers + ICAP body), including ++ chunking metadata (if any). ++ ++ icap::<bs The size of the ICAP response body received from the ++ ICAP server, excluding chunking metadata (if any). + + icap::tr Transaction response time (in + milliseconds). The timer starts when +@@ -4489,9 +4508,9 @@ + The default ICAP log format, which can be used without an explicit + definition, is called icap_squid: + +-logformat icap_squid %ts.%03tu %6icap::tr %>a %icap::to/%03icap::Hs %icap::<size %icap::rm %icap::ru% %un -/%icap::<A - ++logformat icap_squid %ts.%03tu %6icap::tr %>A %icap::to/%03icap::Hs %icap::<st %icap::rm %icap::ru %un -/%icap::<A - + +- See also: logformat, log_icap, and %adapt::<last_h ++ See also: logformat and %adapt::<last_h + DOC_END + + NAME: logfile_daemon + +=== modified file 'src/client_side.cc' +--- src/client_side.cc 2016-06-18 13:36:07 +0000 ++++ src/client_side.cc 2016-09-16 11:53:28 +0000 +@@ -555,8 +555,6 @@ + aLogEntry->http.method = request->method; + aLogEntry->http.version = request->http_ver; + aLogEntry->hier = request->hier; +- if (request->content_length > 0) // negative when no body or unknown length +- aLogEntry->http.clientRequestSz.payloadData += request->content_length; // XXX: actually adaptedRequest payload size ?? + aLogEntry->cache.extuser = request->extacl_user.termedBuf(); + + // Adapted request, if any, inherits and then collects all the stats, but +@@ -593,6 +591,9 @@ + al->cache.objectSize = loggingEntry()->contentLen(); // payload duplicate ?? with or without TE ? + + al->http.clientRequestSz.header = req_sz; ++ // the virgin request is saved to al->request ++ if (al->request && al->request->body_pipe != NULL) ++ al->http.clientRequestSz.payloadData = al->request->body_pipe->producedSize(); + al->http.clientReplySz.header = out.headers_sz; + // XXX: calculate without payload encoding or headers !! + al->http.clientReplySz.payloadData = out.size - out.headers_sz; // pretend its all un-encoded data for now. +@@ -2744,6 +2745,7 @@ + request->my_addr = conn->clientConnection->local; + request->myportname = conn->port->name; + request->http_ver = http_ver; ++ request->hdr_sz = http->req_sz; + + // Link this HttpRequest to ConnStateData relatively early so the following complex handling can use it + // TODO: this effectively obsoletes a lot of conn->FOO copying. That needs cleaning up later. + +=== modified file 'src/format/Format.cc' +--- src/format/Format.cc 2016-04-07 04:43:45 +0000 ++++ src/format/Format.cc 2016-09-16 11:53:28 +0000 +@@ -309,6 +309,38 @@ + *p = '\0'; + } + ++/// XXX: Misnamed. TODO: Split <h (and this function) to distinguish received ++/// headers from sent headers rather than failing to distinguish requests from responses. ++/// \retval HttpReply sent to the HTTP client (access.log and default context). ++/// \retval HttpReply received (encapsulated) from the ICAP server (icap.log context). ++/// \retval HttpRequest received (encapsulated) from the ICAP server (icap.log context). ++static const HttpMsg * ++actualReplyHeader(const AccessLogEntry::Pointer &al) ++{ ++ const HttpMsg *msg = al->reply; ++#if USE_ADAPTATION ++ // al->icap.reqMethod is methodNone in access.log context ++ if (!msg && al->icap.reqMethod == Adaptation::methodReqmod) ++ msg = al->adapted_request; ++#endif ++ return msg; ++} ++ ++/// XXX: Misnamed. See actualReplyHeader(). ++/// \return HttpRequest or HttpReply for %http::>h. ++static const HttpMsg * ++actualRequestHeader(const AccessLogEntry::Pointer &al) ++{ ++#if USE_ADAPTATION ++ // al->icap.reqMethod is methodNone in access.log context ++ if (al->icap.reqMethod == Adaptation::methodRespmod) { ++ // XXX: for now AccessLogEntry lacks virgin response headers ++ return NULL; ++ } ++#endif ++ return al->request; ++} ++ + void + Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logSequenceNumber) const + { +@@ -547,8 +579,8 @@ + + case LFT_REQUEST_HEADER: + +- if (al->request) +- sb = al->request->header.getByName(fmt->data.header.header); ++ if (const HttpMsg *msg = actualRequestHeader(al)) ++ sb = msg->header.getByName(fmt->data.header.header); + + out = sb.termedBuf(); + +@@ -567,15 +599,15 @@ + + break; + +- case LFT_REPLY_HEADER: +- if (al->reply) +- sb = al->reply->header.getByName(fmt->data.header.header); ++ case LFT_REPLY_HEADER: { ++ if (const HttpMsg *msg = actualReplyHeader(al)) ++ sb = msg->header.getByName(fmt->data.header.header); + + out = sb.termedBuf(); + + quote = 1; +- +- break; ++ } ++ break; + + #if USE_ADAPTATION + case LFT_ADAPTATION_SUM_XACT_TIMES: +@@ -757,8 +789,8 @@ + break; + #endif + case LFT_REQUEST_HEADER_ELEM: +- if (al->request) +- sb = al->request->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator); ++ if (const HttpMsg *msg = actualRequestHeader(al)) ++ sb = msg->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator); + + out = sb.termedBuf(); + +@@ -776,18 +808,27 @@ + + break; + +- case LFT_REPLY_HEADER_ELEM: +- if (al->reply) +- sb = al->reply->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator); ++ case LFT_REPLY_HEADER_ELEM: { ++ if (const HttpMsg *msg = actualReplyHeader(al)) ++ sb = msg->header.getByNameListMember(fmt->data.header.header, fmt->data.header.element, fmt->data.header.separator); + + out = sb.termedBuf(); + + quote = 1; +- +- break; ++ } ++ break; + + case LFT_REQUEST_ALL_HEADERS: +- out = al->headers.request; ++#if USE_ADAPTATION ++ if (al->icap.reqMethod == Adaptation::methodRespmod) { ++ // XXX: since AccessLogEntry::Headers lacks virgin response ++ // headers, do nothing for now ++ out = NULL; ++ } else ++#endif ++ { ++ out = al->headers.request; ++ } + + quote = 1; + +@@ -802,6 +843,10 @@ + + case LFT_REPLY_ALL_HEADERS: + out = al->headers.reply; ++#if USE_ADAPTATION ++ if (!out && al->icap.reqMethod == Adaptation::methodReqmod) ++ out = al->headers.adapted_request; ++#endif + + quote = 1; + + diff --git a/src/patches/squid/squid-3.5-14083.patch b/src/patches/squid/squid-3.5-14083.patch new file mode 100644 index 0000000..1f2b24c --- /dev/null +++ b/src/patches/squid/squid-3.5-14083.patch @@ -0,0 +1,48 @@ +------------------------------------------------------------ +revno: 14083 +revision-id: squid3@treenet.co.nz-20160916185004-ql4nhzwswotza6zw +parent: squid3@treenet.co.nz-20160916115328-0fpefy6pbtfq40cd +author: Eduard Bagdasaryan eduard.bagdasaryan@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Sat 2016-09-17 06:50:04 +1200 +message: + Fix potential ICAP null pointer dereference after rev.14082 + + Adjusted Adaptation::Icap::ModXact::finalizeLogInfo(), fixing possible + "Null pointer dereference". + + Detected by Coverity Scan. Issue 1372977 +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160916185004-ql4nhzwswotza6zw +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 900564075422fd0beeb1a82e5d4f1073b02bb326 +# timestamp: 2016-09-16 18:50:59 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160916115328-\ +# 0fpefy6pbtfq40cd +# +# Begin patch +=== modified file 'src/adaptation/icap/ModXact.cc' +--- src/adaptation/icap/ModXact.cc 2016-09-16 11:53:28 +0000 ++++ src/adaptation/icap/ModXact.cc 2016-09-16 18:50:04 +0000 +@@ -1261,7 +1261,7 @@ + { + HttpRequest *adapted_request_ = NULL; + HttpReply *adapted_reply_ = NULL; +- HttpRequest *virgin_request_ = (virgin.cause ? virgin.cause : dynamic_cast<HttpRequest*>(virgin.header)); ++ HttpRequest *virgin_request_ = const_cast<HttpRequest*>(&virginRequest()); + if (!(adapted_request_ = dynamic_cast<HttpRequest*>(adapted.header))) { + // if the request was not adapted, use virgin request to simplify + // the code further below +@@ -1269,7 +1269,7 @@ + adapted_reply_ = dynamic_cast<HttpReply*>(adapted.header); + } + +- Adaptation::Icap::History::Pointer h = (virgin_request_ ? virgin_request_->icapHistory() : NULL); ++ Adaptation::Icap::History::Pointer h = virgin_request_->icapHistory(); + Must(h != NULL); // ICAPXaction::maybeLog calls only if there is a log + al.icp.opcode = ICP_INVALID; + al.url = h->log_uri.termedBuf(); +