The SharedListenResponse because copied using memcpy function (TypedMsgHdr.cc
file, 154 line, Ipc::TypedMsgHdr::getRaw method) can not have complex class
members like the SharedListenResponse::conn which is a RefCounted object.
This patch
- Remove the SharedListenResponse::conn member and replaced with a
single SharedListenResponse::fd (integer filedescriptor) member.
- Does not create a new Comm::Connection object for listening sockets
inside IPC code , but use the Comm:Connection object created while
initializing the listening socket and passed to the Ipc::StartListening
method.
Bug 2051: 'default' cache_peer option does not match documentation
Move the default parent to second-lowest priority on the parent
selection. This also allows the other more delicate balancing
algorithms to work properly with a default configured.
sourcehash and userhash are reversed in priority to simplify the
selection code around #if..#endif once default is moved.
Also, getAnyParent() is dropped. It is redundant with the FIRSTUP
algorithm.
In summary:
* use nonBlockingCheck() or fastCheck() to test ACLs.
* be prepared to handle any allow_t in the result.
ACL testing functions publicly available from ACLChecklist are:
- nonBlockingCheck (public), fastCheck public), check (public but not to be used)
- matchAclListFast (public), matchAclListSlow (private), matchAclList (private).
Given that there are only two types of test performed, this array of API
methods has been causing confusion and mistakes for some developers.
This patch seeks to clarify that API by correcting a flaw in the naming
of check() and matchAclListFast().
Due to "Fast" ACLs coming in two types there are two overloaded
fastCheck() functions. Now with identical output behaviour. Both return
the allow_t result of the lookup. This is expected to _usually_ be
ACCESS_ALLOWED / ACCESS_DENIED but that is not always the case.
Callers need to be written with consideration that the set of enum
results may change.
- fastCheck(), no parameters, when a full set of "Fast" *_access lines
are to be scanned. The checklist constructor accepts the list to be
scanned. This is the old fastCheck(), with the new ALLOWED / DENIED
/ DUNNO result.
- fastCheck(list), one parameter, when a single-line set of ACLs is to
be scanned. This is the old matchAclListFast(), with the new ALLOWED
/ DENIED / DUNNO result. Will return ALLOWED whenever the whole set
of ACLs matches. Other results may vary.
- nonBlockingCheck() - for "Slow" non-blocking lookups with asynchronous
callback handler. NP: not touched by this patch.
The output change from boolean to allow_t is due to the fastCheck()
callers mixed set of needs allow/deny/other which boolean cannot meet.
Mapping that tri-state need to a boolean result has led to inconsistent
cases of fastCheck() producing unusual values for "true". Sometimes
wrongly for the caller.
Added result lookup type ACCESS_DUNNO, to indicate a test was unable to
be completed BUT there was no allow/deny/auth-required resulting.
Alters all previous calling code to use the new fastCheck() API output.
Some have been polished up to boolean where appropriate instead of
relying on integer values.
Removes matchAclListFast/matchAclListSlow,
Renames check() to matchNonBlocking;
all match*() functions are internal operations during ACL testing.
FQDN had memDataInit() in its component setup, which movedin rev 11499.
The assert is in memCheckInit() and tests that memInit() worked properly.
But if a pool is initialized only when its component is loaded, that
check will fail on several conditions unrelated to the operation of
memory. Seemingly trivial changes to component loading order is one case.
This patch allows modules to initialize/register their own pools on
demand later in the startup process.
* shuffle MEM_DONTFREE which is an existing fixed entry that must not be
memInitCheck()'d to the end of the MemPool type enum list.
* update memCheckInit() to stop scanning for missing pools at that marker.
* shuffle pool types which are initialized by their components after the
marker value. Such that no false problem is reported if (a) the
component is never initialized for that worker, or (b) the component is
only initialized during the configuration process.
* document this layout significance in the enum list to aid future pool
additions or moves.
* add asserts to memAllocate() and memFree() to highlight the cases of
brokenness memCheckInit() was catching. Using assert() instead of if()
so that optimized builds can avoid the penalty of an extra test on each
alloc/free.
Alter the getMyPort() function to skip ports flagged for special mode
handling (intercept, tproxy, accel) when generating internal URLs.
This allows us to lock down security on these special mode ports and
still have an optional position for the forward-proxy port. Prior to
this only the first port was used, forcing an unnecessary configuration
order.
Since it is now possible to have no port available for these URLs the
fatal()/crash has been reduced to an annoying cache.log message. Port 0
will be inserted into the URLs making them invalid.
For now this is only done on http_port entries. https_port has an
incomplete merge of https_port_list/http_port_list which needs to be
completed before it is easily done there.
The ConnOpener::earlyAbort method shopuld defined as comm close handler
which means that it should have as argument CommCloseCbParams not
CommConnectCbParams
Author: Amos Jeffries <squid3@treenet.co.nz>, Christos Tsantilas <chtsanti@users.sourceforge.net>
Add DNS lookup step to ICAP connect
This patch add an async DNS lookup step to the ICAP connection setup process.
As a side effect it adds a little bit of tcp_outgoing_address support to ICAP.
Its limited to "dst" ACL at present. So outgoing ACL selections depending on
HTTP request details wont work. Which makes sense since this connection may
be reused for multiple requests.
More than one IP result are not used since there seems to be no sane place
to store multiple IPs between connect attempts. It currently relies on ipcache
MarkGood/MarkBad keeping a good usable IP at the top of the IP set.
This patch also try to make Max-Connections ICAP feature cooperate well with
the new DNS lookup code
commit revno:11534 fixes:
- Inside IdleConnList::removeAt method the ellements deleted correctly from
the theList_ array. The problem was that if parent_ exist the size of the
array is not decreased and the last element was duplicated (the last two
ellements pointed to the same connection object).
The commit revno:11534 did not solve this problem and just created a
duplicate entry in an other position.
This patch solves the problems in IdleConnList::removeAt method.
Other:
- Remove the fd_table[fd].flags.read_pending tests inside IdleConnList::pop
and IdleConnList::findUsable methods. This flag currently is not fully
implemented and used only by the ssl stuff.
- Inside IdleConnList::closeN method in two positions we are storing the
reference of the Comm::Connection object which will be deleted, to use it
to clean up and close the connection later:
const Comm::ConnectionPointer &conn = theList_[--size_];
theList_[size_] = NULL;
This is wrong because the second command may delete the conn object, causing
assertion in Comm::Connection destructor, because it is still open, or
segmentation faults when trying to use the conn object later.
This patch replaces the pointer reference with a normal pointer.
- Call clearHandlers inside IdleConnList::pop and IdleConnList::findUsable
methods before return the Comm::Connection object to the user.
via http://$visible_hostname/ requests intercepted by the internal
server feature. Also https:// if SSL/TLS is available on the receiving
port.
In order to safely identify the manager reports the path prefix
/squid-internal-mgr/ is added. The old cache_oject:// scheme format
paths follow that identifier prefix.
To retrieve pages the proxy visible_hostname, management port (first
forward-proxy port), and the path prefix must all be present in the URL.
The "manager" ACL is altered to url_regex in order to match the new
protocol+path URL syntax.
Unlike the cache_object:// scheme, http[s]:// do not accept password
as part of the URL. If one is needed it must be sent via the HTTP
Authorization: Basic authentication header.
NP: use of this per-action cachemgr_passwd is not secure and should be
avoided. Stronger security can be gained via http_access with regular
proxy_auth and other ACLs.
- Inside IdleConnList::removeAt method the last element of the
IdleConnList::theList_ array initialized with random memory
- Inside IdleConnList::removeAt method if the IdleConnList::parent_ is NULL
(ICAP connections pools) the size_ of the array is not decreased after
element removed
- Inside IdleConnList::closeN method, it removes always all elements from the
list except the first one
When FwdServer::_peer is set, HttpStateData constructor creates a new special
HttpRequest, overwriting the request pointer set in the parent (ServerStateData)
constructor to fwd->request.
This special HttpRequest sets the proper urlpath (which maybe different from
the original HttpRequest), the host (HttpRequest::SetHost/GetHost) to be the
peer hostname and inherits flags, protocol, method. Also sets the
HttpRequest::flags.proxying.
Probably this is originaly done to handle only the differences in urlpath and
the host. But this is has as result to have two HttpRequests object in
HttpStateData, but their difference is not clear.
This patch removes the HttpStateData::orig_request member and uses only the
HttpStateData::request member
Bugs fixed with this patch:
- Debugs() and error pages sometimes display the cache_peer hostname as the URL requested domain name when going to an origin. Regardless of what the virtual
host name actually is.
- The request_header_access configuration parameter does not work when
sending requests to parent proxies.
- Squid may cache replies to requests with no-store in headers when uses a
parent cache.
- parent caches which have been configured as "sibling" for specific domains
using the neighbor_type_domain parameter are not counted.
Alex Rousskov [Thu, 23 Jun 2011 21:03:57 +0000 (15:03 -0600)]
Removed pointless call to maybeReadVirginBody() for aborted entries.
No runtime changes expected.
Setting flags.do_next_read to zero and then calling maybeReadVirginBody() is
pointless because maybeReadVirginBody() does nothing but wasting cycles when
the flag is zero. Also, even if we do manage to read the virgin response body
somehow, we cannot write it to the aborted store entry anyway.
This patch
- Adds SQUID_X509_V_ERR_DOMAIN_MISMATCH to TheSslErrorArray and in
errors/templates/error-details.txt template file to enable bypass and
details reporting for that error.
- Fixes the ssl error details handling code, to report the first honored error
in a certificate, not the first certification validation error (which could
have been bypassed).
- Adjust ssl_verify_cb() to copy ctx->error to error_no in the beginning and
never use ctx->error after that.
Amos Jeffries [Fri, 17 Jun 2011 16:32:04 +0000 (04:32 +1200)]
Upgrade comm layer Connection handling
The premise underlying this large patch is that instead of copying and
re-copying and re-lookups for the FD related data we can take the
ConnectionDetail class which is generated to store a few bits of IP
information about newly accept()'d connections and make it persist across
the whole of Squid.
It has been renamed from ConnectionDetails to Comm::Connection and has
absorbed a few FD data fields from other classes long the code paths.
Its scope is to hold an FD (or potential FD) plus meta data.
Comm::Connection are valid before, during and after the period when their
particular FD is open. The meta data may be used beforehand to setup the
FD (in the case of peer selection or other TcpAcceptor usage), and it may
remain in use after FD closure until logging or all linked job and call
objects have detected the closure and terminated. A global function
Comm::IsConnOpen() may be used on the pointer objects to detect whether
they point at an active connection.
Most of the patch is simple parameter changes to functions and methods to
pass a "cont Comm::ConnectionPointer &" instead of an "int FD". Along with
class FD fields being converted to these object pointers.
In order to support this alteration there have been behavioral changes to:
The socket accept() Job
Comm::TcpAcceptor altered to spawn Comm::Connection objects and to
operate with one controlling their active/closed state.
FTP data channel handling Calls.
efficiency improvements making use of Comm::Connection as a feedback
channel between TcpAcceptor and FtpStateData to cancel the listening
Job. Most of the underlying logic change is already in trunk to use
the Subscription API. This just streamlines and fixes some race bugs.
Peer selection
updated to spawn a set of Comm::Connection objects. To do this it is
updated to determine *all* peers including DIRECT ones. Doing the DNS
lookup instead of leaving it to comm_connect() on the other side of
FwdState. It also absorbs the outgoing address selection from FwdState
and can now specify details of local+remote ends of an outgoing TCP link.
Forwarding
updated to handle the new outputs from peer selection and to open sequentially.
pconn handling
updated to use destination IP/port and hold a Comm::Connection instead
of domain name indexing an FD. This allows us to maintain idle pools
and re-use FD more efficiently with virtual-hosted servers. Along
with maintaining certainty that the pconn selected actually goes to
the exact destination IP:port needed by forwarding.
comm layer outgoing connections
now have a control job Comm::ConnOpener to do this. Due to the peer
selection and forwarding changes this is a much simpler operation.
HTTP / CONNECT tunnel / gopher / whois / FTP
updated to receive a server and client Comm::Connection object from
forwarding. To operate on those until they close or are finished with.
SNMP / ICP / HTCP / DNS port listeners
updated to work with Comm::Connection holding their listening socket
meta data. This is a side-effect of the ICP and Comm read/write/timeout
changes.
This project adds support for a translatable and customisable error detail file
(errors/templates/error_details.txt). The file is stored like we store error
page templates today. Inside the file, an HTTP-like format used that can be
later extended to other error details (and beyond):
name: value
details: "value"
descr: "value"
or
name: value
details: "multi
line
value"
descr: "value with a \"quoted string\" inside"
The code supports future translations, just like Squid already support error
page translations.
This is a Measurement Factory project
----
Some Technical details:
- The errorpage code which is related to loading and parsing error templates
moved to TemplateFile class. This class is used as base class for
ErrorPageFile class which used to load error page templates.
- The HttpHeader parser used to parse error details
- The error details for various languages cached to memory
- The ErrorDetailsList used to store a list of error details for a
language/locale
- The ErrorDetailsManager is a class used to load and manage multiple error
details list (ErrorDetailsList objects) for many languages. It also
implements a simple cache.
Amos Jeffries [Fri, 17 Jun 2011 02:31:45 +0000 (14:31 +1200)]
Fix squidclient -V option and allow non-HTTP protocols to be tested
The "-" case is for old style HTTP (called 0.9) where there is no version
string. The "-V 0.9" is for testing servers with broken version number
tag "HTTP/0.9". Do not mix these up!
This also adds the ability to send non-HTTP version tags for testing.
ie "-V ICAP/1.0" or "-V ICY/1.0"
Amos Jeffries [Fri, 17 Jun 2011 02:14:01 +0000 (14:14 +1200)]
Upgrade ICAP persistent connection handling
ICAP services use a "service" model of pconn different from the
"TCP destination" model which PconnPool objects are designed for.
This patch alters Adaptation::Icap::ServiceRep to use the simpler
IdleConnList object for pconn storage. IdleConnList stores a
"set of idle connections" more compatible with the ICAP model.
In order to implement ICAP max-connections feature the closeN()
operation is added to IdleConnList.
The result is removal of the complex hash and management operations on
push/pop of the idle conn set. The only expected behaviour change is
more frequent re-use of idle connections on services with multiple IP
addresses. Speed gains are minimal, but positive.
Fixed bypass of SSL certificate validation errors.
The bypass code was calling ACLChecklist::fastCheck() multiple times
if multiple certificate errors were found. That method should not be
called multiple times because it changes the internal ACLChecklist
state, producing wrong answers for repeated calls.
This patch fixes the ACLChecklist::fastCheck() method so it can be called
multiple times. Each fastCheck() call results in an independent access
list check.
author: Alex Rousskov <rousskov@measurement-factory.com>, Christos Tsantilas <christos@chtsanti.net>
Bug 3153 fix: Prevent ICAP RESPMOD transactions getting stuck with the adapted body.
Part 1.
Server is expected to receive adapted response headers and then consume the
adapted response body, if any. If the server receives the headers and then
aborts, it must notify the ICAP side that nobody will consume the body.
Otherwise, the ICAP transaction will fill the BodyPipe buffer and get stuck
waiting for the consumer to free some space.
Part 2:
This fix still leaves one potential race condition unhandled: The ICAP
Initiatee disappears right after sending the adapted headers to the Server
(because there is nothing else for that initiatee to do). After the
noteAdaptationAnswer() call is scheduled by ICAP and before it is received by
the Server job, there is no usable link between Server and ICAP. There is no
way for the Server to notify the ICAP transaction that the Server job is
aborting during that time (and there is no Server job at all after it aborts,
naturally).
The solutions is to develop a custom AsyncCall which will call the
expectNoConsumption() on the message pipe if the call cannot be dialed (i.e.,
the message cannot be delivered to Server).
Amos Jeffries [Mon, 13 Jun 2011 12:25:12 +0000 (06:25 -0600)]
Auth lookup state cbdata upgrade
The authenticators utilize a "statedata" structure to store and pass
the callback and Auth::UserRequest an auth lookup is about.
This patch converts the structure from a CBDATA_GLOBAL_TYPE struct to a
CBDATA_CLASS2 and adds a parameterized constructor for it.
The result is that all the code using it no longer has to explicitly
manage fields assignments and cbdata referencing. Simply new the object
when submitting to the helper system and delete once its handler has
been called.