Amos Jeffries [Thu, 26 Feb 2015 13:19:35 +0000 (05:19 -0800)]
Parser-NG: HTTP Response Parser upgrade
1) convert the HTTP server read buffer to an SBuf using the same design
and Comm::Read API implemented previously for the client connections.
The buffer remains default initialized at 16KB per connection but is no
longer absolutely limited to 256KB. Instead it is limited by
configuration options controlling maximum server input sizes on
read_ahead_gap and response message headers.
The Client API has been extended with a new method to estimate size
requirements of an SBuf I/O buffer. Modelled on and deprecating the
existing MemBuf estimator.
The Comm::ReadNow() API is extended to allow limited-size read(2)
operations by setting the CommIoCbParams::size parameter.
The HttpStateData buffer is partially detached from
StoreEntry::delayAwareRead() API due to requirements of the
Comm::ReadNow() API. Instead StoreEntry::bytesWanted() is used directly
to determine read(2) size, and DeferredRead are generated only when
ReadNow() is actually and immediately to be deferred. Theoretically this
means less read operations get deferred in some high load cases.
Practically it means there is no longer an AsyncCall queue plus socket
wait delay between delay_pools promising a read size, doing the
read(2), and accounting for the bytes received - accuracy should be much
improved under load.
This introduces one temporary performance regression converting the SBuf
content to MemBuf for chunked decoder to process.
2) add Http1::ResponseParser class for parsing HTTP response messages.
Modelled on the same design as used for the HTTP RequestParser, and
inheriting from a mutual parent Http1::Parser.
The Parser is Tokeniser based, incremental and 'consumes' bytes out of
the buffer as they are parsed.
The Tokenizer int64 API is updated to handle limited-length scans and
optional +/- symbols.
This Parser class recognises HTTP/1.x and ICY/1 syntax messages. Any
unknown syntax input is assumed to be HTTP "0.9" and it will
gateway/transform the response to HTTP/1.1.
NOTE: these are all semantic actions performed by the code being
replaced in (3). Only the form and OO scoping has changed.
The mime header block detection operation is generalized into the
Http1::Parser for use by both RequestParser and ResponseParser. The
request_parse_status error code has also been adapted for shared use.
3) integrate the HTTP1::ResponseParser with HttpStateData server
response processing.
This is largely code shuffling. Though I have extended the EOF \r\n hack
such that it enables Squid to parse truncated response headers now.
Amos Jeffries [Thu, 26 Feb 2015 10:37:41 +0000 (02:37 -0800)]
Bug 2741 (partial): Initial libsecurity API
The first step(s) towards a generic TLS/SSL security API for Squid.
Creates the basic security/libsecurity.la library and Security::
namespace infrastructure. Symbols provided by this API are always
available instead of conditionally compiled (unlike the ssl/* code for
OpenSSL use).
Merge the TLS/SSL context parameters into a Security::PeerOptions
object instead of maintaining multiple member variables in the
CachePeer and SquidConfig objects.
Squid now provides an error if SSL-specific squid.conf parameters are
used for a Squid without OpenSSL support, instead of silently ignoring
them.
Amos Jeffries [Thu, 26 Feb 2015 06:05:02 +0000 (22:05 -0800)]
Bug 2907: high CPU usage on CONNECT when using delay pools
When delay pools are active on a CONNECT tunnel and the pool is drained
the I/O loop cycles very often transferring 1 byte until the pool is
topped-up at the end of the second.
Instead of looping constantly trying to read 1 byte at a time, add an
asynchronous event to wait for a few I/O cycles or until more bytes can
be read.
To protect against infinite loops of waiting when many tunnels are
competing for the pool allowance we only delay for a limited number of
loops before allowing at least 1 byte through. Also, the amount of time
waited is an odd fraction of 1 second so re-tries naturally spread
across any given second fairly, with connections rotating closer or
further from the time when pool topup happens. That behaviour still
allows some variance in service times, but overall the CPU consumption
and (as a result) total proxy speed appears to be much improved.
NP: Initial production testing shows a 36% RPS speed increase,
with a 50% reduction in total CPU usage.
Squid closes the SSL client connection with "Failed to start fake CONNECT
request for ssl spliced connection". This happens especially often when
the pipeline_prefetch configuration parameter is set to "0" (i.e., default).
When a transparent SSL connection is peeked and then spliced in step2, we are
generating a fake CONNECT request. The fake CONNECT request is counted as a
new pipelined request and may exceed the configured limit. This patch solves
this problem by raising the limit for that request.
Needs more work to better identify the requests that need a different limit.
Joshua Root [Wed, 25 Feb 2015 13:32:14 +0000 (14:32 +0100)]
Bug 3805: support shared memory on MacOS X in Mem::IPC::Segment
MacOS X doesn't support the O_TRUNC flag to shm_open; it is redundant anyway
because the shared memory segment is truncated immediately after opening
as per best practices. With this support Squid can now be built and run
under MacOS X.
Amos Jeffries [Mon, 23 Feb 2015 08:09:21 +0000 (00:09 -0800)]
Allow removal of delayed read events on connection close
.. also use the TunnelStateData directly instead of wrapped in
generic_cbdata so the event engine can identify dead tunnels.
Its a cbdata class anyway so the generic_cbdata was redundant.
Amos Jeffries [Mon, 23 Feb 2015 06:34:49 +0000 (22:34 -0800)]
Remove cache_peer_domain directive
Identical functionality is provided through cache_peer_access.
While this check appears at face value to be simpler than ACLs, the
reality is that:
* the difference is simply the time it takes to initialize and destruct
an on-stack Checklist,
* processing the checks may take longer than ACLs (linked-list of string
comparisons vs single tree lookup),
* ACLs are the common case due to their extra flexibility, and
* extra work is being done per-transaction just to check which of the
two features is in use.
By removing we gain less code and configuration directives to work
around in the long term.
Amos Jeffries [Sat, 21 Feb 2015 12:29:16 +0000 (04:29 -0800)]
Use RefCount::dereference() correctly in move assignment
The dereference() member is actually an update operation on the stored
pointer. We can and should just use it to steal the others pointer
instead of using it on our own then stealing.
Amos Jeffries [Sat, 21 Feb 2015 03:31:22 +0000 (19:31 -0800)]
Bug 2907: high CPU usage on CONNECT when using delay pools
When delay pools are active on a CONNECT tunnel and teh pool is drained
the I/O loop cycles very often transferring 1 byte until the pool is
topped-up at the end of the second.
Instead of looping constantly trying to read 1 byte at a time, add an
asynchronous event to wait for a few I/O cycles or until more bytes can
be read.
To protect against infinite loops of waiting when many tunnels are
competing for the pool allowance we only delay for a limited number of
loops before allowing at least 1 byte through. Also, the amount of time
waited is an odd fraction of 1 second so re-tries naturally spread
across any given second fairly, with connections rotating closer or
further from the time when pool topup happens.
Amos Jeffries [Thu, 19 Feb 2015 02:50:51 +0000 (18:50 -0800)]
basic_nis_auth: fail authentication on crypt() failures
... instead of crashing the helper.
"
Starting with glibc 2.17 (eglibc 2.17), crypt() fails with EINVAL (w/
NULL return) if the salt violates specifications. Additionally, on
FIPS-140 enabled Linux systems, DES or MD5 encrypted passwords passed to
crypt() fail with EPERM (w/ NULL return).
"
Amos Jeffries [Thu, 19 Feb 2015 02:48:23 +0000 (18:48 -0800)]
basic_getpwnam_auth: fail authentication on crypt() failures
... instead of crashing the helper.
"
Starting with glibc 2.17 (eglibc 2.17), crypt() fails with EINVAL (w/
NULL return) if the salt violates specifications. Additionally, on
FIPS-140 enabled Linux systems, DES or MD5 encrypted passwords passed to
crypt() fail with EPERM (w/ NULL return).
"
Problem description:
- Squid sslproxy_options deny the use of TLSv1_2 SSL protocol:
sslproxy_options NO_TLSv1_2
- Squid uses peek mode for bumped connections.
- Web client sends an TLSv1_2 hello message and squid in peek mode, forwards
the client hello message to server
- Web server respond with an TLSv1_2 hello message
- Squid while parsing server hello message aborts with an error because
sslproxy_options deny the use ot TLSv1_2 protocol.
This patch fixes squid to ignore sslproxy_options when peek or stare bumping
mode selected on bumpStep2 bumping step.
The sslproxy_options applied if bump (server-first or client-first) mode
selected on bumpStep1 or bumpStep2 bumping step.
Also applied for "GET https://..." requests.
The original intent for this option was to improve caching. However
HTTP/1.1 permits caching of authenticated messages under conditions
which Squid does check for and obey already.
The legacy popularity of this option from old Squid without the HTTP/1.1
compliant behaviour is now just forming a security and privacy abuse.
Amos Jeffries [Tue, 10 Feb 2015 22:55:58 +0000 (14:55 -0800)]
Parser-NG: HTTP request-line parser replacement
Converts the request-line parse method from a char* string parser to
using ::Parser::Tokenizer based processing.
* The characters for each token are now limited to the RFC 7230
compliant values. The URI is taken as a whole token and characters which
are valid in only one sub-token segment are accepted regardless of their
position. In relaxed parse that is extended beyond the valid URI
characters to include the whitespace characters.
* Whitespace tolerance is extended to include "binary" whitespace VTAB,
HTAB, CR and FF characters specified in RFC 7230.
* The Squid specific tolerance for whitespace prefix to method is
removed. RFC 2730 clarifies that tolerance before request-line is
specfifically and only for whole empty lines (sequences of CRLF or LF).
* The unit tests are extended to check strict and relaxed parse within
the new characterset limits. Drip-feed incremental test updated to check
both parser modes explicitly.
* ::Parser:Tokenizer is extended with methods to skip or retrieve a
token at the suffix of the stored buffer. This is used by the whitespace
tolerant parse to process the URL and HTTP-version tokens from the line
"backwards" from the LF position.
CoAdvisor and Polygraph show no differences. Which is expected since
coadvisor does not test RFC 7230 edge cases (yet), and polygraph is not
stressing incremental parse capabilities.
Eldar Akchurin [Tue, 10 Feb 2015 03:44:32 +0000 (19:44 -0800)]
Bug 4073: Cygwin compile errors
Remove the definition of _SQUID_WINDOWS_ for Cygwin builds. The blend
of win32 and Linux environments is sufficiently different to have major
build issues. We have a precedent in kFreeBSD blend of BSD and Linux to
consider Cygwin a blend and first-class OS.
Also, temporarily disable the Win32-specific libraries and objects until
they can be properly tested.
Fix some small remaining compile errors after the above.
Cygwin Windows build is sponsored by Diladele B.V.
Amos Jeffries [Fri, 6 Feb 2015 21:23:19 +0000 (13:23 -0800)]
Fix for Coverity Scan false positives in SBuf
Coverity scanner gets badly confused with SBuf::npos being used as
default parameter value, even though its used to indicate that
strlen() needs to be used on the string.
This is an experiment to see if it gets less confused by having
explicit overloads for the two cases and not using SBuf::npos value
in relation to the unknown length c-strings.
Amos Jeffries [Fri, 6 Feb 2015 20:09:03 +0000 (12:09 -0800)]
Fix crash when parsing invalid squid.conf
If a time value is going to overflow with default units the
critical debugs() can trigger a segfault instead of logging
and aborting Squid with self_destruct().
Amos Jeffries [Fri, 6 Feb 2015 12:46:54 +0000 (04:46 -0800)]
Add tolerance for whitespace within URI
RFC 7231 advises that there are still clients failing to properly encode
characters within URI. Tolerant parsers should accept that, and later
'reject' with a redirection to a properly encoded form of the URL.
Amos Jeffries [Wed, 4 Feb 2015 21:37:28 +0000 (13:37 -0800)]
Drop unused cbdata.h definitions and re-document
Remove the now unused cbdataFree, cbdataAlloc, CBDATA_TYPE,
CBDATA_INIT_TYPE, CBDATA_INIT_TYPE_FREECB symbols.
Re-write CBDATA documentation to reflect the current available
API symbols, their usage, and mechanisms that should be used
instead of CBDATA such as AsyncJob/Call and RefCount.
Along with some doxygen polishing to meet currently agreed
style for how to document major code features.
Make generic_cbdata::data a private member. The constructor
and unwrap() operator provide all necessary public API.
Replace store_client.cc use of cbdataInternalLock/Unlock with
a CbcPointer<> smart pointer equivalent. The use remains an
abuse of CBDATA, just no longer directly referencing the
internal API functions.
Amos Jeffries [Wed, 4 Feb 2015 17:38:27 +0000 (09:38 -0800)]
Fix some cbdataFree related memory leaks
The delete operator should have been called for these objects after
previous code changes converted them to CBDATA_CLASS. As a result any
member objects relying on their destructor to cleanup were being leaked.
Also, make generic_cbdata::data a private member. The unwrap() method is
easily used now.
Amos Jeffries [Sun, 1 Feb 2015 21:25:46 +0000 (13:25 -0800)]
Cleanup: migrate CachePeer to CBDATA_CLASS API
Replace the alloc/free for CachePeer with new/delete from
the CBDATA_CLASS API.
Shuffle class member default values to constructor.
Shuffle class cleanup code from the (3!) different mechanisms
where it was being done to the class destructor. Also
releasing some memory which was previously leaked on
reconfigure.
Drop the now unused CBDUNL type definition and peerDestroy()
cleanup handler for CachePeer.
Amos Jeffries [Sat, 31 Jan 2015 19:09:22 +0000 (08:09 +1300)]
Per-rule refresh_pattern matching statistics
.. to make it blindingly obvious from the cachemgr report which rules
are completely useless. Such as when the global dot pattern (.) is
placed ahead of custom rules, or one rules pattern is always a subset
of an earlier pattern.
This also allows sysadmin to tune refresh_pattern ordering so most
commonly matching rules are first.
Amos Jeffries [Sat, 31 Jan 2015 14:10:25 +0000 (06:10 -0800)]
Stop emitting (Proxy-)Authentication-Info for Negotiate
This header is not defined for use by RFC 4559, and there seem to
be no clients actually using it.
The syntax Squid was using to emit the details was also clashing
with the syntax defined for use in Digest which is becoming the
standardized ABNF syntax for the header in general.