Regression fix: Handle dstdomain duplicates and overlapping names better
Since 3.2 changes to dstdomain overlap detection teh case of duplicate
wildcards has become a fatal error needlessly.
This silently ignores all exact duplicates, even if they are wildcards.
Also, adjust the message display to always display the longer of the
domains first. Since we are dealing with sub-domains it is the most
reliable indicator of which should be removed to safely fix the detected
issue.
Alex Rousskov [Sat, 22 Sep 2012 03:12:33 +0000 (21:12 -0600)]
Do not reuse persistent connections for PUTs to avoid ERR_ZERO_SIZE_OBJECT.
A compliant proxy may retry PUTs, but Squid lacks the [rather complicated]
code required to protect the PUT request body from being nibbled during the
first try or [also tricky] code to send 100-continue expectation requiredto
delay body sending. Thus, Squid cannot safely retry some PUTs today, and
FwdState::checkRetriable() must return false for all PUTs, to avoid
bogus ERR_ZERO_SIZE_OBJECT errors (especially for clients that did not
reuse a pconn and, hence, may not be ready to handle/retry an error response).
In theory, requests with safe or idempotent methods other than PUT might have
bodies so we apply the same logic to them as well.
This reopens Squid bug #3398, undoing trunk r11859 commit which attempted
to close that bug.
Alex Rousskov [Sat, 22 Sep 2012 03:11:24 +0000 (21:11 -0600)]
Do not chunk responses carrying a Content-Range header.
When Squid forwards a response with a Content-Range header,
ClientSocketContext::socketState() detects the end of the response range(s)
and returns STREAM_*COMPLETE to ClientSocketContext::writeComplete().
The latter thinks that the writing of the response to the client must be
over and calls keepaliveNextRequest() instead of writing the last-chunk
(if any). If the to-client response was chunked, the client gets stuck
waiting for that missing last-chunk.
The multipart Range request case was already excluded from chunking (or it
would probably suffer from the same problem). With this change, no
Content-Range responses will be chunked.
N.B. Some servers send Content-Range responses to basic GET requests
without a Range header, so the problem affects more than just Range requests.
TODO: A proper fix would be to rewrite ClientSocketContext::writeComplete()
and other code so that it does not mix internal ClientStream completion with
[possibly chunk-encoded] writing completion. This should probably be done
along with fixing ClientSocketContext::socketState() and other state-checking
code to ignore to-client persistence (flags.proxy_keepalive), which is not
related to the internal ClientStream state.
Changes to interception handling in 3.2 series (namely the preference
for using ORIGINAL_DST) have increased the chances that misconfigured
network systems involving Squid will hit forwarding loops.
Two instances are currently known:
* passing forward-proxy traffic to a interception port.
* NAT performed on a separate box.
This enacts an old TODO by removing the loop detection bypass for
intercepted traffic and accelerated traffic. Now we always check for
loops regardless of how the request was received.
NOTE: accel mode was only included due to the TODO.
If problems are found there it can be re-instated.
According to CPU_SET(3) and, apparently, on some systems (e.g.,
OpenSuSE 10.3) CPU_COUNT macro expects a non-const argument. The
patch fixes build error on these systems.
Bug 3616: Retrieve client connection for ACL checks from the related HttpRequest object
This patch enable SSL client certificate ACL checks (user_cert and ca_cert)
in all cases the client connection information can retrieved from the related
HttpRequest object, eg when making peering decisions (peer_cache_access ACL).
Also fixes assertion failure caused by acl dst on transparent proxy.
Discussed under the "Supply client connection and IDENT information to
peer_cache_access ACL check" thread on squid-dev.
Amos Jeffries [Wed, 29 Aug 2012 03:36:29 +0000 (21:36 -0600)]
Regression: login=PASS send no credentials when none available.
login=PASS should act like PASSTHRU, sending no credentials header, when
no client supplied OR external ACL credentials are available.
3.2 has been found wrongly adding the username "PASS" in this case.
clang++ doesn't support c++ variable arrays for non-pod types.
Change variable arrays to dynamically-allocated arrays, Ipc::QueueReaders,
Ipc::StoreMap and Ipc::Mem::PageStack
Amos Jeffries [Thu, 2 Aug 2012 03:31:52 +0000 (21:31 -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.
This re-enables Squid peer selection algorithms for intercepted
traffic which has failed Host header verification.
When host verification fails Squid will use, in order of preference:
* an already PINNED server connection
* the client ORIGINAL_DST details
* cache_peer as chosen by selection algorithms
NOTE: whenever DIRECT is selected by routing algorithms the
ORIGINAL_DST is used instead.
Peer selection results are updated to display PINNED and
ORIGINAL_DST alongside DIRECT and cache_peer.
SECURITY NOTE:
At this point Squid will pass the request to cache_peer using the
non-trusted Host header in their URLs. Meaning that the peers
may still be poisoned by CVE-2009-0801 attacks. Only the initial
intercepting proxy is protected.
Full protection against CVE-2009-0801 can be enjoyed by building
Squid with the -DSTRICT_HOST_VERIFY compile-time flag. This will
make the peers unreachable for intercepted traffic where the
Host verification has failed.
The AccessLogEntry objects currently are only members of the ClientHttpRequest
objects. There are cases where we need to access AccessLogEntry from server
side objects to retrieve already stored informations for the client request and use it in
server side code with format/* interface (eg use Format::Format::assemble
inside http.cc)
This patch convert AccessLogEntry class to RefCountable to allow link it with
other than the ClientHttpRequest objects.
When '-k parse' is used deprecation notices and upgrade help messages etc
need to be bumped consistently up to level-0 and this macro will help
reducing the (?:) code mistakes.
Alex Rousskov [Sat, 21 Jul 2012 01:28:42 +0000 (19:28 -0600)]
Avoid bogus "Disk space over limit" warnings when rebuidling dirty ufs index.
Subtract sizes of added-then-rejected entries while loading ufs cache index.
Before SMP changes, the ufs code was incorrectly ignoring the size of
loaded-but-not-yet-validated entries, leading to cache disk overflows.
After SMP changes, the ufs code was not subtracting sizes of
loaded-but-then-rejected entries, leading to bogus "Disk space over
limit" warnings. Now we correctly account for both kinds of entries.
Bug 3478: Partial fix: Connection-auth on intercepted connections is broken
Currenty in the case of intercepted connections each request is open a new
connection to the destination server, even if the connection is a valid pinned
connection.
This patch fixes this problem and reuses pinned connections on intercepted
requests.
Fail with an explanation instead of asserting. The assertion fails when a ufs
cache_dir's swap.state has inconsistencies AND the user starts Squid with a -S
command line option. Normally, such inconsistencies are ignored and many of
them are benign. For example, a missing cache file with an ADD record in
swap.state is such an inconsistency.
The -S option was meant to help developers troubleshoot inconsistencies by
analyzing core dumps, but (a) admins treat assertions as Squid bugs and file
bug reports and (b) in most cases, it is really difficult to find the
inconsistency when Squid asserts after detecting all of them (and leaving the
detection context).
We now explicitly tell the admin what their options are and quit instead of
asserting.
TODO: Consider adding a ufs cache_dir option that checks for and removes
inconsistencies instead of not checking at all (default) or checking and
quitting (-S). This is difficult because some valid cache entries may look
inconsistent while they are being updated and some invalid cache entries
are not visible to Squid without a full directory scan.
Alex Rousskov [Sat, 21 Jul 2012 01:14:53 +0000 (19:14 -0600)]
Bug 3525: Do not resend nibbled PUTs and avoid "mustAutoConsume" assertion.
The connected_okay flag was not set when we were reusing the connection.
The unset flag overwrote bodyNibbled() check, allowing FwdState to retry
a PUT after the failed transaction consumed some of the request body
buffer, triggering BodyPipe.cc:239: "mustAutoConsume" assertion.
We now set the connected_okay flag as soon as we get a usable connection
and do not allow it to overwrite the bodyNibbled() check (just in case).
Restore memory caching ability lost since r11969.
Honor maximum_object_size_in_memory for non-shared memory cache.
Since r11969, Squid calls trimMemory() for all entries to release unused
MemObjects memory. Unfortunately, that revision also started trimming objects
that could have been stored in memory cache later. Trimmed objects are no
longer eligible for memory caching.
It is possible that IN_MEMORY check in StoreEntry::trimMemory() was preventing
excessive trimming in the past, but after SMP changes, IN_MEMORY flag is set
only after the object is committed to a memory cache in
StoreController::handleIdleEntry(), so we cannot rely on it. For example:
StoreEntry::unlock()
StoreController::handleIdleEntry() // never get here because entry is
set IN_MEMORY status // already marked for release
This change adds StoreController::keepForLocalMemoryCache() and
MemStore::keepInLocalMemory() methods to check if an entry could be stored in
a memory cache (and, hence, should not be trimmed). The store-related part of
the trimming logic has been moved from StoreEntry::trimMemory() to
StoreController::maybeTrimMemory(). StoreEntry code now focuses on checks that
are not specific to any memory cache settings or admission policies.
These changes may resulted in Squid honoring maximum_object_size_in_memory for
non-shared memory cache. We may have been ignoring that option for non-shared
memory caches since SMP changes because the check for it was specific to a
shared memory cache.
Bug 3577: File Descriptors not properly closed in trunk r12185.
Bug 3583: Server connection stuck after TCP_REFRESH_UNMODIFIED.
These changes fix FD leaks and stuck connections under two conditions:
1) Client aborts while Squid's server-side establishes a connection
Bug 3577: When a client quits while ConnOpener is trying to open the
connection to the next hop, FwdState cancels its ConnOpener callback.
ConnOpener notices that when trying to connect again and quits before
establishing a connection. The ConnOpener cleanup code did not close the
temporary FD used for establishing the connection. It did call fd_close(),
but fd_close() does not close the FD, naturally.
ConnOpener was probably leaking the temporary FD in other error handling
cases as well. It was never closed unless the connection was successful.
2) Client aborts after Squid's server-side established a connection:
Bug 3583: When a client aborts the store entry after receiving an HTTP 304 Not
Modified reply in response to a cache refreshing IMS request, HttpStateData
notices an aborted Store entry (after writing the reply to store), but does
virtually nothing, often resulting in a stuck server connection, leaking a
descriptor. Now we abort the server-side transaction in this case.
Bug 3577: Similarly, when a client disconnects after Squid started talking to
the origin server but before Squid received a [complete] server response,
HttpStateData notices an aborted Store entry (during the next read from the
origin server), but does virtually nothing, often resulting in a stuck server
connection, leaking a descriptor. Now we abort the server-side transaction in
this case.
FwdState now also closes the server-side connection, if any, when the client
aborts the store entry and FwdState::abort() callback is called. This helps
reduce the number of concurrent server-side connections when clients abort
connections rapidly as Squid no longer has to wait for the server-side I/O to
notice that the entry is gone. The code to close the connection was temporary
removed in trunk r10057.1.51.
Alex Rousskov [Fri, 13 Jul 2012 11:53:18 +0000 (05:53 -0600)]
[request|reply]_header_* manglers fixes to handle custom headers
This patch fix the [request|reply]_header_[access|replace] configuration
parameters to support custom headers. Before this patch the user was able
to remove/replace/allow all custom headers using the "Other" as header name.
Dmitry Kurochkin [Thu, 28 Jun 2012 05:14:18 +0000 (23:14 -0600)]
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, 28 Jun 2012 05:12:14 +0000 (23:12 -0600)]
Cleanup: disconnect Authentication and URL-rewrite callback handlers
The authentication handlers were for some reason using RH (rewrite helper)
callback typedef. But specifying it as a fatal error if the char*
parameter was used in auth.
Assign a new callback typedef AUTHCB for use by authentication callers.
This allows auth callers to use different parameters (none) and to avoid
possibly fatal mistakes when coding new auth modules.
Alex Rousskov [Thu, 28 Jun 2012 05:11:18 +0000 (23:11 -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.
Julien Pinon [Thu, 21 Jun 2012 00:39:01 +0000 (18:39 -0600)]
Bug 3462: Delay Pools and ICAP
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 AnyP::PortCfg 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
AnyP::PortCfg object.
But this is breaks the intercepted https traffic.
This patch:
1. Inside prepareTransparentURL read the protocol from the related
AnyP::PortCfg object
2. add_http_port() locks the new port pointer before linking it.
3. parsePortCfg() locks the new port pointer before linking it.
4. free_PortCfg() unlock 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.
This can occur due to long lists of unit tests needing a lot of local
variable state tracking. Essentially 'function too long' after CPPUNIT
macros have been expanded.
Break the large set of request-line unit tests into groups related to
sub-parts of the request-line.
Amos Jeffries [Mon, 4 Jun 2012 10:50:34 +0000 (04:50 -0600)]
Define PRIuSIZE for displaying size_t
This allows us to avoid casting size_t to long long for printf.
./configure script auto-detects the supposedly standard %zu macro in case
it is missing and compat/types.h defines some alternatives for systems
which do not define it.
projects / thirdparty / squid.git / log
