Alex Rousskov [Thu, 30 Jul 2015 03:51:08 +0000 (20:51 -0700)]
Do not blindly forward cache peer CONNECT responses.
Squid blindly forwards cache peer CONNECT responses to clients. This
may break things if the peer responds with something like HTTP 403
(Forbidden) and keeps the connection with Squid open:
- The client application issues a CONNECT request.
- Squid forwards this request to a cache peer.
- Cache peer correctly responds back with a "403 Forbidden".
- Squid does not parse cache peer response and
just forwards it as if it was a Squid response to the client.
- The TCP connections are not closed.
At this stage, Squid is unaware that the CONNECT request has failed. All
subsequent requests on the user agent TCP connection are treated as
tunnelled traffic. Squid is forwarding these requests to the peer on the
TCP connection previously used for the 403-ed CONNECT request, without
proper processing. The additional headers which should have been applied
by Squid to these requests are not applied, and the requests are being
forwarded to the cache peer even though the Squid configuration may
state that these requests must go directly to the origin server.
This fixes Squid to parse cache peer responses, and if an error response
found, respond with "502 Bad Gateway" to the client and close the
connections.
Backport by Raphaël Hertzog based on original work by Alex Rousskov
Protect against buffer overrun in DNS query generation
see SQUID-2013:2.
This bug has been present as long as the internal DNS component however
most code reaching this point is passing through URL validation first.
With Squid-3.2 Host header verification using DNS directly we may have
problems.
Fix build with GCC 4.7 (and probably other C++11 compilers).
User-defined literals introduced by C++11 break some previously valid
code, resulting in errors when building with GCC v4.7. For example:
error: unable to find string literal operator 'operator"" PRIu64'
In particular, whitespace is now needed after a string literal and
before something that could be a valid user-defined literal. See
"User-defined literals and whitespace" section at [1] for more details.
The patch adds spaces between string literals and macros.
Amos Jeffries [Thu, 2 Aug 2012 11:43:53 +0000 (05:43 -0600)]
Support -DFAILURE_MODE_TIME=n compiler flag
This value determins at compile-time how long Squid spends in HIT-only
mode after the failure ratio goes over 1.0. see checkFailureRatio() in
src/client_side_request.cc for details on the ratio.
This flag is supported to remove the need for patching when alteration
is required.
Alex Rousskov [Sat, 21 Jul 2012 04:12:30 +0000 (22:12 -0600)]
Account for Store disk client quota when bandwidth-limiting the server.
It is not clear why the store client type matters when
MemObject::mostBytesAllowed() is trying to find the maximum delay pool
quota for reading from the next hop HTTP server. Whether the client(s)
are reading from disk or RAM, the corresponding server-side bandwidth
ought to be limited.
This code was removed as a part of bug 3462 investigation, but it is not
needed to fix bug 3462.
Allow StoreEntry::bytesWanted() API to ignore delay pools. Use that
feature when shoveling adapted response body from ICAP/eCAP BodyPipe
into Store.
If we do not ignore delay pools in
ServerStateData::handleMoreAdaptedBodyAvailable() context, and our pool
quota is exhausted, we will stop reading from the adaptation BodyPipe,
but there is no code/API to notify a BodyPipe reader when the quota
becomes available again. With no reads from the pipe, the ICAP service
stops sending more adapted body data and possibly stops reading the
virgin body data from Squid, resulting in a stuck ICAP RESPMOD and the
master HTTP transaction.
We do need to call StoreEntry::bytesWanted() in this context because
otherwise Squid may run out of RAM (Squid bug #2619). The fix for that
problem inadvertently created this bug when delay pools were enabled.
Long-term, a "kick BodyPipe consumer when there is quota" code should be
added, and delay pools should be enabled for ICAP responses, but with
enough knobs for admins to disable ICAP pooling where needed. Most ICAP
environments will probably want to disable bandwidth controls because
ICAP service traffic is usually "local".
Removed StoreEntry::bytesWanted() TRY_FWD_HDR_WAIT guard that disabled
delay pool application until the final HTTP response headers are
received (HttpStateData::haveParsedReplyHeaders() is called). Nobody
could fully explain why that condition was really needed (and we
obviously want to limit bandwidth consumption at all response processing
stages, in general). The now-removed guard was bypassing delay pool
checks for virgin HTTP response (until the ICAP server responded with
adapted headers), causing bandwidth overuse.
Possibly fixes or helps with Squid bug #2606 as well.
Bug 2976: squid reports ERR_INVALID_URL for transparently captured requests when reconfiguring
During reconfigure the configured port config objects in http_port_list
may deleted so it is not safe to use them while processing Http requests.
For this reason inside prepareTransparentURL (file client_side.cc) function
the protocol was hard-coded to "http" instead of read it from the related
port config object.
But this is breaks the intercepted https traffic.
This patch:
1. Inside prepareTransparentURL read the protocol from the related
port config object
2. add_http_port() locks the new port pointer before linking it.
3. parse_*() locks the new port pointer before linking it.
4. free_*() unlocks the old port pointer before unlinking
it. It does not delete the old pointer.
This patch also discussed in squid-dev user mailing list in
"Re: [PATCH] Squid host rewrite for intercepted https requests"
thread.
Alex Rousskov [Tue, 29 May 2012 11:35:09 +0000 (05:35 -0600)]
Bug 3466: Adaptation stuck on last single-byte body piece
Changed StoreEntry::bytesWanted(range) to return range.end when the entry can
accommodate range.end bytes. This makes it possible to use that method for
single-byte ranges. Old code returned zero for such ranges, which was
difficult to distinguish from situations where no bytes were wanted at all.
TODO: The StoreEntry::bytesWanted(range) API is left undocumented because it
seems to be slightly broken and/or inconsistent with callers and with the
DelayId::bytesWanted(min, max) API. AFAICT, we should convert
StoreEntry::bytesWanted API from range-based to min/max-based or even just
max-based.
Store Entry API does not use the lower end of the range (except for the
now-removed assertion that the range is not empty). I suspect that Store API
was meant to be used with (first, last+1) "byte position" parameters (returning
the number of bytes wanted) while the DelayId API was meant to be used with
(min, max) "number of bytes" parameters. However, StoreEntry::bytesWanted
implementation does not follow this assumption so perhaps my speculation is
wrong and there are more problems, including this change.
Amos Jeffries [Fri, 23 Mar 2012 03:37:46 +0000 (21:37 -0600)]
Bug 3504: Regression: clientside_tos fails to mark traffic
revison squid-3.1-10302 removing clientside_tos was a mistake.
The actual problem was portage of ssl_bump access control wrongly
dropping the clientside_tos_done flag.
Alex Rousskov [Wed, 7 Mar 2012 01:09:59 +0000 (18:09 -0700)]
Better helper-to-Squid buffer size management.
The minimum buffer size is reduced from 8KB to 4KB after a squid-dev
discussion to prevent wasting of "several hundred KB of unused permanent
memory on some installations".
We now increase the buffer if we cannot parse the helper response message.
The maximum buffer size is now 32KB. This should be enough for all known
helper responses.
We now warn if the read buffer reaches its capacity and kill the offending
helper explicitly. An increase in maximum buffer capacity to 32KB should make
such events rare.
Motivation: ssl_crtd helper may produce responses exceeding 9907 bytes in size
(and possibly much larger if multiple chained certificates need to be returned
to Squid). The old helper.cc code would fill the read buffer completely,
schedule a read for zero bytes, receive zero bytes, declare an EOF condition,
and close the stream (which kills ssl_crtd). Due to insufficient information
logged, the observable symptoms were pretty much the same as if ssl_crtd
closed the stream first, indicating a ssl_crtd bug.
HONDA Hirofumi [Sat, 3 Mar 2012 23:50:50 +0000 (16:50 -0700)]
Bug 3502: client timeout uses server-side read_timeout, not request_timeout
Also adjusts request_timeout description in squid.conf to clarify that
request_timeout applies to receiving complete HTTP request headers and not
just the first header byte or body. We reset the connection timeout to
clientLifetimeTimeout after parsing request headers.
https_port was correctly using Config.Timeout.request already.
Alex Rousskov [Sat, 4 Feb 2012 05:46:26 +0000 (22:46 -0700)]
Bug 3441: Part 1: Minimize cache size corruption by malformed swap.state.
If swap.state gets corrupted, a single entry with bogus size value will screw
up Squid idea of the current cache size. A newly added StoreSwapLogData sane()
method attempts to minimize the chance of corruption by ignoring log entries
with obviously bogus values.
However, without expensive size checks (-S or "Does the log entry matches the
actual cache file size?"), it is not possible to reliably detect all bogus log
entries.
If Squid gets a wrong idea of the current cache size, it may either cache too
much (and possibly run out of space) OR delete everything.
Amos Jeffries [Wed, 1 Feb 2012 07:44:49 +0000 (00:44 -0700)]
Bug 3370: external ACL sometimes skipping
Emit tag/user/log/message/pass details to the request in the case where
the external ACL entry has expired but within graceful revalidate period.
The result of this bug appears as incorrect matches later down the ACL
processing in any config relying on the external ACL output values.
Example; for bypassing auth login, or for filtering tagged traffic.
Alex Rousskov [Sat, 21 Jan 2012 01:16:48 +0000 (18:16 -0700)]
Do not add HTTP 110 and 111 Warnings to TCP_REFRESH_UNMODIFIED responses.
The old "stale if hit" logic did not account for cases where the stored
stale response became fresh due to a successful revalidation with the
origin server.
When the stored response was stale at the time of the request, we were
adding 110 "Response is stale" and even 111 "Revalidation failed"
Warning headers to responses while logging TCP_REFRESH_UNMODIFIED, which
is considered a hit.
Alex Rousskov [Sat, 14 Jan 2012 07:19:41 +0000 (00:19 -0700)]
Bug 3420: Request body consumption races and !theConsumer exception.
Also fixes endless waiting for HTTP client to send req body we no longer need.
Before these changes, the client side used a single "closing" state to
handle two different error conditions:
1. We stopped receiving request body because of some error.
2. We stopped sending response because of some error.
When a "directional" error occurred, we try to keep the transaction going in
the other direction (e.g., to give ICAP the entire request or to give HTTP
client the entire response). However, because there was just one "closing"
state, the code failed to correctly detect or process many corner cases,
resulting in stuck transactions and !theConsumer assertions/exceptions due to
races between enableAutoConsumption() and expectNoConsumption() calls.
This patch replaces the "closing" state with two direction-specific "we
stopped sending/receiving" flags.
Now, when the response sending code is done, it now checks whether the
receiving code stopped and closes the connection as needed. This is done both
when we encounter a sending error (ClientSocketContext::initiateClose) and
when we successfully sent the entire response to the client
(ClientSocketContext::keepaliveNextRequest).
Similarly, when the request body reading code is done, it now checks whether
the receiving code stopped and closes the connection as needed. This is done
both when we encounter a receiving error
(ConnStateData::noteBodyConsumerAborted) and when we successfully receive the
entire request body from the client (ClientSocketContext::writeComplete).
TODO: This patch focuses on various error cases. We might still have problems
when there is an early HTTP response and no errors of any kind. I marked the
corresponding old code with an XXX.
External ACL sometimes cannot find the credentials in ACL Checklist even
if they are attached to the HTTPRequest object.
This seems to happen when the checklist is created and the line match
started before the credentials are known. The credentials validation
updates the HTTP request state but is not aware of ACL checklists needing
to be updated so it never happens.
This patch:
* locate the %LOGIN value from either place where credentials can be found,
* updates the checklist if it was unset,
* passes '-' to the helper if no credentials at all were given.
Although the earlier logics forcing a lookup means this '-' case should
not happen it might if the external ACL were processed in 'fast' check.
Martin Huter [Fri, 2 Dec 2011 12:17:07 +0000 (05:17 -0700)]
Bug 2619: Excessive RAM growth due to unlimited adapted body data consumption
If the client does not read from the open connection (i.e. the user does not
confirm the browsers download-message-box in microsofts IE), squid keeps on
reading data from the ICAP server into the store entry, while no more data
can be delivered to the client.
Thus the store entry in memory is growing and squid may - in worst case -
consume memory up to the size of the users download.
This patch add API to StoreEntry to call the producer back when released
memory/space from the StoreEntry and add code to the ICAP client code to not
consume body data comes from the ICAP server when there is not available space
in the store entry.
projects / thirdparty / squid.git / log
