Amos Jeffries [Wed, 26 Aug 2015 17:51:18 +0000 (10:51 -0700)]
Bug 3553: cache_swap_high ignored and maxCapacity used instead
Also, to make matters worse the amount of objects (max 70) being purged on
each of the 1-second maintenance loops was far too small for the traffic
speeds of up to 20k RPS now being processed by proxies.
This fixes the cache_swap_high behaviour to closer match what is documented
at present, although some documentatino does say it cleans all the way down
to the low-water mark. Which appears never to have been true in regards to
one cycle but would occur over several of the proxy speed was not too high.
With this updated algorithm there is almost no limit to how far the
aggressiveness can scale, but it is linear at 200 objects per multiple of the
gap between low- and high- watermark.
SwapDir::maintain is now fairly well documented and debug traces added. With
several TODO ideas for future improvement also documented in the method code.
Alex Rousskov [Mon, 24 Aug 2015 21:07:31 +0000 (15:07 -0600)]
When a RESPMOD service aborts, mark the body it produced as truncated.
Without these changes, the recipient of the truncated body often
cannot tell that the body was actually truncated (e.g., when Squid
uses chunked encoding for body delivery). Lying about truncation
may result in rather serious user-level problems.
Implement constructors for wordlist allowing them to support non-zeroing pools
Make destructor private in order to force clients to use wordlistDestroy
Implement wordlistChopHead to support the only user of deleting the head of a wordlist
Amos Jeffries [Sun, 23 Aug 2015 11:53:54 +0000 (04:53 -0700)]
Docs: auto-build release notes for snapshots
This adds conditional build support to generate release notes whenever
a tarball is being created, regardless of what the code branch status
is. All that is required is the linuxdoc tool chain.
Formal release branch snapshots have been publishing the notes files
built for their previous release. But development versions of Squid
have not been getting documented at all which can be annoying for
testers.
The release-N.html file is also removed from the repository. With this
update it should no longer be needed by the snapshot machinery.
Amos Jeffries [Sat, 22 Aug 2015 19:06:46 +0000 (12:06 -0700)]
TLS: failure of https:// context non-fatal for non-OpenSSL builds
Only OpenSSL library is 'guaranteed' to produce a TLS context at this point
in the conversion to library-agnostic security. Any others may produce
nothing.
Match the DBG_IMPORTANT used for debug level of the 'initializing' message.
Amos Jeffries [Fri, 21 Aug 2015 09:43:53 +0000 (02:43 -0700)]
Cleanup: fix assertion in Store unit tests
The old Squid String implementation cannot handle appending nullptr or
negative lengths. So if the test code using CapturingStoreEntry ever
tries to append such it will crash instead of working like a StoreEntry
should.
Revert override keyword in Ftp::Server::callException
If even one only method is marked override in a class, then clang
requires all overriding methods in the class to be marked as such.
This uncovers a problem where toCbdata is defined virtual in
AsyncJob (which Ftp::Server inherits from) and implemented nonvirtual
in the CBDATA_CLASS macro. Fixing this will be the result of a sepearate
effort, for now covering up by removing keyword and marking XXX.
Handle nil HttpReply pointer inside various handlers called from
Ftp::Server::handleReply(). For example, when the related StoreEntry
object is aborted, the client_side_reply.cc code may call the
Ftp::Server::handleReply() method with a nil reply pointer.
The Ftp::Server::handleReply() methods itself cannot handle nil replies
because they are valid in many states. Only state-specific handlers know
whether they need the reply.
The Ftp::Server::handleReply() method is called [via Store] from Client code.
Thus, exceptions in handleReply() are handled by the Ftp::Client job. That job
does not have enough information to know whether the client-to-Squid connection
should be closed; the job keeps the connection open. When the reply is nil,
that open connection becomes unusable, leading to more problems.
This patch fixes the Ftp::Server::handleReply() to handle exceptions,
including closing the connections in the case of an exception. It also
adds Must(reply) checks to check for nil HttpReply pointers where the
reply is required. Eventually, Store should start using async calls to
protect jobs waiting for Store updates. Meanwhile, this should help.
Also, some squid.conf.documented updates for new features:
* Update icap_service TLS options to replace "ssl" prefix with "tls-"
on newly added options.
* Remove icap_service 'sslcapath=' option from public display.
It is still supported, but not very portable outside OpenSSL so
do not encourage use at this time.
Ignore impossible SSL bumping actions, as intended and documented.
According to Squid wiki: "Some actions are not possible during
certain processing steps. During a given processing step, Squid
ignores ssl_bump lines with impossible actions". The distributed
squid.conf.documented has similar text.
Current Squid violates the above rule. Squid considers all actions,
and if an impossible action matches first, Squid guesses what the
true configuration intent was. Squid may guess wrong. For example,
depending on the transaction, Squid may guess that a matching
stare or peek action during bumping step3 means "bump", breaking
peeked connections that cannot be bumped.
This unintended but gross configuration semantics violation remained
invisible until bug 4237, probably because most configurations in
most environments either worked around the problem (where admins
experimented to "make it work") or did not result in visible
errors (where Squid guesses did not lead to terminated connections).
While configuration workarounds are possible, the current
implementation is very wrong and leads to overly complex and, hence,
often wrong configurations. It is also nearly impossible to document
accurately because the guessing logic depends on too many factors.
To fix this, we add an action filtering/banning mechanism to Squid
ACL code. This mechanism is then used to:
- ban client-first and server-first on bumping steps 2 and 3.
- ban peek and stare actions on bumping step 3.
- ban splice on step3 if stare is selected on step2 and
Squid cannot splice the SSL connection any more.
- ban bump on step3 if peek is selected on step2 and
Squid cannot bump the connection any more.
The same action filtering mechanism may be useful for other
ACL-driven directives with state-dependent custom actions.
This change adds a runtime performance overhead of a single virtual
method call to all ORed ACLs that do not use banned actions.
That method itself just returns false unless the ACL represents
a whole directive rule. In the latter case, an std::vector size()
is also checked. It is possible to avoid this overhead by adding
a boolean "I may ban actions" flag to Acl::OrNode, but we decided
the small performance harm is not worth the extra code to set
that flag.
Amos Jeffries [Tue, 11 Aug 2015 06:15:34 +0000 (23:15 -0700)]
Polish: add debug section,level to cache.log
Cache.log produced at level ALL,9 are very verbose, and tracking down
what specific section,level details to log for a shorter trace without
lost details can sometimes be tricky and time consuming. Particularly
when multiple sections are involved.
This patch adds a column containing the relevant debug_options
SECTION,LEVEL value on each line right after the kidN number for debug
levels 2+.
Amos Jeffries [Tue, 11 Aug 2015 04:41:55 +0000 (21:41 -0700)]
TLS: fix various bugs in HTTPS proxying context creation
cache_peer with "ssl" option and DIRECT HTTPS outgoing traffic was
not having TLS context initialized at all. Resulting in TLS outgoing
being disabled unless explicit extra options were used.
With this patch:
The default squid.conf sets "tls_outgoing_options min-version=1.0".
Which auto-enables DIRECT outgoing, the new explicit "disable" option
is required to turn off.
http_port ... protocol=HTTPS and https_port forces
"encryptTransport=true;" explicitly based on the expected protocol. So
it is either enabled by the parse() call when TLS options are used, or
forced on anyway later when the protocol is validated.
icaps:// services also explicitly set "encryptTransport=true;"
explicitly based on 's' in the service URI scheme.
The cache_peer requires a minimum of "ssl" option to be configured. Any
use of TLS/SSL options other than "disable" will enable TLS to the peer.
In summary TLS should be:
* default-on for all https_port, icaps:// services, and outgoing
https:// traffic.
Alex Rousskov [Mon, 10 Aug 2015 21:23:12 +0000 (15:23 -0600)]
Reject non-chunked HTTP messages with conflicting Content-Length values.
Squid used to trust and forward the largest Content-Length header. This
behavior violated an RFC 7230 MUST in Section 3.3.3 item #4. It also confused
some ICAP services and probably some HTTP agents. Squid now refuses to forward
the badly framed message to the ICAP service and HTTP agent, responding with
an HTTP 411 or 502 (depending on the message direction) error instead.
This is a quick-and-dirty implementation. A polished version should reject
responses with invalid Content-Length values as well (per RFC 7230 MUST) and
should behave the same regardless of the relaxed_header_parser setting (this
is not a header parsing issue).
Rename http_hdr_cc_type to HttpHdrCcType and reference it by full qualifier.
Remove module cleanup functions for HttpHeader, HttpHdrCc, HttpHdrSc.
Remove useless includes.
Rename Http::any_registered_header to Http::any_HdrType_enum_value.
Remove useless assert()s in HttpHeaderEntry dtor and HttpHeader::parse.
Clarify documentation for Http::HeaderLookupTable
Amos Jeffries [Tue, 4 Aug 2015 02:40:16 +0000 (19:40 -0700)]
Boilerplate: add Foundation details to rfcnb and smblib documentation files
We had hoped to be removing this old library code by now. But it appears
that there is no alternative and users are still requesting the helpers
that depend on them.
Amos Jeffries [Mon, 3 Aug 2015 09:15:27 +0000 (02:15 -0700)]
Cleanup: de-duplicate fake-CONNECT code
Over the course of the peek-n-splice development and followup patches
the code generating fake CONNECT requests to tunnel various intercepted
traffic has been copy-n-pasted several times.
Add a new method fakeAConnectRequest() that takes a debug reason and
SBuf containing any payload to preserve from the original I/O buffer.
Amos Jeffries [Mon, 3 Aug 2015 03:50:25 +0000 (20:50 -0700)]
Use automake subdir-objects feature
Now that there are no longer cross-directory collisions in the built
binaries or libraries we can enable this feature from ./configure
instead of on a per-Makefile basis
Amos Jeffries [Mon, 3 Aug 2015 02:08:22 +0000 (19:08 -0700)]
Place unit tests in src/tests to make automake happy
The auto* toolchain warns that automake future versions
will be enablign aubdir-objects mechanism by default.
Some unit tests were moved into per-library subdirs
with the plan of keeping all convenience library code
together. However the current layout state of Squid
means that most still require some objects in other
libraries or at the top level. This does not build
happily with the auto-tools subdir-objects feature.
In particular the distclean target has a tendency
to erase objects twice and die on the second attempt.
Temporarily undo that SourceLayout shuffing in order
to be more compatible with automake 1.1n versions.