Amos Jeffries [Sat, 31 May 2014 17:00:05 +0000 (10:00 -0700)]
Cleanup: de-duplicate auth_param program parameter code
Moves the "program" parse and dump code into Auth::Config.
Also, changes API to Auth::Config::dump() to not dump any config settings
for schemes which are not configured with a "program". Including scheme
specific settings.
Also, fixes missing Digest "utf8" parameter in config dump.
Move realm parse and config dump logics to Auth::Config base object.
This de-duplicates Basic, Digest (and future schemes ie Bearer) config
processing code. Also makes realm available to NTLM and Negotiate
schemes, although at present it remains unused by those schemes.
Also, convert the realm parameter string to an SBuf. Removing the need
for some memory maintenance code.
Alex Rousskov [Fri, 23 May 2014 06:34:59 +0000 (00:34 -0600)]
Revised ftp-gw timeout handling to cope with very long data downloads/uploads
that triggered bogus ctrl connection timeouts due to ctrl channel inactivity.
Unset ctrl timeout when we are done waiting for the ctrl response
(ServerStateData::readControlReply).
Removed code setting data timeout from Ftp::ServerStateData::dataRead()
because the data timeout is set in ServerStateData::maybeReadVirginBody() that
dataRead calls to read data.
Removed switchTimeoutToDataChannel() from
Ftp::Gateway::ServerStateData::startDataDownload because
* ctrl timeout should be cleared when we are done waiting for the ctrl
response (ServerStateData::readControlReply) and
* data timeout should be set when we start waiting for the data
(ServerStateData::maybeReadVirginBody)
- Defines three steps of the SSL bumping processing:
step1: Get TCP-level and CONNECT info. Evaluate ssl_bump and perform
the first matching action (splice, bump, peek, stare, terminate,
or err)
step2: Get SSL Client Hello info. Evaluate ssl_bump and perform the
first matching action (splice, bump, peek, stare, terminate,
or err). Peeking usually prevents future bumping. Staring
usually prevents future splicing.
step3: Get SSL Server Hello info. Evaluate ssl_bump and perform the
first matching action (splice, bump, terminate, or err).
In most cases, the only remaining choice at this step is
whether to terminate the connection. The splicing or bumping
decision is usually dictated by either peeking or staring at the
previous step.
- The ssl_bump ACLs list may evaluated in all SSL Bumping processing steps to
take a decision for the next step:
splice or none: Become a TCP tunnel without decoding the connection.
bump: Establish a secure connection with the server and, using a
mimicked server certificate, with the client
peek: Receive client (step1) or server (step2) certificate while
preserving the possibility of splicing the connection. Peeking
at the server certificate usually precludes future bumping of
the connection.
stare: Receive client (step1) or server (step2) certificate while
preserving the possibility of bumping the connection. Staring at
the server certificate usually precludes future splicing of the
connection.
terminate or err: Close client and server connections.
All actions except peek and stare correspond to final decisions: Once an
ssl_bump directive with a final action matches, no further ssl_bump
evaluations will take place, regardless of the current processing step.
- Add the atstep acl to match against SSL bumping step: "step1", "step2" or
"step3"
Current Implementation details:
---------------------------------
1) If the "peek" mode selected in step2 then the client hello message
forwarded to server. If this mode selected in step2 the splice is always
possible and bump maybe is not possible (in most cases where the client uses
different SSL client library implementation)
2) If the "stare" mode selected in step2 then the squid builds a new
hello message, which try to mimic, if it is possible , client hello message.
If stare selected in step2 the bump is always possible, but splice maybe is
not possible any more.
3) In step3 if bump decided, and bump is not possible any more then squid
is always splicing.
4) In step3 if splice decided but splice is not possible any more then
squid is always bumping.
5) Because of (3) and (4), in practice, if firefox browser used with
peek mode, squid always splice the connection, because squid/openSSL
does not support the firefox SSL features reported in client hello message.
6) In step2 if ACL list evaluation result to terminate or err then we just
close client connection. If the check result to ssl-bump then just bump.
If check result to client-first, server-first, then bump the connection
else do peek/stare.
7) In step3 the ssl_bump ACL list evakuation result client-first, server-first,
bump or peek result to bumping (if bumping is possible).
Amos Jeffries [Thu, 22 May 2014 06:04:05 +0000 (23:04 -0700)]
Cleanup: drop Auth::User::proxy_auth_list header cache
This list/cache was originally used to short-circuit auth helper lookups
based on previousy seen [Proxy-]Authorization header strings.
However, that permitted replay attacks in most auth schemes and has been
replaced by scheme-specific mechanisms:
* Basic and Digest credentials are cached in the global user name cache
wih additional nonce/password comparisons to verify located entries.
* NTLM and Negotiate credentials are cached in the ConnStateData with
exact-match comparison done to verify tokens.
After r13324 patch the SBuf argument of the ConnStateData::handleReadData member
is used only to check if ConnStateData::In::buf is correctly filled with read
data. ConnStateData::handleReadData considers that the data already written
in ConnStateData::in.buf and checks if the passed Sbuf argument is the
ConnStateData::in.buf:
The httpsSslBumpAccessCheckDone function needs to write the CONNECT request
generated internally to force tunnel mode, in ConnStateData::In::buf and then
call ConnStateData::handleReadData method.
Amos Jeffries [Tue, 20 May 2014 11:00:04 +0000 (04:00 -0700)]
Cleanup: drop parsedCount_ tracking
Now that parse() is receiving a buffer directly we no longer have to
track how many bytes have been consumed by the parse. It can be
calculated by comparing the current and original SBuf.
Amos Jeffries [Sun, 18 May 2014 10:36:05 +0000 (03:36 -0700)]
Fix infinite parse loop on partial request reads
parseHttpRequest() returns NULL on incomplete parse. This case was not
exiting the loop to parse multiple requests. As a result traffic would
only receive a response if the request headers were received entirely
within one read(2) event. Pipelined requests received over multiple hung.
Amos Jeffries [Thu, 15 May 2014 10:44:05 +0000 (03:44 -0700)]
Fix outstanding build issues and parser audit results
* Give SBuf I/O buffer directly to Http1::RequestParser
* Redesign parser state engine to represent the current state
being parsed instead of previous completed. This allows much
more incremental resume of a parse and reliable consume() of
the input buffer as sections complete instead of complex byte
accounting outide the parser.
* Maintain an internal counter of bytes parsed and consumed by
the parser instead of a buffer offset. This allows much more
reliable positioning of the state/section boundaries.
* Remove erroneous fprintf debug left in previous commit.
* Redesign HttpRequestMethod constructor to drop end parameter.
* Redesign all parser unit tests. Marking RFC non-compliance
for future fixing.
author: Alex Rousskov <rousskov@measurement-factory.com>
Avoid on-exit crashes when adaptation is enabled.
After trunk r13269 (Vector refactor) destroyed vector objects still have
positive item counts. This exposes use-after-delete bugs. In this particular
case, global adaptation rule/group/service arrays are destructed by global
destruction sequence first and then again by Adaptation::*::TheConfig objects
destructors.
This change avoiding static destruction order dependencies by storing those
global adaptation arrays on heap.
Alex Rousskov [Thu, 8 May 2014 22:43:01 +0000 (16:43 -0600)]
Temporary fix for segmentation faults in FwdState::serverClosed.
r13388 (cache_peer standby=N) moved noteUses() call from Comm to FwdState, to
avoid exposing Comm to pconn pools. Unfortunately, the closing handler does
not get a valid FD value when the closing callback shares the Connection
object with the code that called conn->close(). It gets -1. The FD of the
FwdState connection itself is already -1 at that point, for similar reasons.
The code thinks it got a matching FD and calls noteUses() with an invalid FD.
This temporary workaround prevents noteUses() calls when FD is unknown.
Without those calls, pconn usage statistics will be wrong. A different
long-term solution is needed.
Amos Jeffries [Wed, 7 May 2014 10:05:58 +0000 (03:05 -0700)]
Cleanup: Refactor external_acl_type format codes representation
Removes enum_external_acl_format::format_type from external_acl.cc
by replacing it with enum Format::ByteCode_t.
Several missing logformat codes related to URL display have been added
to the logformat token set for general use.
Several of the external ACL format codes have been added to
Format::ByteCode_t without equivalent logformat TokenTableEntry's at
this stage as both desirable token naming and access to the data to
produce them generically is unclear.
The external_acl_type parser is updated to accept logformat tokens
wherever an equivalent exists and map directly to the ByteCode_t values.
The mgr:config report dumper is also updated to output the logformat
tokens. But as yet the official deprecation has not been done in
squid.conf.
A client may hit on an incomplete shared memory cache entry. Such entry is
fully backed by the shared memory cache, but the copy of its data in local RAM
may be trimmed. When that trimMemory() happens, StoreEntry::storeClientType()
assumes DISK_CLIENT due to positive inmem_lo, and the store_client constructor
asserts upon discovering that there is no disk backing.
To improve shared cache effectiveness for "being cached" entries, we need to
prevent local memory trimming while the shared cache entry is being filled
(possibly by another worker, so this is far from trivial!) or, better, stop
using the local memory for entries feeding off the shared memory cache. The
latter would also require revising DISK_CLIENT designation to include entries
backed by a shared memory cache.
This access list is a temporary solution for peek-and-splice project and used to
take the final decision "bump" or "splice" in peek-and-splice bumping mode.
This is what this patch try to do:
- Get Client Hello message
- Start connection.
- Inside bio, before write the SSL HELLO message, try to emulate client hello
message:
a) extract client hello message features
b) Check if we are able support client features and if not, splicing is not
able to be supported.
c) Creates an SSL object to connect to server and try to set it with
the extracted features.
This step currently includes many hacks and modify undocumented SSL
object members.
extensions)
- in PeerConnector.cc
a) If can not be spliced do not splice.
b) check the ssl_bump_peeked access list to splice or not.
Amos Jeffries [Mon, 5 May 2014 08:35:47 +0000 (01:35 -0700)]
Support concurrency channels in Digest authentication helpers
All bundled digest helpers will now automatically detect the existence
of a concurrecy channel-ID and adjust responses appropriately.
The auth_param children concurrency= parameter can now be set to any
valid value without needing to alter the helper binary. This resolves
issues upgrading to default-on concurrency on the digest auth interface.
The HttpMsg::protocol removed with "Bug 1961: pt1: URL handling redesign" patch,
and as a result the eCAP squid subsystem does not build because used this memberto implement libecap::RequestLine and libecap::StatusLine classes.
The HttpMsg::protocol used to hold the protocol part of the request URI.
However the libecap::FirstLine::protocol() is meant for things like
* the HTTP-Version part of HTTP messages (in RFC 2616 terminology) or
* the ICAP-Version part of ICAP messages (in RFC 3507 terminology).
It is not related to the URI.
This patch fix this and now libecap::RequestLine and libecap::StatusLine
implemented to return the protocol information from request or status line
of headers.
Amos Jeffries [Sat, 3 May 2014 10:35:31 +0000 (03:35 -0700)]
Fix generated HTTP message version labels
Squid being conditionally compliant with RFC 2616 should be handling
HTTP/1.1 at all times unless another version was explicitly received.
This makes the default version number for all generated messages be 1.1
unless the alternative constructor is used or the numeric members are
explicitly set to other values. As a result all Squid generated messages
are labelled correctly as 1.1 by default now.
Fixes message version details sent to ICAP/eCAP on many error or
internally generated responses.
author: Alex Rousskov <rousskov@measurement-factory.com>
cache_peer standby=N implementation.
The feature focus is to instantly provide a ready-to-use connection to a
cooperating cache peer, virtually at all times. This is useful when connection
establishment is "too slow" and/or when infrequent peer use prevents Squid from
combating slow connection establishment with the regular idle connection pool.
The feature is similar to Squid2 idle=N feature, but there are key differences:
* Standby connections are available virtually at all times, while Squid2 unused
"idle" connections are available only for a short time after a peer request.
* All N standby connections are not opened at once, reducing the chance of
the feature being mistaken for a DoS attack on a peer.
* More consistent support for peers with multiple IP addresses (peer IPs are
cycled through, just like during regular Squid request forwarding).
Besides, "idle" is a poor choice of adjective for an unused connection pool
name because the same term is used for used persistent connections, which have
somewhat different properties, are stored in a different pool, may need
distinct set of tuning options, etc. It is better to use a dedicated term for
the new feature.
The relationship between the max-conn limit and standby/idle connections is a
complex one. After several rewrites and tests, Squid now obeys max-conn limit
when opening new standby connections and accounts for standby connections when
checking whether to allow peer use. This often works OK, but leads to standby
guarantee violations when non-standby connections approach the limit. The
alternative design where standby code ignores max-conn works better, but is
really difficult to explain and advocate because an admin expects max-conn to
cover all connections and because of the idle connections accounting and
maintenance bugs. We may come back to this when the idle connections code is
fixed.
Fixed max-conn documentation and XXXed a peerHTTPOkay() bug (now in
peerHasConnAvailable()) that results in max-conn limit preventing the use of a
peer with idle persistent connections.
Decided to use standby connections for non-retriable requests. Avoiding
standby connections for POSTs and such would violate the main purpose of the
feature: providing an instant ready-to-use connection. A user does not care
whether it is waiting too long for a GET or POST request. Actually, a user may
care more when their POST requests are delayed (because canceling and
retrying them is often scary from the user point of view). The idea behind
standby connections is that the admin is responsible for avoiding race
conditions by properly configuring the peering Squids. If such proper
configuration is not possible or the consequences of rare races (e.g., due to
peer shutdown) are more severe than the consequences of slow requests, the
admin should not use standby=N. This choice may become configurable in the
future.
TODO: Teach peer probing code to push successful probing connections into the
standby pool (when enabled). Should be done as a followup project because of
the differences in standby and probe connection opening code, especially when
SSL peers are supported. Will require some discussion.
A standby pool is using a full-blown PconnPool object for storage instead of
the smaller IdleConnList, like the ICAP code does. The primary reasons for
this design were:
* A peer may have multiple addresses and those addresses may change. PconnPool
has code to deal with multiple addresses while IdleConnList does not. I do not
think this difference is really used in this implementation, but I did not
want to face an unknown limitation. Note that ICAP does not support multiple
ICAP server addresses.
* PconnPool has reporting (and cache manager integration) code that we should
eventually improve and report standby-specific stats. When this happens,
PconnPool will probably become abstract and spawn two kids, one for pconn and
one for standby pools.
Seemingly unrelated changes triggered by standby=N addition:
* Removed PconnPool from fde.h. We used to create immortal PconnPool objects.
Now, standby pools are destroyed when their peer is destroyed. Sharing raw
pointers to such pools is too dangerous. We could use smart pointers, but
PconnPools do not really belong to such a low-level object like fde IMO.
* Added FwdState::closeServerConnection() to encapsulate server connection
closing code, including the new noteUses() maintenance. Also updated
FwdState::serverClosed() to do the same maintenance.
* Close all connections in IdleConnList upon deletion. The old code did
not care because we never deleted PconnPools (although I am not sure
there were no bugs related to ICAP service pools which use IdleConnList
directly and do get destroyed).
* Fixed PconnPool::dumpHash(). It was listing the first entry twice because
the code misused misnamed hash_next().
* Removed unnecessary hard-coded limit on the number of PconnPools. Use
std::set for their storage.
* Fixed very stale PconnPool::pop() documentation and polished its code.
* Added RegisteredRunner::sync() method to use during Squid reconfiguration:
The existing run() method and destructor are great for the initial
configuration and final shutdown, but do not work well for reconfiguration
when you do not want to completely destroy and then recreate the state.
The sync() method (called via SyncRegistered) can be used for that.
Eventually, the reconfiguration API should present the old "saved" config
and the new "current" config to RegisteredRunners so that they can update
their modules/features intelligently. For now, they just see the new config.
Currently note values printed with "%note" formating code, which contain non
alphanumeric characters, were quoted and quotes were then escaped, resulting
in bizarre logged rendition of empty or simple values (often received from
various helpers):
%22-%22
%22Default_Google%22
%22pg13,US%22
This patch:
- does not use quotes to print annotations
- allow system admin to define a separator to use for logged
annotations. The %note logformat accepts the following argument:
[name][:separator]
The separator can be one of the ',' ';' or ':'.
By default, multiple note values are separated with "," and multiple
notes are separated with "\r\n". When logging named notes with
%{name}note, the explicitly configured separator is used between note
values. When logging all notes with %note, the explicitly configured
separator is used between individual notes. There is currently no way to
specify both value and notes separators when logging all notes with %note.
- makes the Format::Token::data a struct (now is a union) and initialize
Format::Token::data data members in Format::Token::Token constructor.
CbcPointer<> is used from code outside of Job protection where it is
safe to use Must(). In order to get a useful backtrace we need to assert
immediately at the point of failure. Particularly necessary since these
are in generic operators used "everywhere" in the code.
Anatoli [Tue, 29 Apr 2014 18:08:35 +0000 (06:08 +1200)]
Fix order dependency between cache_dir and maximum_object_size
parse_cachedir() has a call to update_maxobjsize() which limits the
store_maxobjsize variable used as the internal maximum_object_size
variable of the store data structure) to the value of maximum_object_size
defined at the moment of execution of this function, for all stores (all
store directories). So if parse for cache_dir is called before
maximum_object_size, we get the effect of the default 4 MB.
BUT, when we get to parse maximum_object_size line(s) after the last
cache_dir, the maximum_object_size option is processed and only shown on
the cachemgr config page without having updated store_maxobjsize.
Replace the HttpMsg::protocol member (only used by HttpRequest) with a
class URL member HttpRequest::url. To do this we adjust the class URL
scheme_ member to be mutable via the makeScheme() setter, and add a
clear() method to reset its internal state. These are necessary for
HttpRequest init() and initHTTP() mechanisms, but fiddling with the
scheme should be avoided as much as possible.
Remove the hack of forcing internal requests to http:// for processing
and cache lookup, then to internal:// for FwdState. Instead use the
available flags.internal for requests identified to be served by this proxy.
Drop the non-standard and now meaningless "internal://" scheme.
Add debugging to display what internal indicators are detected and when
the internal*() server and CacheManager are used by FwdState.
Also document HttpMsg::http_ver which is a copy of the HTTP-Version
field in the "on-wire" message syntax. It is unrelated to the socket
transport protocol and the URL scheme protocol.
Alex Rousskov [Sat, 26 Apr 2014 17:30:33 +0000 (11:30 -0600)]
Document counter-intuitive round-robin cache_dir selection bias; decrease it.
Many squid.confs have at least two groups of cache_dir lines. For example,
rare "large" objects go to larger/slower HDDs while popular "small" objects go
to smaller/fast SSDs:
# HDDs
cache_dir rock /hdd1 ... min-size=large
cache_dir rock /hdd2 ... min-size=large
cache_dir rock /hdd3 ... min-size=large
# SSDs
cache_dir rock /ssd1 ... max-size=large-1
cache_dir rock /ssd2 ... max-size=large-1
cache_dir rock /ssd3 ... max-size=large-1
# rock store does not support least-load yet
store_dir_select_algorithm round-robin
Since round-robin selects the first suitable disk during a sequential scan,
the probability of /hdd1 (/ssd1) selection is higher than other HDDs (SSDs).
Consider a large object that needs an HDD: /hdd1 is selected whenever scan
starts at /ssd1, /ssd2, /ssd3, and /hdd1 while /hdd2 is selected only when the
scan starts at /hdd2 itself! Documentation now warns against the above
cache_dir configuration approach and suggests to interleave cache_dirs:
cache_dir rock /hdd1 ... min-size=large
cache_dir rock /ssd1 ... max-size=large-1
cache_dir rock /hdd2 ... min-size=large
cache_dir rock /ssd2 ... max-size=large-1
cache_dir rock /hdd3 ... min-size=large
cache_dir rock /ssd3 ... max-size=large-1
store_dir_select_algorithm round-robin
Squid's historic implementation of round-robin made its natural bias even
worse because it made the starting point of the sequential scan to be the last
selected dir. In the first configuration example above, it boosted the
probability that the scan will start at one of the SSDs because smaller
objects are more popular and, hence, their dirs are selected more often. With
the starting point usually at an SSD, even more _large_ objects were sent to
/hdd1 compared to other HDDs! The code change avoids this artificial boost
(but the cache_dir lines should still be interleaved to avoid the natural
round-robin bias discussed earlier).
author: Alex Rousskov <rousskov@measurement-factory.com>, Christos Tsantilas <chtsanti@users.sourceforge.net>
Ssl::PeerConnector class
This patch investigates the new Ssl::PeerConnector class. This class connects
Squid client-side to a SSL cache_peer or SSL server. It is used by
TunnelStateData and FwdState to initiate and establish the SSL connection.
This class handles peer certificate validation.
The caller receives a call back with PeerConnectorAnswer. In the SSL connection
is not established because of an error, an error object suitable for error
response generation is attached to PeerConnectorAnser
The Ssl::PeerConnector class includes the old SSL initialization code from
FwdState class.