Alex Rousskov [Mon, 13 May 2013 22:48:23 +0000 (16:48 -0600)]
Major ACL handling update, including the following changes:
* Expressiveness: Two new Boolean ACLs (all-of and any-of) that allow admins
to group ACLs as needed, to express complex conditions more naturally, with
fewer squid.conf lines. Conditions such as "(a or b) and (c or d)" are
easily expressed now. Explicit groups of ACLs of different types can now be
configured, named, and used in any ACL expression.
* Correctness and performance: When a slow ACL (that has suspended checks to
wait for an async lookup) is ready to resume checking, resume checking from
that ACL, instead of rechecking all ACLs for the same action (or the same
squid.conf directive) again.
* Internals: Store ACL-related configurations as an expression tree, streamlining
the code and clearing the way for future math-style/natural ACL conditions
support.
This patch try to fix current current Notes interface and usage.
The changes done having in mind that we need:
1) to add multiple notes with the same key
2) to support 3 different note types: adaptation meta headers, helper notes
and custom notes added by the system administrator
3) to log notes using the %note formating code
4) to use the %note formating code everywhere the formating API is used. For
example use the %note with the request_header_add configuration parameter.
5) to use notes with ACLs.
Details:
- The NotePairs class is not a kid of HttpHeader class anymore. It is
implemented from scratch to cover Helper/adaptation and custom notes needs.
* The new class stores key:value pairs in list. It allow multiple entries
with the same key.
* Includes a find method which return a coma separated list of values
for a given key
- The HttpRequest::helperNotes is now a Refcount of a HttpPairs object
- The HelperReply::notes is now a HttpPairs object
- The AccessLogEntry::notes now is a RefCount of a HttpPairs object, and
stores only the custom notes add by the "note" configuration parameter
- Add the AccessLogEntry::helperNotes which is a RefCount of a HttpPairs object
to store notes added by helpers.
Now the notes added by adaptation or helpers are accessible to format/* code
imediatelly after added. Before this patch are accessible only for logging.
Future work:
- Posible merge AccessLogEntry::notes and AccessLogEntry::helperNotes
- Performance fixes
Alex Rousskov [Wed, 24 Apr 2013 21:22:39 +0000 (15:22 -0600)]
Prevent external_acl.cc "inBackground" assertion on queue overloads.
The enqueue check for external ACL lookups was inconsistent with the final
queue length check in ExternalACLLookup::Start(). The former allowed adding to
the already full (but not yet overflowing) queue while the latter
rightfully(?) asserted that the queue should not overflow.
The code changes to prevent several useless allocations on missing
sslproxy_cert_error directive left a few lines out of place.
Shuffle cert.sslErrors setup back to the original code sequence and
define allowDomainMismatch early as false for the default handling.
Some systems like GNU Hurd provide teh mmap() API but lack MAP_NORESERVE
support. This option is an optimization, so we can define the macro
ourselves to nil and apparently not suffer (many) bad side effects.
Bug 3816: SSL_get_certificate call inside Ssl::verifySslCertificate crashes squi
d
The SSL_get_certificate implementation in OpenSSL 1.0.1d and 1.0.1e releases,
will crash if called before the certificate sent to the client.
This patch add a hack when one of the problematic OpenSSL versions used to
retrieve the certificate directly from SSL_CTX object, instead of creating
a temporary SSL object, and call SSL_get_certificate.
Docs: remove Squid prefix from ntlm_auth example paths
There is no longer a helper called ntlm_auth shipped with Squid and the
example configs are supposed to be referring to the Samba helper now
as demonstrated by the command line options.
/usr/bin/ is probably not the only location where Samba installs its
helper, but is easily recognised and better than prefixing with a custom
Squid-specific path from --prefix.
* Mostly adding DEFAULT_DOC directive to hide strange default values.
This wll help us move to different internal values for no-limit etc
at some point in the future.
* Migrates some access controls to "DEFAULT: none" instead of "deny all".
This reduces run-time CPU cycles running useless ACL tests.
NP: some access controls have been left unchanged due to complexity in
the code testing them (ie icap_access).
* added documentation for several directives which were missing it
* corrected buffered_logs documentation (text by Alex Rousskov)
* Updated cf_gen tool to produce more descriptive error messages.
Bug 3817: Memory leak in SSL cert validate for alt_name peer certs
Inside function Ssl::matchX509CommonNames which checks a domain name against
certificate common name and alternate names, if the domain matches any of the
alternate names the function return without releasing allocated data.
Current OpenBSD implementation of PF divert-to works similarly to TPROXY
and only requires a getsockname() lookup to locate the TCP packet
original destination.
We can use the same PF configuration to preform "intercept" option but
the old PF transparent code does lookups on /dev/pf which fails badly
on the new PF versions. getsockname() is what is really required and
already performed by TcpAcceptor on all incoming connections, so there
is no need for a special PF lookup code now.
Add a new ./configure option --with-nat-devpf to enable the old /dev/pf
NAT lookup code in a backward-compatible way for older OS versions and
OpenBSD based distros which have not yet ported the new PF code. The
option is disabled by default since the systems requiring it are fairly
old now.
Also remove the getsockname() lookup in the IPFW lookup implementation
which is redundant behind TcpAcceptor.
Michal Luscon [Tue, 16 Apr 2013 00:26:10 +0000 (18:26 -0600)]
Bug 3825: ncsa_auth segfaulting with glibc-2.17
It appears the crypt() function may return NULL strings. Check for those
before all strcmp() operations.
NOTE: The MD5 output checks are probably not needed but since SquidMD5 is
an object build-time switched between several encryption library API
definitions it is better to be safe here as well.
Polish: upgrade TunnelStateData to CBDATA_CLASS2()
CBDATA_CLASS2() removes the need to define new/delete operators and
removes soem uses of cbdataFree()/cbdataAlloc()
Also replaces several abuses of the cbdataInternal*() locking API with
CbcPointer<> auto-pointers. The existence of some of these locks needing
to be done is questionable since AsyncCalls scheduling protects better
against the 'deleted under our feets' problem.
For now the locks are retained since it is not yet easy to track down
which are safe and which are removable.
* convert the C-style tunnelStateFree() functio to a proper destructor.
* create a proper constructor for TunnelStateData
* include debugging for trace-job.pl to track tunnel jobs setup/teardown
Bug 3643: NTLM helpers stuck in reserved state by Safari
NTLM failures are not always cleaning up connection-auth credentials
properly. In particular they are not releasing the NTLM helpers when
the connection is closed between challenge and handshake completion.
Resulting in permanently reserved helpers locking up all access
through the proxy.
This change redesigns the connection authentication state management
to move the auth link/unlink operations into the connection state
manager objects instead of being managed by NTLM auth components.
As a result we are able to manage credentials from any auth scheme
consistently and terminate the connection properly on several
error conditions which the auth components are not easily aware of.
Fix sponsored by Netbox Blue Pty (http://netboxblue.com/)
Fix enter_suid/leave_suid build errors in ip/Intercept.cc
Intercept.cc:210: error: 'enter_suid' was not declared in this scope
Intercept.cc:217: error: 'leave_suid' was not declared in this scope
We should just be including tools.h, but on some GCC the src/tools.h and
src/ip/tools.h include locations clash when building inside src/ip/.
For now we must reference the src/tools.h by its long path.
Amos Jeffries [Sat, 30 Mar 2013 09:47:23 +0000 (03:47 -0600)]
Polish default http_access lines ordering
There is no reason why manager access should be excluded from CONNECT and
Safe_ports security checks. Also, under the new design manager ACL is a
REGEX pattern test, which may be quite slow.
Overall there should be better performnce under certain DoS condtions
having the manager tests after the port tests, with no change under the
other more common traffic.
This adds support for the OpenBSD 'divert' target in PF which presents
the client and remote IPs directly to Squid in accept() parameters
the way Linux TPROXY target does.
It also adds support for the SO_BINDANY option on outgoing traffic for
client IP address spoofing which completes teh TPROXY behaviour.
To enable these features Squid is configured the same as Linux TPROXY:
http_port 1234 tproxy
Amos Jeffries [Thu, 28 Mar 2013 03:16:05 +0000 (21:16 -0600)]
Fix memory leaks in ConnStateData pinning
ConnStateData does not cleanup any of the pinning child structure fields.
This results in a leak of pinned host and peer details.
Run the full un-pin cleanup operation from ConnStateData::swanSong in
order to clean up all the pinning state instead of just the release the
Comm::Connection.
This fix keeps Rock cache_dirs visible to Cache Manager after reconfigure. We
still lack proper support for complex reconfiguration changes involving
cache_dirs.
Amos Jeffries [Tue, 26 Mar 2013 10:33:10 +0000 (04:33 -0600)]
HTTP/1.1: partial support for no-cache and private= controls with parameters
Since we now support HTTP/1.1 storage and revalidation of
Cache-Control:no-cache it is important that we at least detect the cases
where no-cache= and private= contain parameters.
These are likely still rare occurances due to the historic lack of
support. So for now Squid just detects and exempts these responses from
the caching performed. The basic framework for adding handling of the
header lists is made available but not at this time used.
Amos Jeffries [Wed, 20 Mar 2013 04:48:17 +0000 (22:48 -0600)]
Fix bogus 'invalid response' message on URL rewriter interface
The empty-line response from rewriter and redirector should be converted
to ERR reply code in the new API. It was being left as Unknown.
While this reply used to only be valid on URL helper interface, and it
woudl be more appropriate to map other helpers to BH the ERR response
seems to be safe for use on any of the helper interfaces for an empty
line response. At worst it will prevent the lookup being re-tried on
other possibly better working helper instance.
Amos Jeffries [Mon, 18 Mar 2013 10:10:13 +0000 (04:10 -0600)]
Polish: clarify authenticate_ip_ttl code
This patch alters the directive implementation to only perform TTL
addition when setting the expiry value. This improves speed a little when
comparing timestamps, and allows the config file to display 1 second TTL
instead of displaying 0 seconds and actually being 0-1 seconds.
Which resolves some confusion about why max_user_ip ACL still works when
the TTL is set to 0 seconds.
Also, document the AuthUserIP class used to store the IP information.
Amos Jeffries [Mon, 18 Mar 2013 04:55:51 +0000 (22:55 -0600)]
SourceLayout: shuffle HttpStatusLine into http/libsquid-http.la
* moves HttpStatusLine.* to http/StatusLine.*
* renames HttpStatusLine to Http::StatusLine
* renames httpStatusLine*() functions as members of Http::StatusLine
* shuffles StatusCode string conversion function into http/StatusCode
* makes reason parameter of StatusLine::set() function optional.
There is no logic change involved but callers now no longer need to
set it to the status code string explicitly, nor need to set it to NULL
explicitly unless intending to replace an existing status string.
* adds const-correctness and documentation to StatusLine symbols.
Alex Rousskov [Thu, 14 Mar 2013 23:04:37 +0000 (17:04 -0600)]
Fix concurrency support in stateless helpers: Parse multiple replies correctly.
When multiple helper replies were read at the same time, the old code moved \0
(former EoM mark) in front of the buffer after handling the first reply, which
prevented remaining replies from being parsed.
The code also did not terminate the remaining replies correctly after moving
them to the beginning of the buffer. As far as I could test, such termination
is accidentally(?) not necessary, but I could not figure out why and added it
anyway.
Amos Jeffries [Mon, 11 Mar 2013 23:28:51 +0000 (17:28 -0600)]
Fix SSL Bump bypass for intercepted traffic
The SSL-bump bypass code on intercepted HTTPS traffic generates a fake
CONNECT request from the original destination IP:port in an attempt to
trigger a TCP tunnel being opened for the un-bumped data to be
transferred over.
The current implementation breaks in two situations:
1) when IPv6 traffic is intercepted
The URL field generated does not account for the additional []
requirements involved when IPv6+port are combined.
The resulting fake requests look like:
CONNECT ::1:443 HTTP/1.1
Host: ::1
... which are both invalid, and will fail to parse. Breaking IPv6 HTTPS
interception bypass.
Resolve this by using Ip::Address::ToURL() function which was created
for the purpose of generating URL hostnames from raw-IP + port with
the bracketing inserted when required.
2) when a non-443 port is being intercepted
The Host: header generated is missing the port and Squid Host: header
validity will reject the outbound
CONNECT 127.0.0.1:8443 HTTP/1.1
Host: 127.0.0.1
... this is an invalid request. Squid is currently ignoring the Host
header. However Squid tunnel.cc does make use of peering and may relay
the fake request Host: to upstream peers where we cannot be so sure what
will happen.
Resolve this issue by re-using the generated IP:port string for both URL
and Host: fields, which preserves teh port in Host: regardless of value.
This also means there is an unnecessary :443 tagged on for most HTTPS
traffic, however the omission of port from the Host: header is only a MAY
and this should not cause any issues.
Amos Jeffries [Thu, 7 Mar 2013 23:40:02 +0000 (12:40 +1300)]
Regression fix: Accept-Language header parse
When handling error page negotiation the header parse to detect language
code can enter into an infinite loop. Recover the 3.1 series behaviour
and fix an additional pre-existing off-by-1 error.
The errors were introduced in trunk rev.11496 in 3.2.0.9.
Amos Jeffries [Sun, 3 Mar 2013 12:44:30 +0000 (05:44 -0700)]
Fix authentication headers sent on peer digest requests
Cache digest fetches have been sending the cache_peer login= option
value without sanitizing it for special-case values used internally
by Squid. This causes authentication failure on peers which are checking
user credentials.
Tianyin Xu [Sun, 3 Mar 2013 07:10:22 +0000 (00:10 -0700)]
Make all the parameter names and options case sensitive
Changes "strcasecmp" to "strcmp".
This mainly deals with constant configuration options (e.g., enumerative
options and boolean options). For directive names, it's already
consistent (case sensitive), the parser functions are auto-generated.
The case sensitivity of the following parameter values is not changed:
- user and group names
- host names
- domain and realm names
- ACL names
- filesystem names
- options in request/response/digest messages
The cases which were earlier causing a lot of RAM 'leaks' have been
resolved already and the remaining causes appear to all be in components
with short packet handling pathways where the orphan is not wasting much
in the way of RAM bytes or FD time.
The trace is left at level-4 for future debugging if necessary.
Amos Jeffries [Tue, 26 Feb 2013 00:34:52 +0000 (13:34 +1300)]
MacOS: reduce the testRock unit test UDS path
On MacOS shm_open() requires the name entry to be less than 31 bytes
long. The garbage name used by testRock was 35 bytes and not really
describing what it was used for in the test anyway.
TODO: find out and fix why MacOS still responds EINVAL once the path
is set to a usable length.
Amos Jeffries [Sun, 24 Feb 2013 07:26:26 +0000 (00:26 -0700)]
MacOS: workaround compiler errors and case-insensitivity
MacOS GCC version implicitly searches the local directory for .h includes
despite the absence of -I. in the provided options.
Furthermore it searches with case-insensitive filenames due to the
underlying case-insensitive filesystem.
The combined result is that libacl .cc files include their local copy of
acl/Url.h instead of the base directories src/URL.h which was needed.
The long term fix will be to shuffle URL.h and its related code into
a convenience library. For now we can avoid issues by prefixing the full
src/ path to the includes.
Amos Jeffries [Fri, 22 Feb 2013 13:26:12 +0000 (02:26 +1300)]
SourceLayout: shuffle BasicAuthQueueNode to Auth:: namespace
... and document what it is used for by authentication.
There is only one logic change in this patch. The QueueNode destructor
is added to clear the queued CBDATA entries when the queue is deleted.
Previously the pointer was just erased in hopes that the queue was
notified prior to deletion.
Amos Jeffries [Mon, 18 Feb 2013 13:02:42 +0000 (02:02 +1300)]
Removes the domain from the cache_peer server pconn key
Under the squid-3.2 pconn model the IP:port specifying the destination
are part of the key and can be used to strictly filter selection when
locating pconn. This means the domain is no longer a necessary part
of the key.
Squid using cache_peer can see a large number of wasted idle connections
to their peers due to the key domain value if the peer hostname is not
substituted properly. There is also a similar affect when contacting
servers with virtual hosted domains.
Also a bug was located with peer host and name= values being used
inconsistently as the domain marker. Resulting in failed pop() operations
and extra FD usage.
This has been tested for several months now with only socket usage
benefits seen in several production networks.
NOTE: previous experience some years back with pconn has demonstrated
several broken web servers which assume all requests on a persistent
connection are for the same virtual host. For now this change avoids
altering the behaviour on DIRECT traffic for this reason.
Since debugs() is a macro, it should not change static Debugs::level
before putting the debug message to the internal stream. Otherwise we
encounter problems when debug message itself containg calls to debugs().
projects / thirdparty / squid.git / log
