Amos Jeffries [Mon, 12 Oct 2015 01:38:02 +0000 (18:38 -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.
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.
Only allowing 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
Amos Jeffries [Sun, 11 Oct 2015 14:08:47 +0000 (07:08 -0700)]
Support logformat %macros in external_acl_type format
Update the external_acl_type helper interface to use libformat and thus
make any logformat token valid in its format parameter field.
As a result much of the logic surrounding format code parsing, display
and helper query generation has been completely dropped. What remains is
a basic parse loop handling backward compatibility for the unusual
%CERT_* token syntax, space delimiter and field default encodings.
Extensions to logformat resulting from the merger:
* adds \-escape encoding of output fields
* allows {arg} field to be placed before or after the format code.
* extended to accept the old external_acl_type %macros. But not
documented, these are deprecated and only for backward compatibility.
* extended to support outputting formats without a format-name prefix
as was required by the original logformat config lines.
The major side effect of this change is that these ACLs now require
AccessLogEntry to be filled out with state data, rather than just the
ACLChecklist object members.
The requires*() mechanism of ACLChecklist has been extended to catch
some cases resulting from missing the ALE entirely. But it cannot catch
the more subtle problem of data members inside the ALE being unset.
To try and catch those a syncAle() mechanism has been added that fills
out missing ALE members and prints out debug warnings about the action.
Amos Jeffries [Sun, 11 Oct 2015 13:56:33 +0000 (06:56 -0700)]
TLS: shuffle EECDH configuration to libsecurity
* add class ServerOptions to libsecurity to manage server specific
configuration options. Based on class PeerOptions.
* shuffle the DH config parse and dump logics to ServerOptions
* shuffle the DH params pre-loading logic to ServerOptions
* add configuration warning when tls-dh= is used and overrides
dhparams= logacy configuration. Also, auto-upgrade the config
settings when dhparams= is dumped in mgr:config report.
The new %ssl::<cert_errors logformat code lists server certificate
validation errors detected by Squid (including OpenSSL and the
certificate validation helper components). The errors are listed in
the discovery order. By default, the error codes are separated by ':'.
Custom separators are also supported. For example:
Amos Jeffries [Thu, 8 Oct 2015 12:44:41 +0000 (05:44 -0700)]
Set default pid_filename based on service name
This makes pid_filename directive no longer need to be set explicitly to
the service name in multi-tenant installations. Unless the default value
has been replaced with --with-pidfile=Foo it will use the service name
as the .pid filename.
Amos Jeffries [Thu, 1 Oct 2015 12:58:19 +0000 (05:58 -0700)]
URL-encode the implicit %DATA appended to helper format
There is nothing we can easily do about %DATA explicitly used inside the
format. It will by non-encoded unless specific encoding is written in the
format config, according to logformat design.
Amos Jeffries [Thu, 1 Oct 2015 12:35:09 +0000 (05:35 -0700)]
Fix potential memory leak on GopherStateData constructor errors
In the unusual event that the GopherStateData object constructor fails
it is possible that the destructor gets called without having gone
through the deleteThis() and swangSong() dance. Since the constructor
allocates memory buffer we need to clean that up.
Bug 4190: assertion 'hash_remove_link' from Auth::User::cacheCleanup
The hash_link based cache depends on raw-ptr key comparisons to store
hash entries. This does not work at all well with SBuf as the key,
since the backing MemBlob behind SBuf can change its memory location.
* replace the implementation of User credentials caching with an STL
based container class that can handle SBuf.
* revert the global Auth::User cache design to per-scheme caches
which get combined only when reporting statistics.
* add a RunnersRegistry helper class to control Squid startup,
reconfigure, and shutdown events activity in regards to the caches.
* suppress useless cache garbage collection events when auth has no
credentials to cleanup.
* make the cache key dynamic at the caller codes discretion.
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.
Certificate Revokation Lists have gone through several iterations
of logic redesign leading to duplicated code and non-optimal I/O.
Client contexts were loading CRL directly from disk into the
context on every new context creation. Whereas the server contexts
were loading into an OpenSSL STACK_OF structure and adding from
memory instead of disk. This later design is more performant.
* Move the pre-loaded CRL set to Security::PeerOptions and store
in a std::list structure as LockingPointer which will deallocate
as needed on shutdwown and reconfigure.
This depends on trunk rev.14304
* Replace the client context disk I/O with the pre-loaded CRL list
* Add GnuTLS CRL list types. Though at this point GnuTLS does not
pre-load the CRL files.
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);
Add assigment and move operators to LockingPointer
These operators are required to use LockingPointer instances in STL
containers and unlike TidyPointer the LockingPointer can do them safely
due to the lock preventing premature deletions.
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 try to improve the related code to avoid related
problems in the future.
FILE* handles need to be closed on exit. Shuffle the processing loop logics
to a static function to avoid code duplication from all the requires close
points.
Also, use the available global flag debug_enabled instead of local variable
to avoid having to pass it down explicitly.