William Lima [Tue, 23 Feb 2016 15:47:10 +0000 (04:47 +1300)]
Bug 3870: assertion failed: String.cc: 'len_ + len <65536' in ESI::CustomParser
The custom ESI parser used in absence of libxml2 or libexpat parsers was
restricted to handling 64KB buffers but under some conditions could expand
to over 64KB during the parse process. Hitting this assertion.
TODO: the parser can now be redesigned to make use of Tokenizer and
CharacterSet parsing tools. But that is left for later work.
Alex Rousskov [Thu, 18 Feb 2016 04:15:33 +0000 (21:15 -0700)]
Better handling of huge response headers. Fewer "BUG 3279" messages.
When we failed to parse a response, do not store the fake half-baked
response (via a replaceHttpReply() call). Doing so leads to misleading
"BUG 3279: HTTP reply without Date" messages (at best). The fake
response is only meant for continueAfterParsingHeader().
Also removed a misleading XXX that may have caused Bug 4432 in v4.0
(trunk r14548).
In the case SSL errors detected by certificate validator helper the objects
stored in Ssl::ServerBump::sslErrors member and will never released.
This member normally points to an Ssl::CertErrors list attached to the related
SSL object which is responsible to release this list.
When the cert validator detects errors a new errors list created and attached
to the related Ssl::ServerBump::sslErrors member but the SSL objects still
hold the old one. The old list released but not the new one.
This patch also fixes the case the cbdata protected Ssl::CertErrors list,
still is used through the related Ssl::ServerBump object but it is not valid
any more, because the SSL object which hold it gone.
This patch instead of storing the Ssl::CertErrors list to Ssl::ServerBump
object stores the SSL object and increases its reference to avoid be released
Bug 4437: Fix Segfault on Certain SSL Handshake Errors
Squid after an unsuccesfull try to connect to the remote server may make two
concurrent retries to connect to the remote SSL server, calling twice the
FwdState::retryOrBail() method, which may result to unexpected behaviour.
Prevent this by just closing the connection to the remote SSL server inside
FwdState::connectedToPeer method on error and instead of calling the
FwdState::retryOrBail method, just allow comm_close handler to retry the
connection if required.
Markus Mayer [Sun, 31 Jan 2016 05:43:08 +0000 (18:43 +1300)]
Fix handling of shared memory left over by Squid crashes or bugs
A Squid instance may inherit an old shared memory segment from the
previous instance as the result of either a Squid crash or an at-exit
cleanup bug. This change fixes two problems triggered by old segments:
1. After an earlier OS X fix (bug 3805; trunk r13947), Squid stopped
initializing previously used shared memory. Uninitialzed memory
resulted in subtle bugs and crashes.
2. When called for an old Squid shared memory segment, OS X
ftruncate() fails with EINVAL, preventing Squid from starting when
the old segment is still around.
More specifically: Darwin ftruncate() calls pshm_truncate().
pshm_truncate() checks if the PSHM_ALLOCATED flag is already set on
the memory region. If the flag is set, the call fails with EINVAL.
Otherwise, pshm_truncate() sets PSHM_ALLOCATED for the region.
Since Squid must call ftruncate() to size every new segment, all
old Squid segments have that flag set, preventing ftruncate() calls
for old segments in newer Sqid instances from succeeding.
Fix invalid FTP connection handling on blocked content
FTP client gets stuck after the following chain of events:
* Client requests a file that will be blocked by ICAP.
* Squid starts downloading the file from the FTP server
and sends "150 Opening..." to the FTP client.
* Squid aborts the data connection with the FTP server
as soon as the ICAP service blocks it.
* Squid sends "451 Forbidden" to the FTP client.
* The FTP server sends "500 OOPS: setsockopt: linger" to Squid.
* Squid terminates the control connection to the FTP server.
* Squid establishes a new control connection to the FTP server
but does not authenticate itself.
* Further commands from the FTP client do not work any more.
The above and many similar problems exist because Squid handles
FTP client-to-squid and squid-to-FTP server data connections
independently from each other. In many cases, one connection does
not get notified about the problems with the other connection.
This patch:
- Add Ftp::MasterState::userDataDone to record received
the FTP client final response status code to sent (or to be send)
to the client.
- The Ftp::MasterState::waitForOriginData flag to hold status of the
squid-to-server side. If the squid-to-server side is not finishes
yet this is true.
- Send a control reply to the FTP client only after the data transfered
on both server and client sides.
- Split Client::abortTransaction to Client::abortOnData and to
Client::abortAll()
- Implement the Ftp::Relay::abortOnData() and Ftp::Relay::Abort()
(i.e., StoreEntry abort handler) to avoid closing the control
connection when the data connection is closed unexpectedly.
Alex Rousskov [Tue, 5 Jan 2016 05:36:22 +0000 (18:36 +1300)]
Reflect the [ugly] reality in external_acl_type cache=n documentation.
The patch does not change how the cache works, but may help admins
configure the cache correctly if they stumble upon the updated docs.
A typical external ACL cache with the default cache settings consumes
about 70 MB of RAM (more in many cases, e.g., if annotations are used
with external ACLs). Besides memory usage, the default cache is using
only 977 hash buckets for 262144 entries so there may be quite a bit
of linear search going on by default.
Avoid memory leaks when a certificate validator is used with SslBump
When a certificate validator was used, sslCrtvdHandleReplyWrapper delivered
validator response directly to the Ssl::PeerConnector job using job's
Ssl::CertValidationHelper::CVHCB callback. If that synchronous call happened
to be the last job call, then Ssl::PeerConnector::done() would become true
for the job, as it should, but nobody would notice that the PeerConnector
job object should be deleted, and the object would leak.
This fix converts CVHCB into an async job call to avoid direct, unprotected
job calls in this context.
Alex Rousskov [Fri, 18 Dec 2015 12:50:46 +0000 (01:50 +1300)]
Fix startup crash with a misconfigured (too-small) shared memory cache
Controller condition for allocating MemStore is slightly different from
MemStore condition for allocating MemStore::map, resulting in MemStore
without a map. Until that discrepancy is fixed, be careful when
dereferencing MemStore::map.
Fix connection retry and fallback after failed server TLS connections
FwdState should retry connect to the next ip after a Ssl::PeerConnector failure
When the Ssl::PeerConnector fails to establish an SSL connection FwdState does
not retry to connect to the next destination server ip address, but instead
returns an error.
Complete certificate chains using external intermediate certificates
... stored in sslproxy_foreign_intermediate_certs PEM file.
Many origin servers do not send complete certificate chains. Many
browsers use certificate extensions in the server certificate to
download the missing intermediate certificates automatically from
the Internet. Squid does not do that (yet?).
This patch adds the sslproxy_foreign_intermediate_certs configuration directive
to allow an admin to supply a file with intermediate certificates that
Squid may use to complete certificate chains. These intermediate
certificates are _not_ treated as trusted root certificates.
Alex Rousskov [Thu, 19 Nov 2015 05:08:41 +0000 (21:08 -0800)]
Stop using dangling pointers for eCAP-set custom HTTP reason phrases.
Squid still does not support [external] custom reason phrases and,
hence, cannot reliably support eCAP API that sets the reason phrase to
the one supplied by the adapter. This and r14398 changes fix [known]
regression bugs introduced by r12728 ("SourceLayout").
Alex Rousskov [Thu, 19 Nov 2015 05:05:49 +0000 (21:05 -0800)]
Fix status code-based HTTP reason phrase for eCAP-generated messages.
Calling .reason() on a not-yet-set theMessage.sline object resulted in
"Init" status reason phrase for all from-scratch (i.e., not cloned)
eCAP-made HTTP responses. This fix lets Squid compute the reason phrase
based on the status code, just like Squid does for forwarded responses
(IIRC).
Amos Jeffries [Thu, 19 Nov 2015 05:00:37 +0000 (21:00 -0800)]
Revert r13921: Migrate StoreEntry to using MEMPROXY_CLASS
This change has been identified as teh trigger for several object caching
errors. The real cause is not yet known, but reverting this optimisation
avoids it, so is being done for stability.
- Squid receives TLS Hello from the client (TCP connection A).
- Squid successfully negotiates an TLS connection with the origin server
(TCP connection B).
- Squid successfully negotiates an TLS connection with the client
(TCP connection A).
- Squid marks connection B as "idle" and waits an HTTP request from
connection A.
- The origin server continues talking to Squid (TCP connection B).
Squid detects a network read on an idle connection and closes TCP
connection B (and then the associated TCP connection A as well).
This patch:
- When squid detects a network read on server idle connection do an
SSL_read to:
a) see if application data received from server and abort in this case
b) detect possible TLS error, or TLS shutdown message from server
c) or ignore if only TLS protocol related packets received.
Stuart Henderson [Tue, 10 Nov 2015 09:30:19 +0000 (01:30 -0800)]
Fix SSL_get_certificate() problem detection
The autoconf check for SQUID_SSLGETCERTIFICATE_BUGGY fails on ssl library
builds which don't include SSLv3; as a result of the autoconf decision
this can end up triggering the assert(0) in Ssl::verifySslCertificate()
in ssl/support.cc (line 1712 in 3.5.11).
Amos Jeffries [Fri, 30 Oct 2015 18:26:41 +0000 (11:26 -0700)]
Add Locker friend class to SBuf for protection against memory issues
When appending or otherwise modifying an SBuf based on a SBuf& or char*
the parameter used may be pointing at the MemBlob memory buffer
indirectly without holding a separate ref-count lock to it.
If 'this' SBuf then requires reallocation for any reason the char* or
buffer pointer taken from the SBuf&, which is being manipulated may in
fact be left pointing at invalid memory.
Utilize a private Locker class to create relatively cheap ref-count locks
on the store_ MemBlob when this problem MAY occur. This Locker needs to
be used on all non-const SBuf methods accepting char* or SBuf& argument.
Alex Rousskov [Wed, 28 Oct 2015 03:47:56 +0000 (20:47 -0700)]
Connection stats, including %<lp, missing for persistent connections.
The code reusing a pconn was missing a hier.note() call, resulting in 0
values logged for %<lp (local port number of the last server or peer
connection) and probably other missing stats.
Also refactored poorly copied statistics collection code to remove
duplication and always update to-server connection stats when the actual
connection becomes available.
Positive side effect: Upon setsockopt(2) failures, the tos and nfmark
fields of a pinned connection were set to the desired (but not actually
applied) values, while persistent connection fields were left intact
(and, hence, stale). Both fields are now reset to zero on failures, for
both types of connections.
Aymeric Vincent [Tue, 27 Oct 2015 22:48:24 +0000 (15:48 -0700)]
Fix incorrect authentication headers on cache digest requests
login=NEGOTIATE can have an additional parameter specified,
like login=NEGOTIATE:xxx
One test added in rev.12714 does not take this case into account and it
will send a garbage "login:password" (== "NEGOTIATE:xxx") to its peer
when requesting a digest.
This is a workaround patch to remove the broken Authentication headers
entirely. Support for Negotiate to the peer on these digest requests is
still needed.
Amos Jeffries [Tue, 27 Oct 2015 22:21:38 +0000 (15:21 -0700)]
Avoid errors when parsing manager ACL in old squid.conf
ACL manager is now a built-in definition and has a different type. That
has been causing FATAL errors when parsing old squid.conf. We can be
nicer and just ignore the obsolete config lines.
Amos Jeffries [Wed, 14 Oct 2015 05:29:29 +0000 (22:29 -0700)]
Bug 3574: crashes on reconfigure and startup
When Squid receives a reconfigure signal before its signal handler
has been registered on startup it will crash with unhandled signal
exceptions. This can be triggered on system boot when a resolv.conf
alteration signal wins a race with the daemon service initialization.
Fix:
Register the reconfigure signal handler early and ignoring signals
until initial squid.conf load has completed.
When Squid receives a reconfigure signal while it is already in the
process of reconfiguring, the two async sequences can interfere and
result in confusing fatal error or crashes.
Fix:
Only allow one reconfigure sequence to be initiated at a time.
Also, if shutdown signal has been received while waiting for a
reconfigure to finish, let shutdown take precedence over any pending
reconfigure repeats.
Based on work by Clint Byrum and D J Gardner, Ubuntu
Bug 4330: Do not use SSL_METHOD::put_cipher_by_char to determine size
... of cipher on hello messages
The use of these methods can cause many problems in squid:
- In earlier openSSL libraries the SSL_METHOD::put_cipher_by_char method with
NULL arguments returned the size of cipher in the SSL hello message.
In newer openSSL releases, calling this method with NULL arguments is not
valid any more, and can result to segfaults.
- In newer libreSSL library releases, the SSLv23_method it is used to produce
TLS messages and does not return the size of a cipher in an v2 HELLO
message.
Fix cache_peer login=PASS(THRU) after CVE-2015-5400
The patch for CVE-2015-5400 converts all non-200 peer responses
into 502 Bad Gateway responses when relaying a CONNECT to a peer.
This happens to break login=PASS and login=PASSTHRU behaviour
which relies on the 401 and 407 status being relayed transparently.
We need to relay the auth server responses as-is when login= is
set to PASS or PASSTHRU but then unconditionally close the
connections to prevent CVE-2015-5400 from occuring.
After the exception is thrown, Squid attempts to wind down the affected
transaction (as it should), but the code either quits with an unhandled
exception error or hits the !callback assertion, depending on whether
the async job processing was in place when the exception was hit (which
depends on whether non-blocking/slow ssl_bump ACLs were active).
The attached patch does three things:
1. Teaches Squid to guess the final ssl_bump action when no ssl_bump
rules match. The final guessed action is "bump" if the last non-final
action was "stare" and "splice" otherwise. I suspect that the older
Squid code attempted to do something like that, but that code may have
been lost when we taught Squid to ignore impossible ssl_bump actions.
2. Protects ssl_bump-checking code from quitting with an unhandled
exception error.
3. Converts the fatal !callback assertion into [hopefully less damaging]
transaction error, with a BUG message logged to cache.log.
More work may be needed to investigate other exceptions, especially
Must(!csd->serverBump() || csd->serverBump()->step <= Ssl::bumpStep2);
As an historic optimization StoreEntry uses a custom pool chunk size of 2MB.
Knowledge of the actual benefits from this optimization has been lost in time,
and it's not possible to accurately measure its actual impact in all load
scenarios; at the same time this optimization is blocking other potentially
useful developments.
This change is therefore considered a potential performance regression in
some load scenarios.
Bug 4309: Fix the presence of extensions detection in SSL Hello messages
RFC5246 section 7.4.1.3 (Server Hello) says:
The presence of extensions can be detected by determining whether
there are bytes following the compression_method field at the end
of the ServerHello.
Current parsing Hello code checks whether there are bytes in the whole
SSL message. It does not account for the fact that the message may
contain more than just ServerHello.
This patch fixes this issue and tries to improve the related code to
avoid related problems in the future.
Using the MemBuf::buf directly is not great, but it does have a properly
terminated c-string in this instance. We cannot use Raw() interface
because that is for output at DBG_DATA levels and will only display the
buffer name as if that was the raw traffic bytes at 11,2.
Which negates the entire purpose of this 11,2 output.
Alex Rousskov [Tue, 1 Sep 2015 09:25:57 +0000 (02:25 -0700)]
Support splice for SSLv3 and TLSv1 sessions that start with an SSLv2 Hello
Such sessions are created, for example, by some SSL clients using OpenSSL
v0.9.8 with default options. This does _not_ relate to SSLv2 sessions.
Just enacts the permitted exception for Hello messages in RFC 6176.
Amos Jeffries [Sat, 29 Aug 2015 20:21:33 +0000 (13:21 -0700)]
Bug 3553: cache_swap_high ignored and maxCapacity used instead
Also, to make matters worse the amount of objects (max 70) being purged on
each of the 1-second maintenance loops was far too small for the traffic
speeds of up to 20k RPS now being processed by proxies.
This fixes the cache_swap_high behaviour to closer match what is documented
at present, although some documentation does say it cleans all the way down
to the low-water mark. Which appears never to have been true in regards to
one cycle but would occur over several of the proxy speed was not too high.
With this updated algorithm there is almost no limit to how far the
aggressiveness can scale, but it is linear at 300 objects per multiple of
the gap between low- and high- watermark.
SwapDir::maintain is now fairly well documented and debug traces added. With
several TODO ideas for future improvement also documented in the method code.
Alex Rousskov [Sat, 29 Aug 2015 20:11:19 +0000 (13:11 -0700)]
When a RESPMOD service aborts, mark the body it produced as truncated.
Without these changes, the recipient of the truncated body often
cannot tell that the body was actually truncated (e.g., when Squid
uses chunked encoding for body delivery). Lying about truncation
may result in rather serious user-level problems.
Amos Jeffries [Sat, 29 Aug 2015 18:51:19 +0000 (11:51 -0700)]
Cleanup: fix assertion in Store unit tests
The old Squid String implementation cannot handle appending nullptr or
negative lengths. So if the test code using CapturingStoreEntry ever
tries to append such it will crash instead of working like a StoreEntry
should.
Amos Jeffries [Tue, 25 Aug 2015 15:27:18 +0000 (08:27 -0700)]
Docs: auto-build release notes for snapshots
This adds conditional build support to generate release notes whenever
a tarball is being created, regardless of what the code branch status
is. All that is required is the linuxdoc tool chain.
Formal release branch snapshots have been publishing the notes files
built for their previous release. But development versions of Squid
have not been getting documented at all which can be annoying for
testers.
The release-3.N.html file is also removed from the repository. With this
update it is no longer be needed by the snapshot machinery.
Handle nil HttpReply pointer inside various handlers called from
Ftp::Server::handleReply(). For example, when the related StoreEntry
object is aborted, the client_side_reply.cc code may call the
Ftp::Server::handleReply() method with a nil reply pointer.
The Ftp::Server::handleReply() methods itself cannot handle nil replies
because they are valid in many states. Only state-specific handlers know
whether they need the reply.
The Ftp::Server::handleReply() method is called [via Store] from Client code.
Thus, exceptions in handleReply() are handled by the Ftp::Client job. That job
does not have enough information to know whether the client-to-Squid connection
should be closed; the job keeps the connection open. When the reply is nil,
that open connection becomes unusable, leading to more problems.
This patch fixes the Ftp::Server::handleReply() to handle exceptions,
including closing the connections in the case of an exception. It also
adds Must(reply) checks to check for nil HttpReply pointers where the
reply is required. Eventually, Store should start using async calls to
protect jobs waiting for Store updates. Meanwhile, this should help.