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.
This bug investigated after the r14528 patch.
The (forgotten?) Ssl::PeekingPeerConnector::callback member is always NULL and
hides the callback member of the parent Ssl::PeerConnector class. This is has
as result the failure of "Must(callback != NULL)" clause inside the
Ssl::PeekingPeerConnector::tunnelInsteadOfNegotiating method investigated with
the r14528 patch.
Amos Jeffries [Wed, 10 Feb 2016 09:36:47 +0000 (22:36 +1300)]
Cleanup: const correctness for SBuf iterators
The SBuf iterator has almost all properties of a const_iterator but the
begin/end methods producing it had the syntax for returning a non-const
iterator. This leads to several const related problems despite the
SBufIterator providing a const API:
1) range-for loops cannot be used on const SBuf
2) begin/end/rbegin/rend operators cannot be used on const SBuf
Also, the SBufIterator API for operator*() used a temporary char
which, wile const, allows the compiler to implicitly use move semantics
on the return value and allows some (thankfully unused) &*itr() syntax
to compile when it should not be possible.
To avoid even the potential of that causing hidden issues in future we
convert the operator*() to the API definition used widely within the STL
const_iterator's (const char &operator*() const).
Amos Jeffries [Tue, 9 Feb 2016 08:57:33 +0000 (21:57 +1300)]
Cleanup: convenience library renaming
I have been trying to automate graphing of the Squid internal
dependencies. One of the major issues that has encountered is that some
of our convenience libraries use the '-' hyphen character which is a
reserved character in DOT graph format.
To make the scripts much simpler and the visual output reflect exactly
what the library names are this patch cleans up the libraries to follow
our pre-existing policy, and now also to remove punctuation from library
names. Which condition has been added to the guidelines documentation.
The information about PeekingPeerConnector splicing the connections
was lost in some cases, resulting in two different bugs:
- With a certificate validator, the PeekingPeerConnector class calls
back FwdState, which calls the ConnStateData class, which then tries
secure the connection with the already tunneled SSL client and
closes the connection on negotiating errors.
- Without a certificate validator, the PeekingPeerConnector class
never calls FwdState class, and both PeekingPeerConnector and
FwdState objects stall until finishing tunnelState closes server
and client connections.
Now, PeerConnector always calls FwdState back, marking spliced
connections as such. This has the following positive side-effects:
- When FwdState learns about spliced connections, it does not call
ConnStateData back. Instead, it terminates and gets destroyed.
The tunnel continues uninterrupted.
- The PeekingPeerConnector job ends and is destroyed instead of
waiting to call FwdState.
Fix external_acl problems after trunk r14351
(Support logformat %macros in external_acl_type format).
The above changes created the following problems:
- external_acl requires AccessLogEntry but ALE is not available
in many cases such as ssl_bump ACLs.
- The %<cert_subject stopped working because it was supported by
external_acl code and not by logformat code.
This patch:
- Passes AccessLogEntry in most cases.
For example, PeerConnector-related classes are now covered.
- Implements the %<cert_subject formating code for logformat.
Amos Jeffries [Sun, 31 Jan 2016 12:05:30 +0000 (01:05 +1300)]
SourceLayout: rename ClientSocketContext to Http::Stream
... and provided through http/libsquid-http.la.
The name is chosen to match the RFC7540 HTTP/2 "stream" terminology.
Which defines a stream as a bi-directional transaction, including request,
reply and all related 1xx informational and/or control messages.
That same word "stream" is also used in RFC7230 briefly to describe the
same "transaction" scope and details. But not formalized until RFC7540.
Http::Stream's may be initiated by a client HTTP request, Squid internally,
or in HTTP/2 a server PUSH_PROMISE frame.
There are no logic changes in this. Just symbol renaming and move.
Amos Jeffries [Fri, 29 Jan 2016 12:49:28 +0000 (01:49 +1300)]
SourceLayout: rename cert_valid.pl to security_fake_certverify
* creates src/security/cert_validators/ for certificate validation helpers.
To distinguish from certificate generator helpers which would be in
src/security/cert_generators/.
* renames cert_valid.pl to securiy_fake_certverify inline with the helper
naming schema.
* moves helpers/ssl/ to src/security/cert_validators/fake/ as it is the fake
helper.
- building the man(8) documentation that was missing previously.
* adds a ./configure option --enable-security-cert-validator-helpers= to allow
the bundled certverify helper(s) to be managed at build time just like any other.
- this involves addition of the modules.m4, requires.m4 and Makefile.am
infrastructire that helpers/ssl/ was previously lacking.
Alex Rousskov [Thu, 28 Jan 2016 01:49:55 +0000 (18:49 -0700)]
Fixed cleanup of a shared memory segment in an unusual configuration.
Squid was not removing squid-squid-page-pool.shm at exit when started
in non-daemon mode (-N). That segment is used in non-SMP mode if
memory_cache_shared is explicitly set to "on" (an unusual
configuration primarily useful for testing).
Markus Mayer [Thu, 28 Jan 2016 01:30:37 +0000 (18:30 -0700)]
Fixed 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.
Amos Jeffries [Tue, 26 Jan 2016 21:02:00 +0000 (10:02 +1300)]
Cleanup: update TLS session pointer types
* rename SSL* pointer to Security::SessionPtr and SSL_Pointer to
SessionPointer as the smart pointer variant. Matching the model
designed for TLS context storage.
* update fd_table .ssl member to a SessionPointer for safer session
pointer deallocation.
* migrate most uses of SSL* to Securit::SessionPtr or auto
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