Jan Sievers [Thu, 15 Aug 2013 21:28:14 +0000 (15:28 -0600)]
Regression Bug 3077: off-by-one error in Digest header decoding
This is a regression in the 3.2 and later version of the original bug
fix patch.
The effect is that all attributes are treated as quoted-string values
with unescaping performed. This ends with credentials wrongly being
rejected as invalid.
Amos Jeffries [Sat, 3 Aug 2013 07:57:44 +0000 (19:57 +1200)]
Fix configure parsing IP/domain directives
Squid would semi-silently accept invalid IP address or hostname in some
directives which required them (eg wccp_router) and use the magic
IP_ANYADDR value. This change makes configure halt with a FATAL error
instead.
Amos Jeffries [Thu, 1 Aug 2013 20:45:56 +0000 (08:45 +1200)]
Add dns_multicast_local to control mDNS operation
Enable admin control over whether mDNS operates or not. Set the default
to OFF due to .arpa reverse-DNS requests causing a rise in traffic from
this feature even on networks without mDNS responders setup.
Also, polish cachemgr idns report to show for queued queries which
resolver type and query the entry is involving. mDNS can cause a queue
to exist as some lookups timeout on the mDNS resolvers.
Alex Rousskov [Tue, 30 Jul 2013 17:10:57 +0000 (11:10 -0600)]
Prevented STORE_DISK_CLIENT assertions for aborted entries. Polished code.
To prevent store_client.cc:445: "STORE_DISK_CLIENT == getType()" assertions,
rewrote the storeClientNoMoreToSend() function so that it does not send a
STORE_MEM_CLIENT to read from disk. Used this opportunity to polish this
negative function code and convert it into a positive method.
Documented known (and mostly old) StoreEntry::storeClientType() problems.
Specialize SBuf::reserveSpace and reserveCapacity functions to provide guarantee of single store ownership.
Tune SBuf::rawSpace() to take over from reserve* for the optimization role preveiously played by reserve*.
Adjust documentation and callers.
Fix debugs when refresh_pattern is missing the regex pattern
This is a very rare situation and may not be completely required. But it
did hinder debugging of refresh_pattern when the regex field token was
broken by parser updates.
Alex Rousskov [Mon, 29 Jul 2013 00:46:55 +0000 (18:46 -0600)]
Better support for things with shared locks that can be opened many times,
such as Ipc::StoreMap entries. Maintain a lock counter instead of boolean
opened flag.
Better support for things with multipart IDs
such as Ipc::StoreMap entries that have an anchor/inode ID and map name.
Alex Rousskov [Mon, 29 Jul 2013 00:43:55 +0000 (18:43 -0600)]
Re-enabled on-disk collapsing of entries after fixing related code.
Since we started writing partial entries, we cannot rely on negative sidNext
marking the end of the slice/write sequence. Added a WriteRequest::eof field
to signal that end explicitly.
Do not leak db slices when write fails or IoState is closed before the write
succeeds.
Handle store client requesting an offset we have not stored yet. This might
happen for collapsed hits (and also if the client is buggy). May need more
work to slow the reader down.
Do not update various shared stats until the corresponding slot is written.
Alex Rousskov [Mon, 29 Jul 2013 00:27:23 +0000 (18:27 -0600)]
Improved STORE_MEM_CLIENT detection.
IN_MEMORY mem_status does not guarantee that the entore object is in the
memory cache. We may be just loading it from a shared memrory cache, and
loading may fail. We may have nibbled at the entry already (although that may
not be possible, not sure). The whole memory/disk store_client designation
probably needs more work, but the now-removed condition was causing
store_client.cc:445: "STORE_DISK_CLIENT == getType()" assertions.
Alex Rousskov [Sat, 27 Jul 2013 17:19:29 +0000 (11:19 -0600)]
Keep anchor.basics.swap_file_sz in sync with slice sizes.
The old code updated anchor.basics.swap_file_sz _after_ copying all of the
available data into shared memory. An exception in the copying loop (e.g., the
map is out of available slots) could prevent that update. For another worker,
the entry would then appear to be fully completed (no writer, last slice size
stable, and last slice poiner is -1) and that worker would assert due to
anchor.basics.swap_file_sz mismatching the sum of slice sizes.
Handle infinite certificate validation loops caused by OpenSSL bug #3090.
If OpenSSL is stuck in a validation loop, Squid breaks the loop and triggers a
new custom SQUID_X509_V_ERR_INFINITE_VALIDATION SSL validation error. That
error cannot be bypassed using sslproxy_cert_error because to break the loop
Squid has to tell OpenSSL that the certificate is invalid, which terminates
the SSL connection.
Validation loops exceeding SQUID_CERT_VALIDATION_ITERATION_MAX iterations
are deemed infinite. That macro is defined to be 16384, but that default can
be overwritten using CPPFLAGS.
Tied more closely SBuf and MemBlob size_type
Better compliance with standards
Improved SBuf documentation
Improved adherence to std::string method signatures
Improved \0-cleanliness
Removed some methods taking std::string arguments to avoid double data copies,
documented alternate patterns
Implemented cmp and caseCmp shortcuts
Cleaned-up some methods' implementations after Alex Rousskov's suggestions
Improved unit tests' cleanliness by making more effective use of cppunit macros
Implemented more unit tests, better coverage for \0-cleanliness
Update the http(s)_port directives protocol= parameter
... to use AnyP::ProtocolVersion for internal storage instead of opaque
string text.
This both limits the possible parameter values to one of HTTP, HTTP/1.1,
HTTPS or HTTPS/1.1 and ensures that URLs generated from that protocol
parameter value are http:// and https:// URL.
Other values will cause Squid to abort.
Invalid casting seems to confuse the ABI generator and results in
illegal instruction faults when the unit tests is run.
The class API is already const-correct so there is no need for the cast
to occur, and it should not be done on a non-pointer type anyway.
Also, fixes a missing "struct" type identifier found along the way.
Alex Rousskov [Wed, 24 Jul 2013 21:48:45 +0000 (15:48 -0600)]
Disconnect StoreEntries before deleting their memory objects.
The new cleanup order helps identify the write Rock entry state (reading or
writing) and avoid assertions related to state identification bugs (such
as unlocking a writing entry for reading).
Similar to the memory cache code, we should not disconnect disk entries during
shutdown because Store::Root() may be missing by then.
Alex Rousskov [Wed, 24 Jul 2013 21:45:02 +0000 (15:45 -0600)]
Avoid !writeableAnchor_ assertions when Squid shuts down.
A shutting down Squid deletes locked StoreEntry objects, which may trigger
deletion of Rock::IoState that is still writing to disk. We should fix the
shutdown sequence. Meanwhile, the Rock::IoState code does not need to mislead
admins with an assert.
Alex Rousskov [Mon, 22 Jul 2013 17:04:00 +0000 (11:04 -0600)]
Fixed StoreEntry::mayStartSwapOut() logic to handle terminated swapouts.
StoreEntry::mayStartSwapOut() should return true if a swapout can start. If
swapout was started earlier but then terminated for some reason (setting sio
to nil), the method should not return true. Checking swap_status ==
SWAPOUT_DONE does not work reliably because the status may be reset to
SWAPOUT_NONE in some cases (and the check was too late anyway). Checking
decision == swPossible does not work at all because while swapout start was
possible at some point, it is no longer possible after we started swapping
out.
Added MemObject::SwapOut::swStarted to detect started swapouts reliably.
This patch :
- adds support for quoted values in the entire squid.conf
- warn about or prohibit values that can no longer be interpreted as
either quoted strings or simple tokens
- support parameters("/path/to/file.name") syntax to load external
configuration files
- Checks if macros allowed in "double quoted" values.
- replaces the strtok() calls with calls to the new ConfigParser::NextToken()
- modify strtokFile to use new ConfigParser::NextToken()
- Add the new configuration_includes_quoted_values configuration option, to
control the squid parser behaviour. If set to on Squid will recognize each
"quoted string" after a configuration directive as a single parameter
This patch add new logformat codes to log TOS/DSCP values and netfilter marks
for client and server connections. If multiple outgoing connections were used,
the last used connection value logged.
The values printed in hexadecimal form.
The logformat codes are:
%>tos Client connection tos mark set by Squid
%<tos Server connection tos mark set by Squid
%>nfmark Client connection netfilter mark set by Squid
%<nfmark Server connection netfilter mark set by Squid
This patch also modify qos related code to set Comm::Connection::nfmark and
Comm::Connection::tos members in Ip::Qos::setSockNfmark and Ip::Qos::setSockTos
methods. The Comm::Connection members are now set only if the tos and nfmark
set successfuly.
This patch sends an If-None-Match request, when we need to re-validate
if a cached object which has a strong ETag is still valid.
This is also done in the cases an HTTP client request contains HTTP
headers prohibiting a from-cache response (i.e., a "reload" request).
The use of If-None-Match request in this context violates RFC 2616 and
requires using reload-into-ims option within refresh_pattern squid.conf
directive.
The exact definition of a "reload request" and the adjustment/removal of
"reload" headers is the same as currently used for reload-into-ims
option support. This patch is not modifying that code/logic, just adding
an If-None-Match header in addition to the IMS header that Squid already
adds.
Fix external ACL user:pass detail logging after adaptation
When a request is successfully adapted, the external ACL username and
password are now inherited with this patch. This means the
LFT_USER_NAME log token can display the username from an external ACL
if available, for adapted requests.
The HttpRequest will inherit the password for good measure as well -
while none too useful, it seems strange to inherit the username but
not the password.
We can do better than just producing errors about invalid port details
and treatign it as port-0.
We can instead undo the port separation and pass it through as part of
the host name to be verified with the default port number properly
assumed.
Protect against buffer overrun in DNS query generation
see SQUID-2013:2.
This bug has been present as long as the internal DNS component however
most code reaching this point is passing through URL validation first.
With Squid-3.2 Host header verification using DNS directly we may have
problems.
Alex Rousskov [Wed, 10 Jul 2013 00:41:01 +0000 (18:41 -0600)]
Use Rock::IoState::writeableAnchor_ to detect rock entries open for writing.
Just e.mem_obj->swapout.sio presence is not reliable enough because we
may switch from writing to reading while the [writing] sio is still around.
More explicitly disabled on-disk collapsing of entries. The relevant code is
unstable under load [at least when combined with memory caching]. We were not
calling Ipc::StoreMap::startAppending() before so we probably did not fully
disk-collapsed entries before these temporary changes.
Added an XXX to mark an assert() that may fail if we allow on-disk collapsing.
Alan Mizrahi [Tue, 9 Jul 2013 11:15:51 +0000 (05:15 -0600)]
Add storeid_file_rewrite helper
Based on work by Eliezer Croitoru <eliezer@ngtech.co.il>
This program acts as a Store-ID helper program, rewriting URLs passed
by Squid into storage-ids that can be used to achieve better caching
for websites that use different URLs for the same content.
It takes a text file with two tab separated columns.
Column 1: Regular expression to match against the URL
Column 2: Rewrite rule to generate a Store-ID
Rewrite rules are matched in the same order as they appear in the file.
So for best performance, sort it in order of frequency of occurrence.
Alexis Robert [Tue, 9 Jul 2013 10:04:39 +0000 (22:04 +1200)]
Support IPv6 NAT interception on Linux
NAT support has been included for IPv6 in Linux 3.7 (along with
REDIRECT/DNAT rules), as well as IP6T_SO_ORIGINAL_DST in Linux 3.8.
Add support for transparent proxies over IPv6.
There is a bug in linux/netfilter_ipv6/ip6_tables.h on C++ compilers,
the bug report and patch to fix it can be found at
https://lkml.org/lkml/2012/9/30/146.
It is only used for the constant IP6T_SO_ORIGINAL_DST. We attempt to use
the official header whenever possible but if it is detected missing or
broken we define our own version of the option.
IPv6 is now permitted on any http_port or https_port in squid.conf
however on older Linux systems and Unix systems without the required NAT
support Squid will fail when accepting the traffic.
Also, this removes the blocker checks preventing BSD systems using NAT
interception on IPv6 ports. Several version of PF have long since
supported IPv6 NAT operations although it was discouraged, such support
is not easily detected though so results WILL vary by operating system.
Bug 3876: mDNS support segfault when using --disable-ipv6
When IPv6 is disabled the mDNS IPv6 multicast group gets rejected by
idnsAddnameserver() resulting in invalid pointers for the remaining
mDNS NS setup operations.
Convert the hard-coded mDNS nameserver count to dynamic global count and
elide the relevant NS when IPv6 support disabled.
Alex Rousskov [Tue, 2 Jul 2013 19:23:49 +0000 (13:23 -0600)]
Broadcast mem-cache writer departure to transient readers (in more/all cases).
Moved transientsAbandon() call to MemStore::disconnect() to make sure we
catch all cases where a mem-cache writer stops updating the cache entry.
Transient readers need to know so that they do not get stuck when a writer
disappears.
transientsAbandon() needs StoreEntry so MemStore::disconnect requires one now.
Alex Rousskov [Mon, 1 Jul 2013 19:59:32 +0000 (13:59 -0600)]
Do not become a store_client for entries that are not backed by Store.
If we ignore cache backing when becoming a store client, then
StoreEntry::storeClientType() is going to make us a DISK_CLIENT by default.
If there is no disk cache or it cannot be used for our entry, we will assert
in store_client constructor. Prevent those assertions by checking earlier in
StoreEntry::validToSend().
Alex Rousskov [Mon, 1 Jul 2013 02:25:50 +0000 (20:25 -0600)]
Several fixes and improvements to help collapsed forwarding work reliably:
Removed ENTRY_CACHABLE. AFAICT, it was just negating RELEASE_REQUEST AFAICT.
Broadcast transients index instead of key because key may become private.
Squid uses private keys to resolve store_table collisions (among other
things). Thus, a public entry may become private at any time, at any worker.
Using keys results in collapsed entries getting stuck waiting for an update.
The transients index remains constant and can be used for reliable
synchronization.
Using transient index, however, requires storing a pointer to the transient
entry corresponding to that index. Otherwise, there is no API to find the
entry object when a notification comes: Store::Root().get() needs a key.
Mark an entry for release when setting its key from public to private. The old
code was only logging SWAP_LOG_DEL, but we now need to prevent requests in
other workers from collapsing on top of a now-private cache entry. In many
cases, such an entry is in trouble (but not all cases because private keys are
also used for store_table collision resolution).
Fixed syncing of abandoned entries.
Prevent new requests from collapsing on writer-less transient entries.
- The SSL_CTX_new in newer openSSL releases requires a const
'SSL_METHOD *' argument and in older releases requires non const
'SSL_METHD *' argument. Currently we are trying to identify openSSL
version using the OPENSSL_VERSION_NUMBER macro define but we are failing
to correctly identify all cases.
- sk_OPENSSL_PSTRING_value is buggy in early openSSL-1.0.0? releases
causing compile errors to squid.
Amos Jeffries [Sat, 29 Jun 2013 14:43:23 +0000 (08:43 -0600)]
Bug 3762: remove bogus WARNING in cache.log
The warning is bogus for several reasons:
* it appears with memory-only cache configurations
* it only checks the size of first SwapDir (as seen in bug 3762)
* very large memory spaces are now possible which may make disk appear
small by comparison.
Its usefulness in detecting memory and disk misconfigurations has long
been almosy nil. Removing this entirey to resolve the bogus noise in
the above mentinoed legitimate configurations.
Alex Rousskov [Thu, 27 Jun 2013 21:26:57 +0000 (15:26 -0600)]
Tightened StoreEntry locking. Fixed entry touching and synchronization code:
Tightened StoreEntry locking code to use accessors instead of manipulating the
locking counter directly. Helps with locking bugs detection. Do not consider
STORE_PENDING and SWAPOUT_WRITING entries locked by default because it is
confusing and might even leave zero lock_count but locked() entries in the
global table. Entry users should lock them instead.
StoreController::get() is now the only place where we touch() a store entry.
We used to touch entries every time they were locked, which possibly did not
touch some entries often enough (e.g. during Vary mismatches and such where
the get() entry is discarded) and definitely touched some entries too often
(every time the entry was locked multiple times during the same master
transaction). This addresses a design bug marked RBC 20050104.
Fixed interpretation of IN_MEMORY status. The status means that the store
entry was, at some point, fully loaded into memory. And since we prohibit
trimming of IN_MEMORY entries, it should still be fully loaded. Collapsing
changes started to use IN_MEMORY for partially loaded entries, which helps
detecting entries associated with the [shared] memory cache, but goes against
old Squid code assumptions, triggering assertions.
Handle synchronization of entries the worker is writing. Normally, the writing
worker will not receive synchronization notifications (it will send them) but
a stale notification is possible and should not lead to asserts. The worker
writing an entry will see a false mem_obj->smpCollapsed.
Do not re-anchor entries that were already anchored, fully loaded (ioDone),
and are now disassociated from the [shared] memory cache.
For shared caching to work reliably, StoreEntry::setReleaseFlag() should mark
cache entries for future release. We should not wait for release() time.
Waiting creates stuck entries because Squid sometimes changes the key from
public to private and collapsed forwarding broadcasts are incapable of
tracking such key changes (but they are capable of detecting entries abandoned
by their writers via the deletion mark in the transients table).