From 85d0bbd627b1427e7151e9be2c943939c60da44f Mon Sep 17 00:00:00 2001 From: MSVSphere Packaging Team Date: Fri, 15 Nov 2024 03:24:42 +0300 Subject: [PATCH] import squid-5.5-14.el9_5.3 --- ...quid-5.5-ignore-wsp-after-chunk-size.patch | 367 ++++++++++++++++++ SOURCES/squid-5.5-ipv6-crash.patch | 113 ++++++ .../squid-5.5-large-upload-buffer-dies.patch | 117 ++++++ SPECS/squid.spec | 39 +- 4 files changed, 628 insertions(+), 8 deletions(-) create mode 100644 SOURCES/squid-5.5-ignore-wsp-after-chunk-size.patch create mode 100644 SOURCES/squid-5.5-ipv6-crash.patch create mode 100644 SOURCES/squid-5.5-large-upload-buffer-dies.patch diff --git a/SOURCES/squid-5.5-ignore-wsp-after-chunk-size.patch b/SOURCES/squid-5.5-ignore-wsp-after-chunk-size.patch new file mode 100644 index 0000000..ea4025f --- /dev/null +++ b/SOURCES/squid-5.5-ignore-wsp-after-chunk-size.patch @@ -0,0 +1,367 @@ +From 8d0ee420a4d91ac7fd97316338f1e28b4b060cbf Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubo=C5=A1=20Uhliarik?= +Date: Thu, 10 Oct 2024 19:26:27 +0200 +Subject: [PATCH 1/6] Ignore whitespace chars after chunk-size + +Previously (before #1498 change), squid was accepting TE-chunked replies +with whitespaces after chunk-size and missing chunk-ext data. After + +It turned out that replies with such whitespace chars are pretty +common and other webservers which can act as forward proxies (e.g. +nginx, httpd...) are accepting them. + +This change will allow to proxy chunked responses from origin server, +which had whitespaces inbetween chunk-size and CRLF. +--- + src/http/one/TeChunkedParser.cc | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc +index 9cce10fdc91..04753395e16 100644 +--- a/src/http/one/TeChunkedParser.cc ++++ b/src/http/one/TeChunkedParser.cc +@@ -125,6 +125,7 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + // Code becomes much simpler when incremental parsing functions throw on + // bad or insufficient input, like in the code below. TODO: Expand up. + try { ++ tok.skipAll(CharacterSet::WSP); // Some servers send SP/TAB after chunk-size + parseChunkExtensions(tok); // a possibly empty chunk-ext list + tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf()); + buf_ = tok.remaining(); + +From 9c8d35f899035fa06021ab3fe6919f892c2f0c6b Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubo=C5=A1=20Uhliarik?= +Date: Fri, 11 Oct 2024 02:06:31 +0200 +Subject: [PATCH 2/6] Added new argument to Http::One::ParseBws() + +Depending on new wsp_only argument in ParseBws() it will be decided +which set of whitespaces characters will be parsed. If wsp_only is set +to true, only SP and HTAB chars will be parsed. + +Also optimized number of ParseBws calls. +--- + src/http/one/Parser.cc | 4 ++-- + src/http/one/Parser.h | 3 ++- + src/http/one/TeChunkedParser.cc | 13 +++++++++---- + src/http/one/TeChunkedParser.h | 2 +- + 4 files changed, 14 insertions(+), 8 deletions(-) + +diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc +index b1908316a0b..01d7e3bc0e8 100644 +--- a/src/http/one/Parser.cc ++++ b/src/http/one/Parser.cc +@@ -273,9 +273,9 @@ Http::One::ErrorLevel() + + // BWS = *( SP / HTAB ) ; WhitespaceCharacters() may relax this RFC 7230 rule + void +-Http::One::ParseBws(Parser::Tokenizer &tok) ++Http::One::ParseBws(Parser::Tokenizer &tok, const bool wsp_only) + { +- const auto count = tok.skipAll(Parser::WhitespaceCharacters()); ++ const auto count = tok.skipAll(wsp_only ? CharacterSet::WSP : Parser::WhitespaceCharacters()); + + if (tok.atEnd()) + throw InsufficientInput(); // even if count is positive +diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h +index d9a0ac8c273..08200371cd6 100644 +--- a/src/http/one/Parser.h ++++ b/src/http/one/Parser.h +@@ -163,8 +163,9 @@ class Parser : public RefCountable + }; + + /// skips and, if needed, warns about RFC 7230 BWS ("bad" whitespace) ++/// \param wsp_only force skipping of whitespaces only, don't consider skipping relaxed delimeter chars + /// \throws InsufficientInput when the end of BWS cannot be confirmed +-void ParseBws(Parser::Tokenizer &); ++void ParseBws(Parser::Tokenizer &, const bool wsp_only = false); + + /// the right debugs() level for logging HTTP violation messages + int ErrorLevel(); +diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc +index 04753395e16..41e1e5ddaea 100644 +--- a/src/http/one/TeChunkedParser.cc ++++ b/src/http/one/TeChunkedParser.cc +@@ -125,8 +125,11 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + // Code becomes much simpler when incremental parsing functions throw on + // bad or insufficient input, like in the code below. TODO: Expand up. + try { +- tok.skipAll(CharacterSet::WSP); // Some servers send SP/TAB after chunk-size +- parseChunkExtensions(tok); // a possibly empty chunk-ext list ++ // A possibly empty chunk-ext list. If no chunk-ext has been found, ++ // try to skip trailing BWS, because some servers send "chunk-size BWS CRLF". ++ if (!parseChunkExtensions(tok)) ++ ParseBws(tok, true); ++ + tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf()); + buf_ = tok.remaining(); + parsingStage_ = theChunkSize ? Http1::HTTP_PARSE_CHUNK : Http1::HTTP_PARSE_MIME; +@@ -140,20 +143,22 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + + /// Parses the chunk-ext list (RFC 9112 section 7.1.1: + /// chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] ) +-void ++bool + Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &callerTok) + { ++ bool foundChunkExt = false; + do { + auto tok = callerTok; + + ParseBws(tok); // Bug 4492: IBM_HTTP_Server sends SP after chunk-size + + if (!tok.skip(';')) +- return; // reached the end of extensions (if any) ++ return foundChunkExt; // reached the end of extensions (if any) + + parseOneChunkExtension(tok); + buf_ = tok.remaining(); // got one extension + callerTok = tok; ++ foundChunkExt = true; + } while (true); + } + +diff --git a/src/http/one/TeChunkedParser.h b/src/http/one/TeChunkedParser.h +index 02eacd1bb89..8c5d4bb4cba 100644 +--- a/src/http/one/TeChunkedParser.h ++++ b/src/http/one/TeChunkedParser.h +@@ -71,7 +71,7 @@ class TeChunkedParser : public Http1::Parser + private: + bool parseChunkSize(Tokenizer &tok); + bool parseChunkMetadataSuffix(Tokenizer &); +- void parseChunkExtensions(Tokenizer &); ++ bool parseChunkExtensions(Tokenizer &); + void parseOneChunkExtension(Tokenizer &); + bool parseChunkBody(Tokenizer &tok); + bool parseChunkEnd(Tokenizer &tok); + +From 81e67f97f9c386bdd0bb4a5e182395c46adb70ad Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubo=C5=A1=20Uhliarik?= +Date: Fri, 11 Oct 2024 02:44:33 +0200 +Subject: [PATCH 3/6] Fix typo in Parser.h + +--- + src/http/one/Parser.h | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h +index 08200371cd6..3ef4c5f7752 100644 +--- a/src/http/one/Parser.h ++++ b/src/http/one/Parser.h +@@ -163,7 +163,7 @@ class Parser : public RefCountable + }; + + /// skips and, if needed, warns about RFC 7230 BWS ("bad" whitespace) +-/// \param wsp_only force skipping of whitespaces only, don't consider skipping relaxed delimeter chars ++/// \param wsp_only force skipping of whitespaces only, don't consider skipping relaxed delimiter chars + /// \throws InsufficientInput when the end of BWS cannot be confirmed + void ParseBws(Parser::Tokenizer &, const bool wsp_only = false); + + +From a0d4fe1794e605f8299a5c118c758a807453f016 Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Thu, 10 Oct 2024 22:39:42 -0400 +Subject: [PATCH 4/6] Bug 5449 is a regression of Bug 4492! + +Both bugs deal with "chunk-size SP+ CRLF" use cases. Bug 4492 had _two_ +spaces after chunk-size, which answers one of the PR review questions: +Should we skip just one space? No, we should not. + +The lines moved around in many commits, but I believe this regression +was introduced in commit 951013d0 because that commit stopped consuming +partially parsed chunk-ext sequences. That consumption was wrong, but it +had a positive side effect -- fixing Bug 4492... +--- + src/http/one/TeChunkedParser.cc | 10 +++++----- + 1 file changed, 5 insertions(+), 5 deletions(-) + +diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc +index 41e1e5ddaea..aa4a840fdcf 100644 +--- a/src/http/one/TeChunkedParser.cc ++++ b/src/http/one/TeChunkedParser.cc +@@ -125,10 +125,10 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + // Code becomes much simpler when incremental parsing functions throw on + // bad or insufficient input, like in the code below. TODO: Expand up. + try { +- // A possibly empty chunk-ext list. If no chunk-ext has been found, +- // try to skip trailing BWS, because some servers send "chunk-size BWS CRLF". +- if (!parseChunkExtensions(tok)) +- ParseBws(tok, true); ++ // Bug 4492: IBM_HTTP_Server sends SP after chunk-size ++ ParseBws(tok, true); ++ ++ parseChunkExtensions(tok); + + tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf()); + buf_ = tok.remaining(); +@@ -150,7 +150,7 @@ Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &callerTok) + do { + auto tok = callerTok; + +- ParseBws(tok); // Bug 4492: IBM_HTTP_Server sends SP after chunk-size ++ ParseBws(tok); + + if (!tok.skip(';')) + return foundChunkExt; // reached the end of extensions (if any) + +From f837f5ff61301a17008f16ce1fb793c2abf19786 Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Thu, 10 Oct 2024 23:06:42 -0400 +Subject: [PATCH 5/6] fixup: Fewer conditionals/ifs and more explicit spelling + +... to draw code reader attention when something unusual is going on. +--- + src/http/one/Parser.cc | 22 ++++++++++++++++++---- + src/http/one/Parser.h | 10 ++++++++-- + src/http/one/TeChunkedParser.cc | 14 ++++++-------- + src/http/one/TeChunkedParser.h | 2 +- + 4 files changed, 33 insertions(+), 15 deletions(-) + +diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc +index 01d7e3bc0e8..d3937e5e96b 100644 +--- a/src/http/one/Parser.cc ++++ b/src/http/one/Parser.cc +@@ -271,11 +271,12 @@ Http::One::ErrorLevel() + return Config.onoff.relaxed_header_parser < 0 ? DBG_IMPORTANT : 5; + } + +-// BWS = *( SP / HTAB ) ; WhitespaceCharacters() may relax this RFC 7230 rule +-void +-Http::One::ParseBws(Parser::Tokenizer &tok, const bool wsp_only) ++/// common part of ParseBws() and ParseStrctBws() ++namespace Http::One { ++static void ++ParseBws_(Parser::Tokenizer &tok, const CharacterSet &bwsChars) + { +- const auto count = tok.skipAll(wsp_only ? CharacterSet::WSP : Parser::WhitespaceCharacters()); ++ const auto count = tok.skipAll(bwsChars); + + if (tok.atEnd()) + throw InsufficientInput(); // even if count is positive +@@ -290,4 +291,17 @@ Http::One::ParseBws(Parser::Tokenizer &tok, const bool wsp_only) + + // success: no more BWS characters expected + } ++} // namespace Http::One ++ ++void ++Http::One::ParseBws(Parser::Tokenizer &tok) ++{ ++ ParseBws_(tok, CharacterSet::WSP); ++} ++ ++void ++Http::One::ParseStrictBws(Parser::Tokenizer &tok) ++{ ++ ParseBws_(tok, Parser::WhitespaceCharacters()); ++} + +diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h +index 3ef4c5f7752..49e399de546 100644 +--- a/src/http/one/Parser.h ++++ b/src/http/one/Parser.h +@@ -163,9 +163,15 @@ class Parser : public RefCountable + }; + + /// skips and, if needed, warns about RFC 7230 BWS ("bad" whitespace) +-/// \param wsp_only force skipping of whitespaces only, don't consider skipping relaxed delimiter chars + /// \throws InsufficientInput when the end of BWS cannot be confirmed +-void ParseBws(Parser::Tokenizer &, const bool wsp_only = false); ++/// \sa WhitespaceCharacters() for the definition of BWS characters ++/// \sa ParseStrictBws() that avoids WhitespaceCharacters() uncertainties ++void ParseBws(Parser::Tokenizer &); ++ ++/// Like ParseBws() but only skips CharacterSet::WSP characters. This variation ++/// must be used if the next element may start with CR or any other character ++/// from RelaxedDelimiterCharacters(). ++void ParseStrictBws(Parser::Tokenizer &); + + /// the right debugs() level for logging HTTP violation messages + int ErrorLevel(); +diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc +index aa4a840fdcf..859471b8c77 100644 +--- a/src/http/one/TeChunkedParser.cc ++++ b/src/http/one/TeChunkedParser.cc +@@ -125,11 +125,11 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + // Code becomes much simpler when incremental parsing functions throw on + // bad or insufficient input, like in the code below. TODO: Expand up. + try { +- // Bug 4492: IBM_HTTP_Server sends SP after chunk-size +- ParseBws(tok, true); +- +- parseChunkExtensions(tok); ++ // Bug 4492: IBM_HTTP_Server sends SP after chunk-size. ++ // No ParseBws() here because it may consume CR required further below. ++ ParseStrictBws(tok); + ++ parseChunkExtensions(tok); // a possibly empty chunk-ext list + tok.skipRequired("CRLF after [chunk-ext]", Http1::CrLf()); + buf_ = tok.remaining(); + parsingStage_ = theChunkSize ? Http1::HTTP_PARSE_CHUNK : Http1::HTTP_PARSE_MIME; +@@ -143,22 +143,20 @@ Http::One::TeChunkedParser::parseChunkMetadataSuffix(Tokenizer &tok) + + /// Parses the chunk-ext list (RFC 9112 section 7.1.1: + /// chunk-ext = *( BWS ";" BWS chunk-ext-name [ BWS "=" BWS chunk-ext-val ] ) +-bool ++void + Http::One::TeChunkedParser::parseChunkExtensions(Tokenizer &callerTok) + { +- bool foundChunkExt = false; + do { + auto tok = callerTok; + + ParseBws(tok); + + if (!tok.skip(';')) +- return foundChunkExt; // reached the end of extensions (if any) ++ return; // reached the end of extensions (if any) + + parseOneChunkExtension(tok); + buf_ = tok.remaining(); // got one extension + callerTok = tok; +- foundChunkExt = true; + } while (true); + } + +diff --git a/src/http/one/TeChunkedParser.h b/src/http/one/TeChunkedParser.h +index 8c5d4bb4cba..02eacd1bb89 100644 +--- a/src/http/one/TeChunkedParser.h ++++ b/src/http/one/TeChunkedParser.h +@@ -71,7 +71,7 @@ class TeChunkedParser : public Http1::Parser + private: + bool parseChunkSize(Tokenizer &tok); + bool parseChunkMetadataSuffix(Tokenizer &); +- bool parseChunkExtensions(Tokenizer &); ++ void parseChunkExtensions(Tokenizer &); + void parseOneChunkExtension(Tokenizer &); + bool parseChunkBody(Tokenizer &tok); + bool parseChunkEnd(Tokenizer &tok); + +From f79936a234e722adb2dd08f31cf6019d81ee712c Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Thu, 10 Oct 2024 23:31:08 -0400 +Subject: [PATCH 6/6] fixup: Deadly typo + +--- + src/http/one/Parser.cc | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc +index d3937e5e96b..7403a9163a2 100644 +--- a/src/http/one/Parser.cc ++++ b/src/http/one/Parser.cc +@@ -296,12 +296,12 @@ ParseBws_(Parser::Tokenizer &tok, const CharacterSet &bwsChars) + void + Http::One::ParseBws(Parser::Tokenizer &tok) + { +- ParseBws_(tok, CharacterSet::WSP); ++ ParseBws_(tok, Parser::WhitespaceCharacters()); + } + + void + Http::One::ParseStrictBws(Parser::Tokenizer &tok) + { +- ParseBws_(tok, Parser::WhitespaceCharacters()); ++ ParseBws_(tok, CharacterSet::WSP); + } + + diff --git a/SOURCES/squid-5.5-ipv6-crash.patch b/SOURCES/squid-5.5-ipv6-crash.patch new file mode 100644 index 0000000..c47f89d --- /dev/null +++ b/SOURCES/squid-5.5-ipv6-crash.patch @@ -0,0 +1,113 @@ +From a0a9e6dc69d0c7b9ba237702b4c5020abc7ad1f8 Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Sat, 4 Nov 2023 00:30:42 +0000 +Subject: [PATCH] Bug 5154: Do not open IPv6 sockets when IPv6 is disabled + (#1567) + +... but allow basic IPv6 manipulations like getSockAddr(). + + Address.cc:663 getAddrInfo() assertion failed: false + +Squids receives IPv6 addresses from traffic, configuration, or +hard-coded constants even when ./configured with --disable-ipv6 or when +IPv6 support was automatically disabled at startup after failing IPv6 +tests. To handle IPv6 correctly, such Squids must support basic IPv6 +operations like recognizing an IPv6 address in a request-target or +reporting an unsolicited IPv6 DNS record. At least for now, such Squids +must also correctly parse configuration-related IPv6 addresses. + +All those activities rely on various low-level operations like filling +addrinfo structure with IP address information. Since 2012 commit +c5fbbc7, Ip::Address::getAddrInfo() was failing for IPv6 addresses when +Ip::EnableIpv6 was falsy. That change correctly recognized[^1] the need +for such Squids to handle IPv6, but to support basic operations, we need +to reject IPv6 addresses at a higher level and without asserting. + +That high-level rejection work is ongoing, but initial attempts have +exposed difficult problems that will take time to address. For now, we +just avoid the assertion while protecting IPv6-disabled Squid from +listening on or opening connections to IPv6 addresses. Since Squid +already expects (and usually correctly handles) socket opening failures, +disabling those operations is better than failing in low-level IP +manipulation code. + +The overall IPv6 posture of IPv6-disabled Squids that lack http_access +or other rules to deny IPv6 requests will change: This fix exposes more +of IPv6-disabled Squid code to IPv6 addresses. It is possible that such +exposure will make some IPv6 resources inside Squid (e.g., a previously +cached HTTP response) accessible to external requests. Squids will not +open or accept IPv6 connections but may forward requests with raw IPv6 +targets to IPv4 cache_peers. Whether these and similar behavior changes +are going to be permanent is open for debate, but even if they are +temporary, they are arguably better than the corresponding assertions. + +These changes do not effect IPv6-enabled Squids. + +The assertion in IPv6-disabled Squid was reported by Joshua Rogers at +https://megamansec.github.io/Squid-Security-Audit/ipv6-assert.html where +it was filed as "Assertion on IPv6 Host Requests with --disable-ipv6". + +[^1]: https://bugs.squid-cache.org/show_bug.cgi?id=3593#c1 +--- + src/comm.cc | 6 ++++++ + src/ip/Address.cc | 2 +- + src/ip/Intercept.cc | 8 ++++++++ + 3 files changed, 15 insertions(+), 1 deletion(-) + +diff --git a/src/comm.cc b/src/comm.cc +index 4659955b011..271ba04d4da 100644 +--- a/src/comm.cc ++++ b/src/comm.cc +@@ -344,6 +344,12 @@ comm_openex(int sock_type, + /* Create socket for accepting new connections. */ + ++ statCounter.syscalls.sock.sockets; + ++ if (!Ip::EnableIpv6 && addr.isIPv6()) { ++ debugs(50, 2, "refusing to open an IPv6 socket when IPv6 support is disabled: " << addr); ++ errno = ENOTSUP; ++ return -1; ++ } ++ + /* Setup the socket addrinfo details for use */ + addr.getAddrInfo(AI); + AI->ai_socktype = sock_type; +diff --git a/src/ip/Address.cc b/src/ip/Address.cc +index b6f810bfc25..ae6db37da5e 100644 +--- a/src/ip/Address.cc ++++ b/src/ip/Address.cc +@@ -623,7 +623,7 @@ Ip::Address::getAddrInfo(struct addrinfo *&dst, int force) const + && dst->ai_protocol == 0) + dst->ai_protocol = IPPROTO_UDP; + +- if (force == AF_INET6 || (force == AF_UNSPEC && Ip::EnableIpv6 && isIPv6()) ) { ++ if (force == AF_INET6 || (force == AF_UNSPEC && isIPv6()) ) { + dst->ai_addr = (struct sockaddr*)new sockaddr_in6; + + memset(dst->ai_addr,0,sizeof(struct sockaddr_in6)); +diff --git a/src/ip/Intercept.cc b/src/ip/Intercept.cc +index 1a5e2d15af1..a8522efaac0 100644 +--- a/src/ip/Intercept.cc ++++ b/src/ip/Intercept.cc +@@ -15,6 +15,7 @@ + #include "comm/Connection.h" + #include "fde.h" + #include "ip/Intercept.h" ++#include "ip/tools.h" + #include "src/tools.h" + + #include +@@ -430,6 +431,13 @@ Ip::Intercept::ProbeForTproxy(Ip::Address &test) + + debugs(3, 3, "Detect TPROXY support on port " << test); + ++ if (!Ip::EnableIpv6 && test.isIPv6() && !test.setIPv4()) { ++ debugs(3, DBG_CRITICAL, "Cannot use TPROXY for " << test << " because IPv6 support is disabled"); ++ if (doneSuid) ++ leave_suid(); ++ return false; ++ } ++ + int tos = 1; + int tmp_sock = -1; + + diff --git a/SOURCES/squid-5.5-large-upload-buffer-dies.patch b/SOURCES/squid-5.5-large-upload-buffer-dies.patch new file mode 100644 index 0000000..459d528 --- /dev/null +++ b/SOURCES/squid-5.5-large-upload-buffer-dies.patch @@ -0,0 +1,117 @@ +From 4d6dd3ddba5e850a42c86d8233735165a371c31c Mon Sep 17 00:00:00 2001 +From: Alex Rousskov +Date: Sun, 1 Sep 2024 00:39:34 +0000 +Subject: [PATCH] Bug 5405: Large uploads fill request buffer and die (#1887) + + maybeMakeSpaceAvailable: request buffer full + ReadNow: ... size 0, retval 0, errno 0 + terminateAll: 1/1 after ERR_CLIENT_GONE/WITH_CLIENT + %Ss=TCP_MISS_ABORTED + +This bug is triggered by a combination of the following two conditions: + +* HTTP client upload fills Squid request buffer faster than it is + drained by an origin server, cache_peer, or REQMOD service. The buffer + accumulates 576 KB (default 512 KB client_request_buffer_max_size + 64 + KB internal "pipe" buffer). + +* The affected server or service consumes a few bytes after the critical + accumulation is reached. In other words, the bug cannot be triggered + if nothing is consumed after the first condition above is met. + +Comm::ReadNow() must not be called with a full buffer: Related +FD_READ_METHOD() code cannot distinguish "received EOF" from "had no +buffer space" outcomes. Server::readSomeData() tried to prevent such +calls, but the corresponding check had two problems: + +* The check had an unsigned integer underflow bug[^1] that made it + ineffective when inBuf length exceeded Config.maxRequestBufferSize. + That length could exceed the limit due to reconfiguration and when + inBuf space size first grew outside of maybeMakeSpaceAvailable() + protections (e.g., during an inBuf.c_str() call) and then got filled + with newly read data. That growth started happening after 2020 commit + 1dfbca06 optimized SBuf::cow() to merge leading and trailing space. + Prior to that commit, Bug 5405 could probably only affect Squid + reconfigurations that lower client_request_buffer_max_size. + +* The check was separated from the ReadNow() call it was meant to + protect. While ConnStateData was waiting for the socket to become + ready for reading, various asynchronous events could alter inBuf or + Config.maxRequestBufferSize. + +This change fixes both problems. + +This change also fixes Squid Bug 5214. + +[^1]: That underflow bug was probably introduced in 2015 commit 4d1376d7 +while trying to emulate the original "do not read less than two bytes" +ConnStateData::In::maybeMakeSpaceAvailable() condition. That condition +itself looks like a leftover from manual zero-terminated input buffer +days that ended with 2014 commit e7287625. It is now removed. +--- + +diff --git a/src/servers/Server.cc b/src/servers/Server.cc +index 70fd10b..dd20619 100644 +--- a/src/servers/Server.cc ++++ b/src/servers/Server.cc +@@ -83,16 +83,25 @@ Server::maybeMakeSpaceAvailable() + debugs(33, 4, "request buffer full: client_request_buffer_max_size=" << Config.maxRequestBufferSize); + } + ++bool ++Server::mayBufferMoreRequestBytes() const ++{ ++ // TODO: Account for bodyPipe buffering as well. ++ if (inBuf.length() >= Config.maxRequestBufferSize) { ++ debugs(33, 4, "no: " << inBuf.length() << '-' << Config.maxRequestBufferSize << '=' << (inBuf.length() - Config.maxRequestBufferSize)); ++ return false; ++ } ++ debugs(33, 7, "yes: " << Config.maxRequestBufferSize << '-' << inBuf.length() << '=' << (Config.maxRequestBufferSize - inBuf.length())); ++ return true; ++} ++ + void + Server::readSomeData() + { + if (reading()) + return; + +- debugs(33, 4, clientConnection << ": reading request..."); +- +- // we can only read if there is more than 1 byte of space free +- if (Config.maxRequestBufferSize - inBuf.length() < 2) ++ if (!mayBufferMoreRequestBytes()) + return; + + typedef CommCbMemFunT Dialer; +@@ -123,7 +132,16 @@ Server::doClientRead(const CommIoCbParams &io) + * Plus, it breaks our lame *HalfClosed() detection + */ + ++ // mayBufferMoreRequestBytes() was true during readSomeData(), but variables ++ // like Config.maxRequestBufferSize may have changed since that check ++ if (!mayBufferMoreRequestBytes()) { ++ // XXX: If we avoid Comm::ReadNow(), we should not Comm::Read() again ++ // when the wait is over; resume these doClientRead() checks instead. ++ return; // wait for noteMoreBodySpaceAvailable() or a similar inBuf draining event ++ } + maybeMakeSpaceAvailable(); ++ Assure(inBuf.spaceSize()); ++ + CommIoCbParams rd(this); // will be expanded with ReadNow results + rd.conn = io.conn; + switch (Comm::ReadNow(rd, inBuf)) { +diff --git a/src/servers/Server.h b/src/servers/Server.h +index ef105f5..6e549b3 100644 +--- a/src/servers/Server.h ++++ b/src/servers/Server.h +@@ -119,6 +119,9 @@ protected: + /// abort any pending transactions and prevent new ones (by closing) + virtual void terminateAll(const Error &, const LogTagsErrors &) = 0; + ++ /// whether client_request_buffer_max_size allows inBuf.length() increase ++ bool mayBufferMoreRequestBytes() const; ++ + void doClientRead(const CommIoCbParams &io); + void clientWriteDone(const CommIoCbParams &io); + diff --git a/SPECS/squid.spec b/SPECS/squid.spec index 7bc3b3b..246e7d7 100644 --- a/SPECS/squid.spec +++ b/SPECS/squid.spec @@ -2,7 +2,7 @@ Name: squid Version: 5.5 -Release: 13%{?dist} +Release: 14%{?dist}.3 Summary: The Squid proxy caching server Epoch: 7 # See CREDITS for breakdown of non GPLv2+ code @@ -46,6 +46,10 @@ Patch207: squid-5.0.6-active-ftp.patch Patch208: squid-5.1-test-store-cppsuite.patch # https://bugzilla.redhat.com/show_bug.cgi?id=2231827 Patch209: squid-5.5-halfclosed.patch +# https://issues.redhat.com/browse/RHEL-30352 +Patch210: squid-5.5-ipv6-crash.patch +# https://issues.redhat.com/browse/RHEL-12356 +Patch211: squid-5.5-large-upload-buffer-dies.patch # Security patches # https://bugzilla.redhat.com/show_bug.cgi?id=2100721 @@ -78,7 +82,9 @@ Patch513: squid-5.5-CVE-2024-25111.patch Patch514: squid-5.5-CVE-2024-37894.patch # https://bugzilla.redhat.com/show_bug.cgi?id=2260051 Patch515: squid-5.5-CVE-2024-23638.patch - +# Regression caused by squid-5.5-CVE-2023-46846.patch +# Upstream PR: https://github.com/squid-cache/squid/pull/1914 +Patch516: squid-5.5-ignore-wsp-after-chunk-size.patch # cache_swap.sh Requires: bash gawk @@ -96,8 +102,6 @@ BuildRequires: openssl-devel BuildRequires: krb5-devel # time_quota requires TrivialDB BuildRequires: libtdb-devel -# ESI support requires Expat & libxml2 -BuildRequires: expat-devel libxml2-devel # TPROXY requires libcap, and also increases security somewhat BuildRequires: libcap-devel # eCAP support @@ -153,6 +157,8 @@ lookup program (dnsserver), a program for retrieving FTP data %patch207 -p1 -b .active-ftp %patch208 -p1 -b .test-store-cpp %patch209 -p1 -b .halfclosed +%patch210 -p1 -b .ipv6-crash +%patch211 -p1 -b .large-upload-buffer-dies %patch501 -p1 -b .CVE-2021-46784 %patch502 -p1 -b .CVE-2022-41318 @@ -169,6 +175,7 @@ lookup program (dnsserver), a program for retrieving FTP data %patch513 -p1 -b .CVE-2024-25111 %patch514 -p1 -b .CVE-2024-37894 %patch515 -p1 -b .CVE-2024-23638 +%patch516 -p1 -b .ignore-wsp-chunk-sz # https://bugzilla.redhat.com/show_bug.cgi?id=1679526 @@ -212,7 +219,7 @@ sed -i 's|@SYSCONFDIR@/squid.conf.documented|%{_pkgdocdir}/squid.conf.documented --enable-storeio="aufs,diskd,ufs,rock" \ --enable-diskio \ --enable-wccpv2 \ - --enable-esi \ + --disable-esi \ --enable-ecap \ --with-aio \ --with-default-user="squid" \ @@ -396,12 +403,28 @@ fi %changelog -* Mon Jul 01 2024 Luboš Uhliarik - 7:5.5-13 -- Resolves: RHEL-45056 - squid: Out-of-bounds write error may lead to Denial of +* Thu Nov 07 2024 Luboš Uhliarik - 7:5.5-14.3 +- Disable ESI support +- Resolves: RHEL-65076 - CVE-2024-45802 squid: Denial of Service processing ESI + response content + +* Wed Oct 23 2024 Luboš Uhliarik - 7:5.5-14.2 +- Resolves: RHEL-64425 TCP_MISS_ABORTED/100 erros when uploading + +* Mon Oct 14 2024 Luboš Uhliarik - 7:5.5-14.1 +- Resolves: RHEL-62332 - (Regression) Transfer-encoding:chunked data is not sent + to the client in its complementary + +* Mon Jul 01 2024 Luboš Uhliarik - 7:5.5-14 +- Resolves: RHEL-45057 - squid: Out-of-bounds write error may lead to Denial of Service (CVE-2024-37894) -- Resolves: RHEL-45643 - squid: vulnerable to a Denial of Service attack against +- Resolves: RHEL-22594 - squid: vulnerable to a Denial of Service attack against Cache Manager error responses (CVE-2024-23638) +* Thu May 09 2024 Luboš Uhliarik - 7:5.5-13 +- Resolves: RHEL-30352 - squid v5 crashes with SIGABRT when ipv6 is disabled + at kernel level but it is asked to connect to an ipv6 address by a client + * Tue Mar 19 2024 Luboš Uhliarik - 7:5.5-12 - Resolves: RHEL-28530 - squid: Denial of Service in HTTP Chunked Decoding (CVE-2024-25111)