Amos Jeffries [Mon, 14 Jan 2013 05:01:04 +0000 (22:01 -0700)]
MemPools: remove zero on allocate (calloc) from some pool types
As we are closing defects identified by Coverity and improving
constructors everywhere we are creating a minor anti-pattern in
MemPool'ed objects with calloc() in the pool initializing the memory
then constructors re-initializing it in a better way for that object.
MemPools contains a doZeroOnPush flag to optimize performance by removing
use of memset() as chunks are added back into the pool. However, on
closer inspection it is clear that the following pop() process to re-use
those chunks is never performing memset() anyway. As such I believe that
there is no special need to use calloc() on these particular object types
in the first place.
Update MemPools to use malloc() instead of calloc() on all types with
doZeroOnPush set. This should increase performance a little, and allows
us to remove the anti-pattern by setting doZeroOnPush for more objects
as we can verify they are correctly initialized by their constructors.
Alex Rousskov [Sat, 12 Jan 2013 18:34:10 +0000 (11:34 -0700)]
Fix "address.GetPort() != 0" assertion for helpers on FreeBSD (at least).
The order (or set of?) #include files used by src/ssl/helper.cc (and
probably by other helper source files) has changed recently, exposing a
defines.h dependency on sys/socket.h where that system header is
required to define AF_UNIX. With AF_UNIX incorrectly undefined,
IPC_STREAM was set to IPC_TCP_SOCKET instead of IPC_UNIX_STREAM, and
helpers that do not have a notion of a listening port, were trying to
create communication sockets using TCP streams, triggering a "must have
a port" assertion in comm_connect_addr() called from ipcCreate().
TODO: Moving IPC_* defines into an IPC-specific header file may be a
better solution then exposing all defines.h users to sys/socket.h.
Amos Jeffries [Fri, 11 Jan 2013 05:05:52 +0000 (22:05 -0700)]
WCCP: Fix memory leak in mask assignment, improve debuggsing.
* Release temporary weight array allocated on each HERE_I_AM packet sent
by Squid. For mask assignment these were not released properly and may
build up to a significant size of memory over time.
* Add debug traces to send() events to report failures sending packets
* Also, on HERE_I_AM event send() failure, reduce the timeout to 2sec
for the retry in a crude attempt to prevent router state flapping.
* Silence compiler warnings on use of connect() to disconnect a socket.
Inconsistent OS behaviour makes the result useless in this case.
Amos Jeffries [Tue, 8 Jan 2013 23:51:02 +0000 (16:51 -0700)]
Bug 3731: use 'int' on all systems settign TOS value.
FreeBSD is confirmed erroring out on 8-bit variable size. Other BSD are
documented in a way that implies they do as well, although not at this
stage confirmed to be failing.
Linux seems to be the only confirmed system workign with 8-bit size sent
to setsockopt(). So we revert this to 'int' (32-bit or 64-bit) as was
working in Squid 3.1.
Alex Rousskov [Wed, 2 Jan 2013 18:50:24 +0000 (11:50 -0700)]
Fixed SMP cache mgr request handling.
Coordinator could not successfully parse any SMP cache manager request since
r12410 "Polish: Http::MethodType upgrade" because the parsing code assumed the
presence of the request method ID field that was no longer added by the
request packing code.
Amos Jeffries [Tue, 1 Jan 2013 03:20:47 +0000 (20:20 -0700)]
Fix memory leaks in ICMP
InitAddrInfo() and GetAddrInfo() both allocate addrinfo structs which
must be freed by FreeAddrInfo(). Several places in ICMP were not doing
that free step.
Detected by Coverity Scan. Issues 740434, 740435, 740436, 740437.
Amos Jeffries [Mon, 31 Dec 2012 07:42:32 +0000 (20:42 +1300)]
squidpurge: fix META TLV parsing issues
* current Squid may stoe objects with up to 64KB URLs and 64KB headers
in the disk cache. Increas the tool buffer to 128KB to prevent
truncating the loaded meta data.
* check for and report when meta data overruns the end of loaded buffer
content. Ignore the truncated TLV entry and produce a WARNING.
* validate the TLV size field loaded from disk to prevent buffer overrun
errors from corrupted files on disk.
Fix some issues revealed by Coverty Scan, improve documentation of parseHttpRequest
Address issues for the defect classes:
- Array compared against 0
- Copy into fixed size buffer
- Dereference after null check
- Dereference before null check
Amos Jeffries [Thu, 27 Dec 2012 10:17:04 +0000 (03:17 -0700)]
Fix various issues in unit tests
* Define MemObject stub constructor to initialize teh stub object properly
apparently store unit tests needs one defined. Best to make it work and
set base values than leave garbage in the object fields.
* Buffer overrun on config parser if test is ever given a too-long
string input.
* Memory leak in HttpRequest testing. One instance of a short array.
* Range: header testing may thor exceptions which were not caught by the
test binary. Could lead to difficulty debugging exception errors.
Detected by Coverity Scan. Issues 740523, 740482, 740440, 740498
Amos Jeffries [Thu, 27 Dec 2012 01:42:48 +0000 (14:42 +1300)]
kqueue: update status from experimental to fully available net I/O method
kqueue has been in use on FreeBSD and maybe others for some time now and
has less bugs than epoll. So the issues on record should not be held
against it.
The attached patch adds auto-detection for the kqueue dependencies and
enables it by default when it can build. Unfortunately due to the
dependencies we cannot add it to maximus layer for force-enable, but the
default layer will test it on FreeBSD at least. It is still less
preferred than epoll(), but more than select() and poll().
Alex Rousskov [Fri, 14 Dec 2012 16:10:05 +0000 (09:10 -0700)]
Polished helper messages debugging slightly.
Made stateful and stateless debugging a little bit more similar/symmetric.
Debug helperStatefulSubmit message contents length.
Debug accumulated helperStatefulHandleRead buffer.
Polished helperHandleRead buffer debugging.
Alex Rousskov [Fri, 14 Dec 2012 16:07:18 +0000 (09:07 -0700)]
Added Raw, an std::ostream manipulator to print possibly non-terminated buffers
and their labels/sizes.
The Raw manipulator tries to be smart about printing buffers at various
debugging levels: Large buffers are only printed at DBG_DATA by default.
This allows the caller to mix higher-level debug messages with dumping
of potentially large volumes of data. This smartness can be overruled
using an explicit minLevel() method call.
This patch add cache to cert validation helper. The following new options
added to "sslcrtvalidator_program" configuration parameter to control cache
behaviour:
ttl=n TTL in seconds for cached results.The default is 60 secs
cache=n limit the result cache size. The default value is 2048
To implement the cert validation cache a new template class investigated,
the LruMap which implements a simple lru cache.
The LruMap templete class also used to replace the old Ssl::LocalContextStorage
class which implements a SSL contexts cache.
Amos Jeffries [Sat, 8 Dec 2012 01:50:46 +0000 (14:50 +1300)]
Fix helper buffer management
Identified by Tsantilas Christos.
"One byte remains in helpers io buffer after parsing the response. This is
will cause problems when the next response from helper will enter squid.
After the srv->rbuf parsed, the srv->rbuf still contains on byte (a '\0'
char) and the srv->roffset is 1."
Perform memmove() in helperHandleRead() instead of helperReturnBuffer()
* helperReturnBuffer is only dealing with the message sub-string, but has
been made to memmove() the buffer contents which means it has to become
aware of these problematic terminal \0 octet(s). However
helperHandleRead() is where the termination is being done and the
buffer information is all being handled.
Pass the first of the termination \0 octets to helperReturnBuffer() not
the last \0. This is made possible after fixing (1a).
* helpers which return \r\n were still passing the location of the \n as
endpoint to workaround (1a) even though the \r is also replaced with
\0 and shortens the msg portion by one octet. This affects the
HelperReply parser length checks on responses without kv-pair.
Also, clear out an obsolete TODO.
Also, document a remaining design issue in stateful helpers assuming each
read() operation is on response and ignoring any octets in the buffer
after parsing one reply.
SSL server certificate validator implementation: Ssl::CertValidationMsg::parseResponse fix
The parsed SSL errors returned by cert validator checking is wrong and causes
the Ssl::CertValidationMsg::parseResponse method to return false on valid
messages
This patch add the "server_ssl_cert_fingerprint" acl type to match against
server SSL certificate fingerprint.
The new acl type has the form:
acl aclname server_ssl_cert_fingerprint [-sha1] fingerprint1 ...
The fingerprint must given in the form:
XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX
where X are any valid hexadecimal number
Example usage:
acl BrokeServer dst 192.168.1.23
acl GoodCert server_ssl_cert_fingerprint AB:2A:82:AF:46:AE:1F:31:21:74:65:BF:56:47:25:D1:87:51:41:AE
sslproxy_cert_error allow BrokeServer GoodCert
sslproxy_cert_error deny all
This patch implements the certificate validation helper interface described at:
http://wiki.squid-cache.org/Features/SslServerCertValidator
The helper consulted after the internal OpenSSL validation, regardless of the
validation results. The helper will receive:
1) the origin server certificate [chain],
2) the intended domain name, and
3) a list of OpenSSL validation errors (if any).
If the helper decides to honor an OpenSSL error or report another validation
error(s), the helper will return:
1) A list of certificates.
2) A list of items consists the the validation error name (see %err_name
error page macro and %err_details logformat code), error reason
(%ssl_lib_error macro), and the offending certificate.
The returned information mimics what the internal OpenSSL-based validation code
collects now. Returned errors, if any, fed to sslproxy_cert_error, triggering
the existing SSL error processing code.
The helper invocation controlled by the "sslcrtvalidator_program" and
"sslcrtvalidator_children" configurations options which are similar to the
ssl_crtd related options.
A simple testing cert validation helper developed in perl included in this
patch. This helper just echo back the certificate errors.
Amos Jeffries [Fri, 30 Nov 2012 23:02:11 +0000 (12:02 +1300)]
Add kv-pair support to url_rewrite_helper interface
This stage of the helper reply protocol adds kv-pair support to the
url_rewrite_helper interfacefor URL redirect and rewrite operations.
It uses the new Notes objects and kv-pair field added by the stage 2
helper protocol instead of parsing the 'other' field. Although,
the 'other' field is still parsed when *no* result field is received
for backward compatibility with older helpers.
The response syntax for URL helpers becomes:
[channel-ID SP] result [SP kv-pair ...] [SP other] EOL
NP: 'other' field is now deprecated and will be ignored/discarded on any
response containing a result code field.
When result code "OK" is presented by the helper several kv-pairs are
reserved to control Squid actions:
* "rewrite-url=" is added to return a re-written URL.
- When this key is presented the URL is re-written to the new one by
Squid without client interaction.
- The 'url' keys presence will override this key.
* "url=" is added to return the redirected-to URL.
- When this key name is presented an HTTP redirect response is generated
for the client.
- This keys presence overrides the 'rewrite-url' key actions.
* "status=" is added to hold the HTTP status code for use in redirect
operations.
- This field is optional and status is no longer required for marking
redirect actions.
- If no redirect status is provided Squid will assign one (currently the
default is 302, that may change in the future).
- This key is only relevant when 'url' key is also presented. In all
other uses it is currently ignored.
When result codes BH or ERR are presented by the helper no redirect or
rewrite action is performed and no kv-pair key names are reserved for use
at this time.
Any other keys MAY be sent on any response. The URL helper interface
makes no other use of them, but this patch does pass them on to the ALE
object for logging as transaction Notes. All kv-pairs returned by the
helper (including the url, stauts rewrite-url keys) are available for
logging via the %{...}note log format option.
As with changes to other helpers interfaces in earlier updates, only
the first value presented for any of the reserved kv-pairs is used.
Multiple values are accepted as notes, but otherwise ignored by Squid and
do not affect the transaction outcome.
Additionally, when the BH result code is received from the helper a
simple recovery is attempted. The lookup request will be re-scheduled (up
to once) in an attempt to find a better responding helper.
NOTE: Helper notes are *not* passed to adaptation interfaces.
- REQMOD adaptation happens before URL helpers are used.
- REPMOD adaptation may at some point receive them, but that change is
not done by this update.
Amos Jeffries [Fri, 30 Nov 2012 11:57:23 +0000 (04:57 -0700)]
Polish: Improve the messages output by UFS swap log management.
It also includes a small local variable symbol change from "new_path" to
"tmp_path" to prevent future developer mistakes like the one seen in
bug 3663 mistaking "new_path" for the new destination path of xrename().
projects / thirdparty / squid.git / log
