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.
Make %<a and %<p details available to [eCAP] RESPMOD services
... via adaptation_meta.
This patch fills access log entry with server connection
details as soon as possible. Previously, ALE was updated only
in prepareLogWithRequestDetails() when adaptation process has
already finished.
Name-only note ACL stopped matching after trunk r14465 (note -m).
Despite this fix, empty ACLStringData values (also used for myportname,
peername, tag, and other ACLs) must [continue to] match nothing because
Squid ORs acl values. In math, empty disjunction is false.
The note ACL matches name:value entries, not bare values. A valueless
name has a different semantics than an empty list of bare values. It is
(name and value1) or (name and value2)
rather than
name and (value1 or value2)
where an empty value disjunction would have falsified the whole
condition.
Amos Jeffries [Mon, 18 Jan 2016 13:40:42 +0000 (02:40 +1300)]
Tests: Add --action parameter for testing script
This command line option allows automake target to be provided as an
override to the target specified in the test build scripts. With this a
partial test can be run for all specified layers rather than a full
distcheck which can be quite slow.
* disable the use of system CA by default to verify client connection
certificates. Since the use of client certificates is rare.
* no change to verification of upstream server or peer certificates.
Since the use of system CA to sign server certificates is common.
* the new "default-ca" configuration option and its documentation are
updated to make the situation more obvious amongst the other TLS options
changes in Squid-4.
* the action of the sslflags=NO_DEFAULT_CA is already deprecated, so no
change when it is used. On port lines it now merely sets the default.
It may be a good idea to also disable system CA use for cache_peer and
ICAPS connections. For now they are left unchanged.
The new connections_encrypted ACL matches transactions where all HTTP
messages were received over TLS transport connections, including messages
received from ICAP servers.
Some ICAP/eCAP services receive data from unencrypted sources. Some ICAP/eCAP
services are "secure". By default we assume that all eCAP services and all
ICAP services on TLS transport connections are "secure" unless the user
uses the "connection_encryption" option in service configuration line.
Tls flags listed using the [tls-|tls|ssl]flags option are not configured
correctly. In practice always the SSL_FLAG_NO_DEFAULT_CA even if no
selected and all other flags are ingored
Amos Jeffries [Wed, 6 Jan 2016 04:02:24 +0000 (17:02 +1300)]
negotiate_kerberos_auth: check for overflow on count of group SIDs
When processing a Kerberos token the count of group SID records is
received from the remote end. Validate that the count given does not
exceed the possible length values on 32-bit systems.
Detected by Coveriity Scan. Issues 1258701, 1258702,1258703
CharacterSet improvements: implement operators -=, +, -, ==, !=; implement c++11 ranges-constructor, add ostream output operator, and related unit tests.
Change build order in top-level Makefile.am so that src/ is built before helpers/
Alex Rousskov [Thu, 31 Dec 2015 03:32:59 +0000 (20:32 -0700)]
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.
There are several use cases where an annotation may contain a list of values
for a single key. Today it is only possible to match the full annotation value.
This patch investigates the -m flag which can be used to enable delimiter
separated substrings matching on annotations:
acl aclname note [-m[=delimiters]] name value ...
The '-m' flag by default matches comma separated substrings. The optional
"delimiters" parameter is a list of non-alphanumeric characters, which can
be used as alternate delimiters.
E.g. if an external ACL sets an annotation like:
"applications=http,facebook,facebook-chat"
the following ACLs can be used to block access to certain applications:
This patch adds the following formatting codes:
%ssl::>negotiated_version The TLS version of the client-to-Squid connection.
%ssl::<negotiated_version The TLS version of the Squid-to-server connection.
%ssl::>received_hello_version The TLS version of the Hello message received
from TLS client
%ssl::<received_hello_version The TLS version of the Hello message received
from TLS server.
%ssl::>received_supported_version The maximum TLS version supported by the
the TLS client.
%ssl::<received_supported_version The maximum TLS version supported by the
the TLS server.
%ssl::>cipher The negotiated cipher of the client-to-Squid connection.
%ssl::<cipher The negotiated cipher of the Squid-to-server connection.
These are useful for statistics collection, security reviews, and reviews
prior to adjusting the list of the allowed TLS protocols and ciphers.
Amos Jeffries [Tue, 22 Dec 2015 10:57:16 +0000 (23:57 +1300)]
Add cache_peer auth-no-keytab option to use a credentials cache instead of keytab
... when using login=NEGOTIATE to authenticate via kerberos to a peer.
When specified, this option prevents Squid from crafting a kerberos
credentials cache from a keytab, but instead lets GSSAPI use an existing
credentials cache.
Amos Jeffries [Fri, 18 Dec 2015 13:10:26 +0000 (02:10 +1300)]
Convert ClientSocketContext to MEMPROXY class
CBDATA and RefCountable are not very compatible. With the recent I/O
callback shuffling there appears to no longer be any reason for this
class to be CBDATA.
Alex Rousskov [Fri, 18 Dec 2015 04:30:52 +0000 (21:30 -0700)]
Squid with a misconfigured (too-small) shared memory cache might crash
upon startup.
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.
Amos Jeffries [Wed, 16 Dec 2015 23:10:40 +0000 (12:10 +1300)]
Shuffle TLS NPN logic to libsecurity
Also, adds new config option tls-no-npn to fully disable NPN on selected
cache_peer or http(s)_port directives.
ICAPS services set TLS NPN to off by default to prevent NPN wrongly
advertising them as HTTPS connections. The semantic meaning of NPN is to
name the protocol which is being wrapped by TLS, in the case of ICAPS that
is ICAP/1.0 but Squid NPN callback is currently hard-coded to send 'http/1.1'
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.
%ssl::<cert_errors logformat code part2: provide depth information
The first implementation of %ssl::<cert_errors formating code does not provide
information about the certificate the errors belongs.
This patch prints the chain depth information for error, if exist, using the
following format for the printed certificate error:
ERROR@depth=X
The patch also adds the "error_depth_" parameter to cert validator, used to
pass depth information from cert validator to squid.
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.
Amos Jeffries [Tue, 8 Dec 2015 18:47:25 +0000 (10:47 -0800)]
Refactor ClientSocketContext write(2) using Server:: write methods
Writing to the client connection is scoped as an action for class Server
and its child classes. There is no need for ClientSocketContext to be
providing the callback handlers and performing I/O error handling.
With Server providing the current write handler we can move from
CBDATA callbacks to AsyncCall. Initial testing indicates this has some
minor performance benefit.
Amos Jeffries [Tue, 8 Dec 2015 01:48:40 +0000 (17:48 -0800)]
TLS: refactor cert=/key= storage in libsecurity
This updates the cert=/key= filename storage from single entries
in PeerOptions to a list of key pairs in preparation for supporting
multiple certificates on client or server TLS contexts.
key= following a cert= parameter is now enforced, rather than just
warned about.
squid.conf can now be configured with multiple [cert= [key=...]]
pairs of filenames, however only the first is used. This differs
from older behaviour where the last value(s) were used. But since
configurations with multiple values was not supported previously
this seems acceptible breakage.
Since the multi-cert support is not fully existing yet this config
ability is left undocumented for now.
Fix connections over plain squid port to SSL origins
After the "Restrict SslBump inspections of cache_peer connections"/r14425 patch
https requests over plain proxy port (eg. "GET https://www.example.com/" on
http_port) does not work any more.
This is because the BlindPeerConnector class, which used now for any connection
to the https peers or servers designed initialy to work with cache_peer
connections.
This small patch fix Ssl::BlindPeerConnector to initiate SSL connections
destined to origin SSL servers.
Amos Jeffries [Sun, 6 Dec 2015 13:59:59 +0000 (05:59 -0800)]
Cleanup: Expose SSL initialization function to libsecurity
SSL initialize needs to be performed before any security context
objects are generated. Expose the function so that the new blank
context methods can use it.
Amos Jeffries [Fri, 4 Dec 2015 02:28:25 +0000 (18:28 -0800)]
Cleanup: add Security::ContextPointer as smart pointer to SSL_CTX*
Due to circular dependency issues between ssl/libsquidssl.la and
security/libsecurity.la the code within src/ssl/ is restricted to
only using Security::ContextPtr, it MUST NOT use ContextPointer
Code outside of src/ssl/ should always use Security::ContextPointer
when storing a reference to a context.
Unfortunately some uses of SSL_CTX_Pointer and AnyP::PortCfg remain
in src/ssl/support.cc for now.
Complete certificate chains using external intermediate certificates
stored in sslproxy_foreign_intermediate_certs 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.
Amos Jeffries [Mon, 30 Nov 2015 14:23:16 +0000 (06:23 -0800)]
Cleanup: Simplify HTTP 1xx control message writing
The ::Server class heirarchy has the responsibility of writing to
client connection sockets. The logic that was in ClientSocketContext
can be moved (unchanged) to HttpControlMsgSink and ConnStateData.
There was actually never any need to have it spread outside the
ConnStateData class hierarchy in the first place.
No logic is changed in this patch, just symbol shuffling and one
method inlined into its caller.
Restrict SslBump inspections of cache_peer connections.
This change is specific to FwdState code path. It does not affect tunneled
traffic. Thus, it does not affect CONNECT tunnels unless they are being
inspected with SslBump code.
The old code always used PeekingPeerConnector when connecting to a TLS-related
cache_peer. That approach worked because PeekingPeerConnector does not always
inspect the SSL/TLS connection it establishes. We were kind of lucky that
PeekingPeerConnector exceptions matched FwdState needs.
The primary PeekingPeerConnector goal is to inspect. As its code evolves, it may
enable inspection when FwdState does not want it. Non-peeking cases inside
PeekingPeerConnector should all deal with exceptional situations that
are difficult to predict a priori, before the connector object is created.
This change restricts inspection to cases where an inspected SSL client
connection is being forwarded, reducing the probability that a peer
connection is wrongly inspected. This change does not fix any known bugs.
Amos Jeffries [Wed, 25 Nov 2015 04:21:40 +0000 (20:21 -0800)]
Cleanup: Refactor ConnStateData pipeline handling
This refactors the request pipeline management API to use std::list
instead of a custom linked-list with accessors spread over both
ConnStateData and ClientSocketContext.
To do this a new class Pipeline is created with methods wrapping
std::list API and extending it slightly to meet the HTTP/1.1 pipeline
behaviours and perform basic stats gathering. The pipeline management
methods and state variables are moved inside this class.
ClientSocketContext was performing several layering violations in
relation to ConnStateData when one transaction ended and the next needed
starting. Treating the pipeline properly as a std::list forced removal
of that violation.
* actions for starting or resuming a transaction on the connection are
now moved to ConnStateData::kick(). Which gets called after each
transaction completes.
- with some further cleanup it can be called at any point the
ConnStateData needs to resume processing. However, that is left out of
scope for this patch.
* the ClientSocketContext scope now ends when the finished() method is
used to mark completion of these contexts transactions. Which will mark
itself done and de-register from the Pipeline queue. The ConnStateData
kick() method still needs to be called to resume other transactions
processing.
* the queue is now holding RefCounted Pointers. So that the
ClientSocketContext destructor no longer needs to be careful of
registrations, and the queue entries are guaranteed to still exist while
queued.
* The old freeAllContexts() and notifyAllContexts(int) members of
ConnStateData have been combined into Pipeline::terminateAll(int).
The ClientSocketContext and ConnStateData documentation is updated to
describe what they do in regards to connection and transaction processing.
Initial testing revealed CONNECT tunnels always being logged as ABORTED.
This turns out to be techincally correct, since the only way a tunnel
can finish is for client or server to just close the connection.
However, it is not right to log these as abnormal aborts. Instead, I
have now made the context be finished() just prior to the
TunnelStateData being destroyed. That way normal closure should show up
only as TUNNEL, but timeouts and I/O errors should still be recorded as
abnormal.
Two potential bugs have been highlighted:
* The on_unsupported_protocol handling function appears to be a bit
broken. It pop()'s contexts off the pipeline directly without going
through the proper finished() process to release their state data. I
have highlighted that with an XXX and comment.
* The ssl-bump handling logic switching to TLS begins with a terminateAll(0)
run on all active contexts. It does not check whether there is any existing
pipeline of requests waiting to be processed. And the action prematurely
purges the bumped CONNECT message context, which should be closed properly
and logged as successful.
Alex Rousskov [Thu, 19 Nov 2015 05:51:49 +0000 (22:51 -0700)]
Store API and layout polishing. No functionality changes intended.
Fixes "any Store is a Root" API that forced us to bloat the base
Store class with methods needed only in Store::Root() Controller.
Unblocks bug #7 (cached headers update) fixes.
The Store namespace hierarchy now looks like this:
* Storage: Any storage. Similar to the old Store class, but leaner.
* Controller: Combined memory/disks caches and transients. Root API.
* Controlled: Memory cache, disk(s) cache, or transient Storage.
* Disks: All disk caches combined.
* Disk: A single cache_dir Storage.
* Memory: A memory cache.
* Transients: Entries capable of being collapsed for CF.