Bump SSL client on [more] errors encountered before ssl_bump evaluation
... such as ERR_ACCESS_DENIED with HTTP/403 Forbidden triggered by an
http_access deny rule match.
The old code allowed ssl_bump step1 rules to be evaluated in the
presence of an error. An ssl_bump splicing decision would then trigger
the useless "send the error to the client now" processing logic instead
of going down the "to serve an error, bump the client first" path.
Furthermore, the ssl_bump evaluation result itself could be surprising
to the admin because ssl_bump (and most other) rules are not meant to be
evaluated for a transaction in an error state. This complicated triage.
Also polished an important comment to clarify that we want to bump on
error if (and only if) the SslBump feature is applicable to the failed
transaction (i.e., if the ssl_bump rules would have been evaluated if
there were no prior errors). The old comment could have been
misinterpreted that ssl_bump rules must be evaluated to allow an
"ssl_bump splice" match to hide the error.
Amos Jeffries [Sun, 5 Feb 2017 05:57:32 +0000 (18:57 +1300)]
Crypto-NG: initial GnuTLS support for encrypted server connections
Make significant changes to how the options= config settings are
handled internally since GnuTLS does not expose the priority_t
implementation details like OpenSSL. They are also applied to the
session object instead of to the context.
The Security::SessionPointer is converted to std::shared_ptr. This is
required because GnuTLS does not expose the locking like OpenSSL. Since
we store the SessionPointer to fde::Table::ssl we can always access it
from there one way or another and there is actually no need for OpenSSL
locking sessions now.
Most of the remaining session lifecycle logic is moved to
security/Session.* and given a generic API. Only some client-connection
and SSL-Bump related setup remains in ssl/.
A fair amount more debug is added along with some text changes doing
s/SSL/TLS/ in code comments and debug outputs.
Amos Jeffries [Sat, 4 Feb 2017 16:30:49 +0000 (05:30 +1300)]
Cleanup: remove unsused fd_debug_t and fdd_table
This tables used to store the file and line location where each FD was
closed from, but never read for any reports or logs.
Make the cache.log debugging for comm_close display that information
instead of storing it in memory. This way when debug of the comm layer
is enabled the closure information for each FD open/close cycle is
recorded and not just the place of last closure.
Also we save a few hundred KB of memory this table was using and all the
CPU cycles used to maintain it when debugging was not needed.
Amos Jeffries [Sat, 4 Feb 2017 11:16:36 +0000 (00:16 +1300)]
Cleanup: update class fde and _fde_disk to C++11
This update moves the initialization of most class members into default
initizliation values. So clear() can be optimized to a fast emplace
initialization by the compiler and removing the need for memset().
NP: implicit initialization of function pointers and char* arrays is not
supported by C++11. So use of default constructor is still required for now.
Also,
- replace internal uses of fd_table macro with fde::Table.
- replace use of NULL with nullptr
- update some method declaratiosn to current coding style
- improve inline ability for noteUse() method
- const correctness for several inline methods
- documentation updates
Alex Rousskov [Tue, 31 Jan 2017 00:46:31 +0000 (17:46 -0700)]
Fix crash when configuring with invalid delay_parameters restore value.
... like none/none. Introduced in r13701 which fixed another, much
bigger delay_parameters parsing bug.
TODO: Reject all invalid input, including restore/max of "-/100".
TODO: Fix misleading/wrong associated error messages. For example:
ERROR: invalid delay rate 'none/none'. Expecting restore/max or 'none'
ERROR: restore rate in '1/none' is not a number.
This method was improperly used in contexts where actually
Auth::UserRequest::setDenyMessage() expected. Probably the reason was
that both denyMessage() and getDenyMessage() were not constant,
provoking such 'misuse'.
Also placed some common code into UserRequest::denyMessageFromHelper(),
eliminating code duplication. Though there are still many places
where code is duplicated inside auth/ntlm/UserRequest.cc and
auth/negotiate/UserRequest.cc.
author: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Two new ACLs implemented: annotate_transaction and annotate_client.
Both ACLs always match and are useful for their side effect, immediately
adding a key-value pair to the current transaction annotation
(annotate_transaction) or to the current client-to-Squid connection
annotation (annotate_client). Connection annotations are propagated to
the current and all future master transactions on the annotated
connection. Before this patch only 'clt_conn_tag' annotation tag could
be used for a connection annotation.
To reuse the existing notes parsing code, I had to refactor Note, Notes
and NotePairs classes:
* Made data members private and adjusted to follow 'rule of three'.
Having public assess to containers with pointers may cause memory
problems: for example ExternalACLEntry::update() called directly
notes.entries.clear() without deleting the pointers.
* None-fatal check for 'special' characters inside note name.
* Used SBufs instead of Strings and const char* where possible.
* Adjusted ACLNoteStrategy::matchNotes() to avoid 'expanding quoted values'
code duplication inside
Also fixed acl quoted flag parameters syntax. The old code improperly
required quoting both flag and its parameter, e.g., "-m= ," whereas
only parameter should be quoted: -m=" ,".
Also moved UpdateRequestNotes() from Notes.cc to HttpRequest.cc to
resolve dependency problems while bulding unit tests.
TODO: transaction annotation matching code (ACLNoteData) performs
parsing in its own way, using ACLStringData::parse(), lacking special
characters/reserved keywords checks. Consider reusing the existing
Notes parsing code instead.
SSLv2 records force SslBump bumping despite a matching step2 peek rule.
If Squid receives a valid TLS Hello encapsulated into ancient SSLv2
records (observed on Solaris 10), the old code ignored the step2 peek
decision and bumped the transaction instead.
Now Squid peeks (or stares) at the origin server as configured, even
after detecting (and parsing) SSLv2 records.
Mitigate DoS attacks that use client-initiated SSL/TLS renegotiation.
There is a well-known DoS attack using client-initiated SSL/TLS
renegotiation. The severety or uniqueness of this attack method
is disputed, but many believe it is serious/real.
There is even a (disputed) CVE 2011-1473:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-1473
The old Squid code tried to disable client-initiated renegotiation, but
it did not work reliably (or at all), depending on Squid version, due
to OpenSSL API changes and conflicting SslBump callbacks. That
code is now removed and client-initiated renegotiations are allowed.
With this change, Squid aborts the TLS connection, with a level-1 ERROR
message if the rate of client-initiated renegotiate requests exceeds
5 requests in 10 seconds (approximately). This protection and the rate
limit are currently hard-coded but the rate is not expected to be
exceeded under normal circumstances.
Amos Jeffries [Tue, 17 Jan 2017 04:31:41 +0000 (17:31 +1300)]
Initialize options= in all cases
Since we now have long and ptr in different builds we cannot rely on the
default constructor always being correct.
Adjust the parseOptions() method to handle empty options= silently for
OpenSSL buidls (like GnutTLS builds did already) and use it to initialize
the parsedOptions member.
Fix "Source and destination overlap in memcpy" Valgrind errors
Before this patch, source and destination arguments in
log_quoted_string() could point to the same static memory area, causing
multiple Valgrind-reported errors. Fixed by creating another buffer to
store quoted-processed output string.
Amos Jeffries [Fri, 13 Jan 2017 05:14:03 +0000 (18:14 +1300)]
Remove USE_CHUNKEDMEMPOOLS compiler flag
The USE_CHUNKEDMEMPOOLS build-time setting is not very useful and adds
extra complexity to the build system. Even when set it does not always
enable chunked pools. The environment variable MEMPOOLS can easily be
used to enable or disable chunked pools as needed.
The helper protocol for external ACLs [1] defines three possible return values:
OK - Success. ACL test matches.
ERR - Success. ACL test fails to match.
BH - Failure. The helper encountered a problem.
The external acl helpers distributed with squid currently doesn't follow this
definition. For example, upon connection error, ERR is returned:
$ ext_ldap_group_acl ... -d
ext_ldap_group_acl: WARNING: could not bind to binddn 'Can't contact LDAP server'
ERR
This is does not allow to distinguish "no match" and "error" either and
therefore negative caches "ERR", also in the case of an error.
Moreover there are multiple problems inside squid when trying to handle BH
responses:
- Squid-5 and squid-4 retries requests for BH responses but crashes after the
maximum retry number (currently 2) is reached.
- If an external acl helper return always BH (eg because the LDAP server is
down) squid sends infinitely new request to the helper.
Reduce crashes due to unexpected ClientHttpRequest termination.
The underlying problem has been known since r13480: If a
ClientHttpRequest job ends without Http::Stream (and ConnStateData)
knowledge, then Squid is likely to segfault or assert. This patch does
not resolve the underlying issue (a proper fix would require
architectural changes in a consensus-lacking area) but makes an
unexpected ClientHttpRequest job destruction less likely.
BodyPipe and Adaptation-related exceptions are the major causes of
unexpected ClientHttpRequest job destruction. This patch handles them by
closing the client connection. Connection closure should trigger an
orderly top-down cleanup, including Http::Stream, ConnStateData, and
ClientHttpRequest destruction.
If there is no connection to close, then the exception is essentially
ignored with a level-1 error message disclosing the problem. The side
effects of ignoring such exceptions are unknown, but without a client
connection, it is our hope that they would be relatively benign.
Amos Jeffries [Sun, 8 Jan 2017 05:14:34 +0000 (18:14 +1300)]
Cleanup: remove --disable-inline and related macros
With the .cci files gone there is no longer any use of the SQUID_INLINE
macro. Which in turn removes the need for _USE_INLINE_ macro and then the
--disable-inline build option which sets that.
The relevant compiler specific *FLAGS option should be used instead to
disable compiler inlining functions without special macro usage.
Bug 4599 pt6: Restrict safe renegotiation disable to OpenSSL 1.0 and older
From OpenSSL 1.1.0 documentation:
"OpenSSL always attempts to use secure renegotiation as described in RFC5746.
This counters the prefix attack described in CVE-2009-3555 and elsewhere."
From OpenSSL changeLog:
Remove support for all 40 and 56 bit ciphers. This includes all the export
ciphers who are no longer supported and drops support the ephemeral RSA key
exchange. The LOW ciphers currently doesn't have any ciphers in it.
[Kurt Roeckx]
OpenSSL 1.1.0 drops EXPORT ciphers and ephemeral RSA. The ssl_temp_rsa_cb used
here. The OpenSSL manual for this function is:
https://wiki.openssl.org/index.php/Manual:SSL_CTX_set_tmp_rsa_callback(3)
Amos Jeffries [Fri, 30 Dec 2016 06:27:19 +0000 (19:27 +1300)]
Detect HTTP header ACL issues
rep_header and req_header ACL types cannot match multiple different
headers in one test (unlike Squid-2 appears to have done). Produce
an ERROR and ignore the extra line(s) instead of silently changing
all the previous regex to match the second header name.
Also detect and ERROR when header name is missing entirely. Ignore
these lines instead of asserting.
Amos Jeffries [Thu, 22 Dec 2016 04:08:55 +0000 (17:08 +1300)]
Bug 3940 pt2: Make 'cache deny' do what is documented
Instead of overriding whatever cacheability was previously set to
(including changing non-cacheables to be cacheable) actually
prevent both cache read and write.
Amos Jeffries [Wed, 21 Dec 2016 17:11:34 +0000 (06:11 +1300)]
Cleanup: Refactor libauth Config object(s)
Refactoring the Auth::Config objects to get v5 building again.
The object storing auth_param settings is renamed to SchemeConfig.
A new Auth::Config object is added. The objects holding auth_param,
auth_schemes and other authenticate_* directives settings are stored
there instead of just auth_params.
Lots of outdated doxygen docs that have more up to date copies elsewhere
are removed. The "\ingroup AuthAPI" are left for now since some auth
things are not yet in the Auth:: namespace, but other auth related
groups are dropped completely.
Code parsing and dumping auth_param lines has been de-duplicated. Along
with the keep_alive and utf8 settings, which are shared by multiple
schemes. This allows dropping several virtual methods from NTLM and
Negotiate scheme objects.
An auth/forward.h file is added for predefines. Some initial Auth::
symbols are listed there. It is not complete, but others can be added as
needed later.
Some unnecessary includes have been removed. There are probably more. I
just did the obvious ones related to the new auth/forward.h and
auth/Config.h files at this time.
Amos Jeffries [Mon, 19 Dec 2016 10:47:37 +0000 (23:47 +1300)]
Drop old Basic Authenticatio docs
The wiki these days contains a more detailed and correct description of
how Basic auth helpers work in Squid. This old document does not even
describe how the code works, so is no longer useful.
Amos Jeffries [Mon, 19 Dec 2016 10:27:54 +0000 (23:27 +1300)]
Shuffle auth_schemes config objects into Auth::Config
Also, move the directive definition down into "Access Controls" section
of squid.conf to avoid dependency issues with acl directive definitions
being done too late.
projects / thirdparty / squid.git / log
