Amos Jeffries [Wed, 2 Jun 2010 13:44:26 +0000 (01:44 +1200)]
Bug 2305: Multiple leaks and assertion crashes in authentication.
* implements proper RefCounting using the RefCount.h classes for
almost all auth objects in Squid.
* Restructures auth objects with a simpler structure of duties and scopes.
* Prunes away several circular and indirectly circular pointer loops
* Adds an API to auth config for handling the mainRotate() event. To only
shutdown helpers, fixing the loss of cached credentials on rotate.
* Adds a username_cache page to cachemgr interface to display the current
credentials and their TTLs to various revalidation or garbage events.
With this we end up with several global pointers for the auth schemes which
have been built into the current Squid. These are RefCount pointers, fixing
the leak of schemes on shutdown. Schemes are now also permanent structures
for the runtime of Squid, fixing leaks on reconfigure and rotate actions.
These AuthSchemes are responsible for creating auth Config objects for each
auth protocol configured in squid.conf. These config objects are now also
able to be altered with a reconfigure instead of requiring a restart.
Each HTTP request authentication attempt generates AuthUserRequest objects,
which may or may not pointer to an AuthUser set of credentials being checked.
AuthUserRequest is RefCounted instead of locked, fixing several assertion
crashes.
AuthUser is now RefCounted instead of locked. It's children inherit
these properties. This simplifies the object handling a lot and fixes
several assertions.
* This also means AuthUser no longer needs a back-pointer to all
AuthUserRequest in order to see if its still needed alive, fixing one
circular lock loop and a few possible assertions.
* The username cache pointers to only AuthUser objects, fixing a second
cirular lock loop and potentially leakage. Also simplifying the hash cache
handling a lot.
Non-Auth code needing a reference to authentication credentials should
hold a pointer to either an AuthUserRequest or AuthUser object. Not any
other auth object.
FUTURE WORK;
There is still some conditions leading to auth re-challenge when they
are not expected.
A fair chunk of classes and enums have been shuffled into separate files
to keep the scopes clearer. This could be increased in future when
building the Auth namespace.
Potential is now present for simpler TTL handling for all auth types.
This work was a collaboration between multiple interested parties over
the last year, with additional developer time and testing funded by
Netspace Online Systems.
Amos Jeffries [Mon, 31 May 2010 09:32:19 +0000 (21:32 +1200)]
Substitute Perl shell path into Basic auth helper scripts
This fixes these helpers by default on OS where the perl shell is not at
/usr/bin/perl. The problem of cross-compiling shell variation remains as
it was before.
Henrik Nordstrom [Sat, 29 May 2010 22:02:54 +0000 (00:02 +0200)]
Rework memory pools
- Default to the old simple non-chunked pool type using malloc
with a simple freelist per pool.
- Various statistics & cachemgr Memory Utilization fixes
- Source reorganisation to split pool implementations from the general
framework. Allocators now in MemPoolChunked.cc and MemPoolMalloc.cc,
with general framework & statistics in MemPool.cc.
The chunked allocator is still available and can be activated by setting
the environment variable MEMPOOLS=1 but the default is now the non-chunked
allocator which has been fixed to account allocations properly for statistics
and also maintain a simple freelist to cache allocations.
No squid.conf changes, other than that the memory_pools and
memory_pools_limit directives now works as intended (not possible
with the chunked allocator).
Amos Jeffries [Thu, 27 May 2010 00:51:44 +0000 (12:51 +1200)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 2697: Adaptation leaks and extra requests after reconfiguration
This patch "detaches" services from the configuration during reconfiguration.
Detached services do not participate in new adaptation transactions but allow
the old transactions to finish nicely. Once all users are gone, the refcounted
service disappears.
As a side effect of these fixes, several aspects of eCAP service registration
and mapping of loaded and configured eCAP services have been fixed. We will be
able to claim support for eCAP reconfiguration after libecap adds
reconfiguration API.
Amos Jeffries [Tue, 25 May 2010 11:12:20 +0000 (23:12 +1200)]
Author: Wojciech Zatorski <zator@bg.szczecin.pl>
Author: Amos Jeffries <squid3@treenet.co.nz>
Support TPROXYv4 spoofing of X-Forwarded-For client address.
Assumes correct configuration use of the X-Forwarded-For header with
a zone of trusted sources.
SECURITY WARNING:
This patch depends on security features not present in older Squid
versions and is not to be ported or applied to earlier releases.
Amos Jeffries [Tue, 25 May 2010 10:30:55 +0000 (22:30 +1200)]
Author: Mark Nottingham <mnot@pobox.com>
Bug 2631: store-stale refresh_pattern option
RFC2616 allows almost anything to be cached*; most of the constraints are on
what can be used out of cache.
For example, this response:
HTTP/1.1 200 OK
Content-Type: text/html
Date: [date]
[ content ]
is cacheable as per HTTP; it just is considered stale as soon as it is cached.
However, Squid (and many others) don't cache all of these responses, at least
in part because doing so would decrease cache efficiency, and introduce more
load (e.g., on disk subsystems, etc.).
In particular, Squid won't cache a response unless it has either explicit
freshness (e.g., Expires, CC: max-age) or a validator (Last-Modified or ETag).
Nevertheless, doing so is desirable in some circumstances, because in some
deployments the client wants to be able to ask for a stale response. For
example;
GET /foo HTTP/1.1
Host: example.com
Cache-Control: max-age=3600, max-stale
Should retrieve the above response from cache, as long as it is less than an
hour old.
This patch adds a 'store-stale' refresh_pattern option that allows this.
Alex Rousskov [Sun, 23 May 2010 23:04:05 +0000 (17:04 -0600)]
Fixed IpAddress port printing for ports higher than 9999:
snprintf includes zero-terminator in its size limit, so 7
rather than 6 bytes are needed to snprintf a colon followed
by 5 port digits.
Alex Rousskov [Sun, 23 May 2010 16:43:46 +0000 (10:43 -0600)]
Bug 2633 fix: Ecap::HeaderRep::value(name) fails when there is no named header
field
Calling Adaptation::Ecap::HeaderRep::value(const Name &name) with names
of header fields that do not exist leads to
ICAP/AsyncJob.cc(218) dial: AsyncJob::noteStart threw exception:
basic_string::_S_construct NULL not valid
I suspect this is a combination of
- std::string constructor incapable of handling a nil char* pointer.
- String::termedBuf() returning an nil pointer when the string is empty.
When there is no specified header field in the message, the value()
wrapper in Squid gets an empty String for the header value, which is
then used to create std::string, which fails or leads to failure.
I think it is wrong for termedBuf to return nil (because nil is not
0-terminated). I have not tried to fix that because we will have a new
String class soon.
Fixed header accounting to avoid the "Headers[id].stat.aliveCount" assertion.
We were incrementing the alive header field counter twice for each decrement,
which probably resulted in the alive counter wrapping back to zero, triggering
the assertion.
Amos Jeffries [Sat, 22 May 2010 03:55:41 +0000 (15:55 +1200)]
Author: Alex Rousskov <rousskov@measurement-factory.com>
Bug 2879: pt2: 3.0 regression in headers end finding
Consider the case when we received an empty (zero bytes) response.
The committed hack (bzr r9935) makes the "Invalid Response" warning misleading
because it adds CRLF to the empty response. The same hack makes the correct
error determination even more broken than it was (because the rest of the code
now sees content when none exited). It also mentions the wrong bug number.
The attached patch fixes the above and attempts to route empty response
processing to the right error (ERR_ZERO_SIZE_OBJECT).
TODO: Reconsider polluting cache.log with unlimited Invalid Response warnings,
at least in a forward proxy environment where the admin has no control over
responses.
When comm_close() has been called for the server fd but the close handler has
not yet been activated, the Server may receive an async call not associated
with the fd (e.g., more request body data coming from the HTTP or ICAP client)
that prompts the server to write to the fd. We now check whether it is still
safe to write before writing. If it is not safe, we do not write but wait for
our close handler to be called.
TODO: when all comm_write callers check for fd closing, comm API can be
redefined to drop unsafe calls instead of asserting.
Amos Jeffries [Fri, 21 May 2010 04:17:56 +0000 (16:17 +1200)]
Sync socket family type to the outgoing address family.
This evades the COMM_ERR_PROTOCOL results from comm_connect_addr.
Ideally those errors would now be unused, however there are still
components which call comm_connect_addr directly.
Henrik Nordstrom [Fri, 14 May 2010 12:34:31 +0000 (14:34 +0200)]
Reset all addresses as OK after trying them all. This to avoid a "deadlock"
when all addresses of the currnet procotol have been marked bad but
there remains addresses in another protocol which means there is no addresses
we can connect to but unfortunately we don't know that in the current
upside-down layering.
Henrik Nordstrom [Fri, 14 May 2010 12:05:27 +0000 (14:05 +0200)]
Bug #2876: FD_SETSIZE override not working on all linux distributions
The glibc hack for overriding FD_SETSIZE seems to have broken down on some families
of Linux distribution, requiring one more header to be included before redefine.
Hopefully this does not break the FD_SETSIZE override on more systems than
it fixes.. if it does then some additional autoconffuu will be needed.
Henrik Nordstrom [Fri, 14 May 2010 05:37:19 +0000 (07:37 +0200)]
Fall back on IPv4 if IPv6 is not present
automatically fall back on IPv4 operation if it fails creating an
IPv6 socket. This may happen if Squid is built with IPv6 support
enabled but no IPv6 stack is available when it runs.
Henrik Nordstrom [Fri, 14 May 2010 04:04:53 +0000 (06:04 +0200)]
Clean up use of commResetFD when socket incompatible with requested address
This patch backs out part of the patch for Bug #2222 and replaces it by
crudely cycling over the available addresses, trying to skip over
addresses not compatible with the current socket.
This solves issues seen when using tproxy or tcp_outgoing_address and
DNS of the requested host returns AAAA records in addition to A records.
This change is interim, waiting for the larger connection setup
overhaul, but seems to do the trick for now.
One effect of this change is that there will be no fallback to the other
IP generation if the socket is configured to a specific outgoing
address. Priory the code threw away the outgoing address and tried
again when encountering an incompatibility.