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.
Alex Rousskov [Thu, 24 Nov 2011 07:20:46 +0000 (00:20 -0700)]
Avoid crashes when processing bad X509 common names (CN).
X509_REQ_get_pubkey() returns a refcounted object that we must clean after use.
X509_REQ_get_subject_name() does not; cleaning the result may cause segfaults.
How we are supposed to tell the difference is beyond me.
Amos Jeffries [Tue, 11 Oct 2011 02:12:56 +0000 (20:12 -0600)]
Add directive dns_v4_first to make IPv4 connections before IPv6 is tried.
Default off, to prefer the faster protocol.
The use-case for this is networks which are IPv6-enabled but stuck
behind slow tunnels and whose upstream is not supporting full transit
services over IP.
Bug 3190: Large HTTP POST stuck after early ICAP 400 error response
When an ICAP REQMOD service responds with an error to
(or the REQMOD transaction aborts while processing) a large HTTP
request, the HTTP request may get stuck because the request body
buffer gets full and nobody consumes the no-longer-needed content.
The ICAP code quits but leaves the body buffer intact in case the
client-side code wants to bypass the error. After that, nobody consumes
the request body because the buggy client side does not inform the body
pipe that there will be no other consumers, which would have triggered
a noteBodyConsumerAborted() callback and enable auto-consumption or closed
the client connection.
Amos Jeffries [Sun, 28 Aug 2011 06:14:58 +0000 (00:14 -0600)]
Remove hierarchy_stoplist default value
This should have been done long ago with the other dynamic website
handling changes. It has caused a certain amount of confusion when things
which apparently should go to peers fail to reach them.
Alex Rousskov [Sat, 27 Aug 2011 12:34:04 +0000 (06:34 -0600)]
Polished unused code.
The unused dlopen() call is actually useful to enable when lt_dlopen() reports
"file not found" errors for loadable modules that do exist but that Libtool
cannot load successfully due to undefined symbols or other errors.
This inability to correctly report a library loading error is a long-standing
Libtool bug, stemming from Libtool's desire to try and load several
differently named library files until one succeeds, losing true error
information in the process.