Mrcus Kool [Tue, 9 Aug 2011 07:09:03 +0000 (01:09 -0600)]
Optimize regular expression ACLs
This patch is inspired by the work that I did for ufdbGuard and a few emails with Amos.
The new code optimises lists of regular expressions.
The optimisations are:
* initial .* is stripped
* RE-1 RE-2 ... RE-n are joined into one large RE: (RE-1)|(RE-2)|...|(RE-n)
* -i ... -i options are optimised: the second one is ignored, same for +i
If compounding optimization fails it falls back to using unoptimized
expressions.
Amos Jeffries [Mon, 8 Aug 2011 00:21:01 +0000 (12:21 +1200)]
Remove hierarchy_stoplist default value
This should have been done long ago with the other dynamic website
handling changes. It has caused a certain amount of confusion when things
which apparently should go to peers fail to reach them.
author: Christos Tsantilas <chtsanti@users.sourceforge.net>, Amos Jeffries <squid3@treenet.co.nz>
Bug fix: The Ip::Address::IsAnyAddr method return false for IPv4 anyaddr.
- The ip::Address::IsAnyAddr() returns true only for ipv6 anyaddr
(0000:0000:0000:0000:0000:0000:0000:0000) and returns false when we have an
ipv4 anyaddr (0000:0000:0000:0000:0000:FFFF:0000:0000)
- The ip::Address::IsIPv4 method returns false in the case of IPv4 anyaddr.
The above can cause bugs, eg:
- inside Ip::Address::SetIPv4(). When it is called for an IPv6 anyaddr the ip
address will not considred as anyaddr any morei (it IsAnyAddr will return
false).
- inside cache_cf.cc file inside dump_generic_http_port function:
if (s->s.IsAnyAddr() && !s->s.IsIPv6())
storeAppendPrintf(e, " ipv4");
The if condition in the above statement can never be true. But the s->s can
be an ipv4 anyaddr.
- other places where the code will not work as expected in the case we are
listening to an ipv4 anyaddr ip address.
This patch:
- moving the IsIPv4/6 to base purely on the v4-mapped or not
- making both protocols ANYADDR match the same test
- making both protocols NOADDR match the same test
- Fixing the IsIPv4/6 documentation to match the implementation
Amos Jeffries [Thu, 4 Aug 2011 03:21:06 +0000 (21:21 -0600)]
SourceLayout: format namespace for custom tag-based formats
Part 1 of enabling non-logging components to support custom formats in strings
Shuffle the log custom format code into its own library separate from the
logging functionality.
One minor logic change removing redundant LogFileEnabled flag.
TODO:
- use MemBuf instead or as well as StoreEntry as the output buffer
- separate from AccessLogEntry confusion
- upgrade deny_info URL generation format
- upgrade external_acl_type format
- add custom helper formats
Amos Jeffries [Wed, 3 Aug 2011 12:35:41 +0000 (06:35 -0600)]
Bug 3243: CVE-2009-0801 Bypass of browser same-origin access control in intercepted communication
Add a verify step between header parsing and http_access to validate that the
Host: header matches the URL for forward-proxied traffic or the destination
IP:port for intercepted traffic.
This is part 1 of the CVE protections. The validation step required to detect
forgery and protect against cache poisoning.
author: Measurement Factory
Bug 3118: ecap_enable on forces icap_enable on
We were updating [Icap|Ecap]::TheConfig even when [icap|ecap]_enable was false,
which may lead to service activation for Icap or Ecap services that should be
disabled. The patch removes such services from service groups before they are
activated.
The patch also warns the user when an adaptation group loses some but not all
of its services due to the new group cleanup code.
- The "Sender Host Address" field of the ICP messages header it is a 32bit
integer so it can be only an ipv4 ip address. Moreover according the ICP RFC:
"Sender Host Address
The IPv4 address of the host sending the ICP message. This field
should probably not be trusted over what is provided by getpeer-
name(), accept(), and recvfrom(). There is some ambiguity over
the original purpose of this field. In practice it is not used."
This patch set the "Sender Host Address" field always to 0.
- Remove the echo_hdr static variable from neighbors.cc file and the
theIcpPublicHostID variables from the icp_v2.cc file. They are part of the
old "source_ping" squid feature code which does not exist any more.
- Remove the theIcpPrivateHostID variable from the icp_v2.cc file. It was used
only to set the "Sender Host Address" icp message header field.
Display HTTP protocol syntax at section 11 level 2
This enables easy debugging of what HTTP requests and replies are flowing
over the between Squid and external clients/servers. Avoiding the need
for level-9 debug traces or packet-level deciphering.
Default to vhost for accelerator mode (reverse proxy)
Defaults to match HTTP requirements, and Host awareness is a
rather strong HTTP/1.1 requirement. The default in HTTP/1.1 is to read
the Host header (unless URL is absolute) but a server MAY optionally
ignore the Host header if desired.
The option no-vhost is provided to disable this behaviour if necessary.
Regression fix: vhost and defaultsite causing vport to be ignored
Instead of dropping it completely we should be sanely combining them like
Squid-2 does for most cases. This appears to have been lost while removing
the getmyHostname() from the process and reducing the
prepareTransparentUrl code.
This fix makes vport apply even if vhost was used. It will modify the
Host: header contents according to the documented vport semantics.
This fix makes vport apply even if defaultsite= was used. It will append
the specified port to the domain name given. Domains with port attached
are not supported and will produce invalid URLs.
TODO: detect this case while parsing the initial config and warn.
Enable negative cacheing on unknown or -1 expiry timestamp
This syncs the squid-3 code with what squid-2 does. THere seem to be no
problems in squid-2, but squid-3 does not cache at times when it should
according to negative_ttl
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.
projects / thirdparty / squid.git / log
