]> git.ipfire.org Git - thirdparty/squid.git/log
thirdparty/squid.git
7 years agoAdd missing stub definition after rev.14696
Amos Jeffries [Mon, 6 Jun 2016 11:02:59 +0000 (23:02 +1200)] 
Add missing stub definition after rev.14696

7 years agoDocs: fix typo in rev.14521
Amos Jeffries [Mon, 6 Jun 2016 03:28:37 +0000 (15:28 +1200)] 
Docs: fix typo in rev.14521

8 years agoCryptoNG: cleanup TLS/SSL context initialization sequence
Amos Jeffries [Thu, 2 Jun 2016 09:52:43 +0000 (21:52 +1200)] 
CryptoNG: cleanup TLS/SSL context initialization sequence

The libsecurity ServerOptions and PeerOptions class methods are now
supposed to be the API for creating SSL contexts for https_port,
cache_peer and outgoing connections.

Continue the API transition by making the callers of sslCreate*Context()
functions use the libsecurity API instead and repurpose the now obsolete
functions into the Ssl:: namespace to initialize the keying material and
other not-yet-converted OpenSSL state details of an existing context.

A side effect of this is that GnuTLS contexts are now actually created
and initialized as far as they can be.

SSL-Bump context initialization is not altered by this.

8 years agoMark refresh-waiting transactions with REFRESH.
Eduard Bagdasaryan [Thu, 2 Jun 2016 09:49:19 +0000 (21:49 +1200)] 
Mark refresh-waiting transactions with REFRESH.

Before this change, transactions initiating a refresh were still marked
as TCP_HIT*. If such a transaction was terminated (for any reason)
before receiving an IMS reply, it was logged with that misleading tag.
Now, such transactions are logged using TCP_REFRESH[_ABORTED].

After the refresh (successful or otherwise), the tag changes to one of
the other TCP_REFRESH_* values, as before.

8 years agoFix shell substitution typo in rev.14693
Amos Jeffries [Wed, 1 Jun 2016 11:09:57 +0000 (23:09 +1200)] 
Fix shell substitution typo in rev.14693

8 years agoCleanup: improve automake coding guideline adherence
Amos Jeffries [Tue, 31 May 2016 22:47:03 +0000 (10:47 +1200)] 
Cleanup: improve automake coding guideline adherence

Several features were not following guidelines on ENABLE_* automake
variable naming.

As a result fix ESI unit tests which were not being run and simplify
adaptation conditional creation.

And remove unused HAVE_SPEGNO conditional.

8 years agoDeprecating SMB LanMan helpers
Amos Jeffries [Mon, 30 May 2016 01:55:32 +0000 (13:55 +1200)] 
Deprecating SMB LanMan helpers

Bring the SMB LanMan helpers one step closer to removal by dropping them
from the set of helpers which are auto-detected and built by default
with Squid.

They are still available for the minority using them. But need to be
explicitly listed in the ./configure options to be built.

8 years agoRemove unused var after r14689
Christos Tsantilas [Sat, 28 May 2016 07:46:14 +0000 (10:46 +0300)] 
Remove unused var after r14689

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 27 May 2016 18:12:12 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoRemove ie_refresh configuration option
Amos Jeffries [Fri, 27 May 2016 12:58:15 +0000 (00:58 +1200)] 
Remove ie_refresh configuration option

This option was provided as a hack to workaround problems in MSIE 5.01
and older.

Since those MSIE versions are long deprecated and no longer even
registering on the popularity charts for more than 5 years I think its
time we removed this hack.

8 years agoReplace new/delete operators using modern C++ rules.
Alex Rousskov [Fri, 27 May 2016 12:46:02 +0000 (00:46 +1200)] 
Replace new/delete operators using modern C++ rules.

This change was motivated by "Mismatched free()/delete/delete[]" errors
reported by valgrind and mused about in Squid source code.

I speculate that the old new/delete replacement code was the result of
slow accumulation of working hacks to accomodate various environments,
as compiler support for the feature evolved. The cumulative result does
not actually work well (see the above paragraph), and the replacement
functions had the following visible coding problems according to [1,2]:

a) Declared with non-standard profiles that included throw specifiers.
b) Declared inline. C++ says that the results of inline declarations
   have unspecified effects. In Squid, they probably necessitated
   complex compiler-specific "extern inline" workarounds.
c) Defined in the header file. C++ says that defining replacements "in
   any source file" is enough and that multiple replacements per
   program (which is what a header file definition produces) result in
   "undefined behavior".
d) Declared inconsistently (only 2 out of 4 flavors). Declaring one base
   flavor should be sufficient, but if we declare more, we should
   declare all of them.

[1] http://en.cppreference.com/w/cpp/memory/new/operator_new
[2] http://en.cppreference.com/w/cpp/memory/new/operator_delete

The replacements were not provided to clang (trunk r13219), but there
was no explanation why. This patch does not change that exclusion.

I have no idea whether any of the old hacks are still necessary in some
cases. However, I suspect that either we do not care much if the
replacements are not enabled on some poorly supported platforms OR we
can disable them (or make them work) using much simpler hacks for the
platforms we do care about.

8 years agoBug 4517 error: comparison between signed and unsigned integer
Alex Rousskov [Mon, 23 May 2016 23:20:27 +0000 (17:20 -0600)] 
Bug 4517 error: comparison between signed and unsigned integer

The old cast is required when size_t is unsigned (as it should be).
The new cast is required when size_t is signed (as it may be).

We could cast just the left-hand side to be signed instead, but it feels
slightly wrong to do that here because all values we are casting are
meant to be unsigned and, hence, in theory, might overflow in some
future version of the code if we cast them to a signed value now and
forget to fix that cast later while adding support for larger values.

8 years agoBug 4485: off-by-one out-of-bounds Parser::Tokenizer::int64() read errors
Eduard Bagdasaryan [Mon, 23 May 2016 12:35:06 +0000 (00:35 +1200)] 
Bug 4485: off-by-one out-of-bounds Parser::Tokenizer::int64() read errors

8 years agoFixed build-breaking typo in r14684.
Alex Rousskov [Sat, 21 May 2016 23:19:36 +0000 (17:19 -0600)] 
Fixed build-breaking typo in r14684.

8 years agoGnuTLS: Simplify trusted CA loading
Amos Jeffries [Sat, 21 May 2016 21:30:25 +0000 (09:30 +1200)] 
GnuTLS: Simplify trusted CA loading

8 years agoDo not override user defined -std option
Amos Jeffries [Sat, 21 May 2016 12:12:51 +0000 (00:12 +1200)] 
Do not override user defined -std option

8 years agoFixed icons loading speed.
Alex Rousskov [Fri, 20 May 2016 18:16:19 +0000 (12:16 -0600)] 
Fixed icons loading speed.

Since trunk r14100 (Bug 3875: bad mimeLoadIconFile error handling), each
icon was read from disk and written to Store one character at a time. I
did not measure startup delays in production, but in debugging runs,
fixing this bug sped up icons loading from 1 minute to 4 seconds.

8 years agoNever enable OPENSSL_HELLO_OVERWRITE_HACK automatically.
Alex Rousskov [Fri, 20 May 2016 17:19:44 +0000 (11:19 -0600)] 
Never enable OPENSSL_HELLO_OVERWRITE_HACK automatically.

OPENSSL_HELLO_OVERWRITE_HACK, a.k.a adjustSSL(), a.k.a. "splice after
stare and bump after peek" hack requires updating internal/private
OpenSSL structures. The hack also relies on SSL client making SSL
negotiation decisions that are similar to our OpenSSL version decisions.
Squid used to enable this hack if it could compile the sources, but:

* The hack works well in fewer and fewer cases.
* Making its behavior reliable is virtually impossible.
* Maintaining this hack is increasingly difficult, especially after
  OpenSSL has changed its internal structures in v1.1.
* The combination of other bugs (fixed in r14670) and TLS extensions in
  popular browsers effectively disabled this hack for a while, and
  nobody (that we know of) noticed.

This temporary change disables the hack even if it can be compiled. If
an admin is willing to take the risks, they may enable it manually by
setting SQUID_USE_OPENSSL_HELLO_OVERWRITE_HACK macro value to 1 during
the build.

If, after this experimental change, we get no complaints (that we can
address), the hack will be completely removed from Squid sources.

8 years agoCheck for SSL_CIPHER_get_id() support required in adjustSSL().
Alex Rousskov [Fri, 20 May 2016 16:49:07 +0000 (10:49 -0600)] 
Check for SSL_CIPHER_get_id() support required in adjustSSL().

Our adjustSSL() hack requires SSL_CIPHER_get_id() since trunk r14670,
but that OpenSSL function is not available in some environments, leading
to compilation failures.

8 years agoFix OpenSSL detection on FreeBSD
Amos Jeffries [Fri, 20 May 2016 14:57:48 +0000 (02:57 +1200)] 
Fix OpenSSL detection on FreeBSD

8 years agoDo not allow low-level debugging to hide important/critical messages.
Alex Rousskov [Fri, 20 May 2016 13:20:27 +0000 (01:20 +1200)] 
Do not allow low-level debugging to hide important/critical messages.

Removed debugs() side effects that inadvertently resulted in some
important/critical messages logged at the wrong debugging level and,
hence, becoming invisible to the admin. The removed side effects set the
"current" debugging level when a debugs() parameter called a function
that also called debugs(). The last nested debugs() called affected the
level of all debugs() still in progress!

Related changes:

* Reentrant debugging messages no longer clobber parent messages. Each
  debugging message is logged separately, in the natural order of
  debugs() calls that would have happened if debugs() were a function
  (that gets already evaluated arguments) and not a macro (that
  evaluates its arguments in the middle of the call). This order is
  "natural" because good macros work like functions from the caller
  point of view.

* Assertions hit while evaluating debugs() parameters are now logged
  instead of being lost with the being-built debugs() log line.

* 10-20% faster debugs() performance because we no longer allocate a new
  std::ostringstream buffer for the vast majority of debugs() calls.
  Only reentrant calls get a new buffer.

* Removed old_debug(), addressing an old "needs to die" to-do.

* Removed do_debug() that changed debugging level while testing whether
  debugging is needed. Use side-effect-free Debug::Enabled() instead.

Also removed the OutStream wrapper class. The wrapper was added in trunk
revision 13767 that promised to (but did not?) MemPool the debug output
buffers. We no longer "new" the buffer stream so a custom new() method
would be unused. Besides, the r13767 explanation implied that providing
a Child::new() method would somehow overwrite Parent::allocator_type,
which did not compute for me. Finally, Squid "new"s other allocator-
enabled STL objects without overriding their new methods so either the
same problem is still there or it did not exist (or was different?).

Also removed Debug::xassert() because the debugs() assertions now work
OK without that hack.

8 years agoHTTP/1.1: unfold mime header blocks
Amos Jeffries [Fri, 20 May 2016 08:28:33 +0000 (20:28 +1200)] 
HTTP/1.1: unfold mime header blocks

Enact the RFC 7230 section 3 requirement that proxies remove obs-fold from
received mime header blocks and drop all lines that start with whitespace
immediately following the request-line.

Also;

* Shuffle the DelimiterCharacters() and RelaxedDelimiters() helper methods
  to Http::One::Parser for use in processing mime.

* Extend the headersEnd() function with a wrapper to avoid SBuf::c_str()
  and to efficiently report whether obs-fold exists without needing to
  re-scan the block.

* Document the mime_header.h API function(s).

* Add Http::One::CrLf() to present a SBuf CRLF constant. This can be used
  instead of the many local string definitions we have now.
  The implementation could perhapse be done better for performance, but not
  done here.

8 years agosquidclient: Improve shell-escape support in -H option
Amos Jeffries [Fri, 20 May 2016 05:25:58 +0000 (17:25 +1200)] 
squidclient: Improve shell-escape support in -H option

The -H parameter takes a string with some limited shell-escaped
characters. Currently just \n was expanded to the CRLF sequence.
Other shell escaped characters were left untouched.

However, to properly test headers containing weird CR, LF and HTAB
positioning it needs to be able to receive these special characters
individually and thus unescape them.

Add a new function similar to perform shell unescape with special
characters \\, \r, \n, and \t. All other characters are un-escaped
to themselves and a warning printed at verbosity level 1.

8 years agoSpellchecked SPONSORS.list, which was broken since at least 2012!
Alex Rousskov [Thu, 19 May 2016 23:44:12 +0000 (17:44 -0600)] 
Spellchecked SPONSORS.list, which was broken since at least 2012!

8 years agoAcknowledged RM contribution.
Alex Rousskov [Thu, 19 May 2016 23:33:33 +0000 (17:33 -0600)] 
Acknowledged RM contribution.

8 years agoFix typo in rev.14670
Amos Jeffries [Thu, 19 May 2016 13:14:46 +0000 (01:14 +1200)] 
Fix typo in rev.14670

8 years agoDelete cbdata-protected data when built --with-valgrind-debug.
Alex Rousskov [Wed, 18 May 2016 22:46:52 +0000 (16:46 -0600)] 
Delete cbdata-protected data when built --with-valgrind-debug.

Valgrind was correctly reporting every cbdata allocation as leaking!
AFAICT, these regressions were introduced by a combination of trunk
r13977 (Bug 4215: double-free in CBDATA) and trunk r13909 (de-duplicate
cbdata deallocate actions).

Also fixed and polished cbdata debugging that was printing mismatching
Allocating/Freeing pointer values and synced scripts/find-alive.pl.

8 years agoSourceFormat Enforcement
Source Maintenance [Wed, 18 May 2016 18:12:17 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoFast SNI peek
Christos Tsantilas [Wed, 18 May 2016 17:22:44 +0000 (20:22 +0300)] 
Fast SNI peek

Currently, bumping peek mode at step2 and splice at step2, after the SNI is
received is slow.
The most of the performance overhead comes from openSSL. However Squid does not
need openSSL to peek at SNI. It needs only to get client TLS Hello message
analyze it to retrieve SNI and then splice at step2.

This patch:
  - Postpone creation of the OpenSSL connection (i.e. SSL) object for the
    accepted TCP connection until after we peek at SNI (after step2).
  - Implements the Parser::BinaryTokenizer parser for extracting byte-oriented
    fields from raw input
  - Reimplement a new SSL/TLS handshake messages parser using the
    BinaryTokenizer, and remove old buggy parsing code from ssl/bio.cc
  - Adjust ConnStateData, Ssl::Bio, Ssl::PeerConnector classes to use the
    new parsers and parsing results.

This is a Measurement Factory project

8 years agosync with trunk-r14669
Christos Tsantilas [Wed, 18 May 2016 16:35:36 +0000 (19:35 +0300)] 
sync with trunk-r14669

8 years agoPolishing fixes
Christos Tsantilas [Wed, 18 May 2016 16:26:16 +0000 (19:26 +0300)] 
Polishing fixes

Code formatting, variables fixing, comments and debug messages

Most of them proposed by Amos on squid-dev review procedure.

8 years agoIncrease debug level in a peek-and-splice related debug message
Christos Tsantilas [Wed, 18 May 2016 15:26:27 +0000 (18:26 +0300)] 
Increase debug level in a peek-and-splice related debug message

It may produced one debugging line for each SSL transaction in some cases

8 years agoHttpHeaderEntry leaks and HttpHeader::len corruption by delById().
Alex Rousskov [Wed, 18 May 2016 05:21:28 +0000 (23:21 -0600)] 
HttpHeaderEntry leaks and HttpHeader::len corruption by delById().

AFAICT, this regression was introduced in trunk r14285 (Refactor
HttpHeader into gperf-generated perfect hash) and became severe after
trunk r14659 started calling delById() on virtually every request.

8 years agomerge from trunk-r14667
Christos Tsantilas [Fri, 13 May 2016 11:49:15 +0000 (14:49 +0300)] 
merge from trunk-r14667

8 years agorun formatter
Christos Tsantilas [Fri, 13 May 2016 10:27:54 +0000 (13:27 +0300)] 
run formatter

8 years agoBug 4492: chunked parser needs to accept BWS after chunk size
Dan Searle [Thu, 12 May 2016 12:37:50 +0000 (00:37 +1200)] 
Bug 4492: chunked parser needs to accept BWS after chunk size

8 years agoAllow chunking the last HTTP response on a connection.
Eduard Bagdasaryan [Tue, 10 May 2016 23:06:48 +0000 (17:06 -0600)] 
Allow chunking the last HTTP response on a connection.

Squid should avoid signaling the message end by connection closure
because it hurts message integrity and sometimes performance. Squid
now chunks if:

  1. the response has a body;
  2. the client claims HTTP/1.1 support; and
  3. Squid is not going to send a Content-Length header.

AFAICT, Squid used to exclude to-be-closed connections from chunking
because chunking support was added (trunk r10781) specifically to
optimize persistent connection reuse and closing connections were
incorrectly excluded as a non-interesting/out-of-scope case. And/or
perhaps we did not realize the dangers of signaling the message end
by connection closure.

8 years agoFix comment about v23 ciphers
Christos Tsantilas [Tue, 10 May 2016 08:30:52 +0000 (11:30 +0300)] 
Fix comment about v23 ciphers

Also add the RFC links

8 years agoPolishing fixes
Christos Tsantilas [Mon, 9 May 2016 17:19:42 +0000 (20:19 +0300)] 
Polishing fixes

 - Replace Handshake::details pointer with an always-available object
 - Replace Security::ProtocolVersion and its "int" representation in TlsDetails
   and NegotiationHistory classes with the existing Anyp::ProtocolVersion
 - Fix TlsDetails::compressMethod. The clients may send a compression methods
   list with a NULL compression method.
   Rename to TlsDetails::compressionSupported.
 - Other minor fixes.

8 years ago4.0.10 SQUID_4_0_10
Amos Jeffries [Fri, 6 May 2016 11:33:57 +0000 (23:33 +1200)] 
4.0.10

8 years agoPrep for 4.0.10 and 3.5.18
Amos Jeffries [Fri, 6 May 2016 10:06:01 +0000 (22:06 +1200)] 
Prep for 4.0.10 and 3.5.18

8 years agoRemoved stale comment about 32KB limit on shared memory cache entries.
Alex Rousskov [Thu, 5 May 2016 06:31:11 +0000 (00:31 -0600)] 
Removed stale comment about 32KB limit on shared memory cache entries.

8 years agoFix SIGSEGV in ESIContext response handling
Amos Jeffries [Wed, 4 May 2016 03:31:48 +0000 (15:31 +1200)] 
Fix SIGSEGV in ESIContext response handling

HttpReply pointer was being unlocked without heving been locked.
Resulting in a double-free. Make it use RefCount instead of
manual locking to ensure locked/unlock is always symmetrical.

8 years agoFixed header guard style.
Alex Rousskov [Tue, 3 May 2016 17:33:54 +0000 (11:33 -0600)] 
Fixed header guard style.

8 years agoFixed lack of heartbeat detection.
Alex Rousskov [Tue, 3 May 2016 17:21:39 +0000 (11:21 -0600)] 
Fixed lack of heartbeat detection.

8 years agoOptimization: Spend less CPU and RAM on adjustSSL(). Speed gain: ~5%.
Alex Rousskov [Tue, 3 May 2016 17:18:39 +0000 (11:18 -0600)] 
Optimization: Spend less CPU and RAM on adjustSSL(). Speed gain: ~5%.

Do not store extension types just to iterate over them in adjustSSL().
Check for extension support while parsing instead. Since the list of
OpenSSL-supported extensions is constant (does not depend on the
connection), we do not need to create and index extension storage once
for each TLS connection; we now do it once per worker lifetime instead.

Use std::unordered_set instead of std::list for ciphers. Most real-world
cipher lists probably contain dozens of 2-byte entries, making std::list
storage a poor choice. Unlike TLS extensions, supported ciphers depend
on the connection so we have to store all of them to check whether each
stored cipher is supported for the SSL connection object created later.
Having an O(1) lookup speeds up that last check a lot compared to the
old linear search across all stored ciphers.

Do fast adjustSSL() checks before the longer cipher loop check.

Acknowledge TLS_EMPTY_RENEGOTIATION_INFO_SCSV pseudo cipher support.

Added TLSEXT_TYPE_signature_algorithms(13) and
TLSEXT_TYPE_next_proto_neg(13172) to the list of TLS extensions
supported by OpenSSL and recognized by Squid. Recognizing these
extensions is necessary for adjustSSL() to work in more real-world
cases.

Also sorted TLSEXT_TYPE_* entries and replaced "#if 0" code with a way
to build Squid to recognize more extensions as OpenSSL's list grows.

8 years agoBug 4455: SegFault from ESIInclude::Start
Amos Jeffries [Tue, 3 May 2016 01:48:49 +0000 (13:48 +1200)] 
Bug 4455: SegFault from ESIInclude::Start

8 years agoFinalized BinaryTokenizer context handling. Polished.
Alex Rousskov [Mon, 2 May 2016 16:38:05 +0000 (10:38 -0600)] 
Finalized BinaryTokenizer context handling. Polished.

No more funny context fields inside TLS structures. Context is handled
by the parsing code without needlessly storing it long-term.

Hid TLS structures/parsers used exclusively by
Security::HandshakeParser inside security/Handshake.cc to simplify API.

Also skipped unused ServerHello.random (instead of storing it in
TlsDetails::clientRandom) and replaced SQUID_TLS_RANDOM_SIZE macro
with a regular C++ constant.

8 years agoBug 4498: URL-unescape the login-info after extraction from URI
Amos Jeffries [Mon, 2 May 2016 15:18:33 +0000 (03:18 +1200)] 
Bug 4498: URL-unescape the login-info after extraction from URI

8 years agoHTTP/1.1: normalize Host header
Amos Jeffries [Mon, 2 May 2016 06:09:13 +0000 (18:09 +1200)] 
HTTP/1.1: normalize Host header

When absolute-URI is provided Host header should be ignored. However some
code still uses Host directly so normalize it using the previously
sanitized URL authority value before doing any further request processing.

For now preserve the case where Host is completely absent. That matters
to the CVE-2009-0801 protection.

This also has the desirable side effect of removing multiple or duplicate
Host header entries.

8 years agoPrevent Squid forcing -b 2048 into the arguments for sslcrtd_program
Nathan Hoad [Mon, 2 May 2016 03:17:18 +0000 (15:17 +1200)] 
Prevent Squid forcing -b 2048 into the arguments for sslcrtd_program

Previously Squid assumed it was running with the default sslcrtd_program, which
takes an argument for the FS block size. This causes issues for administrators
that use their own helpers that happen to take a -b argument that means
something else entirely, causing confusion and preventing them from removing
this argument.

A summary of the changes:

* Move the block size retrieval from Squid into security_file_certgen. It
  does not use fsBlockSize as that introduces a lot of dependencies on
  unrelated Squid code, e.g. fde, Debug, MemBuf.

* Make the -b argument mostly redundant, but leave it there so
  administrators can overrule xstatvfs.

* Fix a small typo.

This work is submitted on behalf of Bloomberg L.P.

8 years agoSourceFormat Enforcement
Source Maintenance [Mon, 2 May 2016 00:12:09 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoAccumulate fewer unknown-size responses to avoid overwhelming disks.
Alex Rousskov [Sun, 1 May 2016 21:37:52 +0000 (15:37 -0600)] 
Accumulate fewer unknown-size responses to avoid overwhelming disks.

Start swapping out an unknown-size entry as soon as size-based cache_dir
selection is no longer affected by the entry growth. If the entry
eventually exceeds the selected cache_dir entry size limits, terminate
the swapout.

The following description assumes that Squid deals with a cachable
response that lacks a Content-Length header. These changes should not
affect other responses.

Prior to these changes, StoreEntry::mayStartSwapOut() delayed swapout
decision until the entire response was received or the already
accumulated portion of the response exceeded the [global] store entry
size limit, whichever came first. This logic protected Store from
entries with unknown sizes. AFAICT, that protection existed for two
reasons:

* Incorrect size-based cache_dir selection: When cache_dirs use
  different min-size and/or max-size settings, Squid cannot tell which
  cache_dir the unknown-size entry belongs to and, hence, may select the
  wrong cache_dir.

* Disk bandwidth/space waste: If the growing entry exceeds all cache_dir
  max-size limits, the swapout has to be aborted, resulting in waste of
  previously spent resources (primarily disk bandwidth and space).

The cost of those protections include RAM waste (up to maximum cachable
object size for each of the concurrent unknown-size entry downloads) and
sudden disk overload (when the entire delayed entry is written to disk
in a large burst of write requests initiated from a tight doPages() loop
at the end of the swapout sequence when the entry size becomes known).
The latter cost is especially high because swapping the entire large
object out in one function call can easily overflow disker queues and/or
block Squid while the OS drains disk write buffers in an emergency mode.

FWIW, AFAICT, cache_dir selection protection was the only reason for
introducing response accumulation (trunk r4446). The RAM cost was
realized a year later (r4954), and the disk costs were realized during
this project.

This change reduces those costs by starting to swap out an unknown-size
entry ASAP, usually immediately. In most caching environments, most
large cachable entries should be cached. It is usually better to spend
[disk] resources gradually than to deal with sudden bursts [of disk
write requests]. Occasional jolts make high performance unsustainable.

This change does not affect size-based cache_dir selection: Squid still
delays swapout until future entry growth cannot affect that selection.
Fortunately, in most configurations, the correct selection can happen
immediately because cache_dirs lack explicit min-size and/or max-size
settings and simply rely on the *same-for-all* minimum_object_size and
maximum_object_size values.

We could make the trade-off between costly protections (i.e., accumulate
until the entry size is known) and occasional gradual resource waste
(i.e., start swapping out ASAP) configurable. However, I think it is
best to wait for the use case that requires such configuration and can
guide the design of those new configuration options.

Side changes:

* Honor forgotten minimum_object_size for cache_dirs without min-size in
  Store::Disk::objectSizeIsAcceptable() and fix its initial value to
  correctly detect a manually configured cache_dir min-size (which may
  be zero). However, the fixed bug is probably hidden by another (yet
  unfixed) bug: checkTooSmall() forgets about cache_dirs with min-size!

* Allow unknown-size objects into the shared memory cache, which code
  could handle partial writes (since collapsed forwarding changes?).

* Fixed Rock::SwapDir::canStore() handling of unknown-size objects. I do
  not see how such objects could get that far before, but if they could,
  most would probably be cached because the bug would hide the unknown
  size from Store::Disk::canStore() that declares them unstorable.

8 years agoBug 4509: EUI compile error on NetBSD
Leonardo Taccari [Sat, 30 Apr 2016 18:48:35 +0000 (06:48 +1200)] 
Bug 4509: EUI compile error on NetBSD

8 years agoStop parsing SSL records after a fatal SSL Alert.
Alex Rousskov [Sat, 30 Apr 2016 03:38:26 +0000 (21:38 -0600)] 
Stop parsing SSL records after a fatal SSL Alert.

The fatal alert sender should close the connection. Waiting for the next
record is pointless and will obscure the problem when we eventually read
the EOF on the socket.

8 years agoSeparated BinaryTokenizer commits from context debugging. Polished.
Alex Rousskov [Sat, 30 Apr 2016 03:35:46 +0000 (21:35 -0600)] 
Separated BinaryTokenizer commits from context debugging. Polished.

Commits are relatively rare events specific to incremental parsing. Most
parsers are not incremental and do not commit/rollback. However, all
parsers need to debug what they parse. Thus, it was wrong to combine
commits with context debugging.

BinaryTokenizer single-context debugging did not support nested contexts
(such as Hello.version.major) and reported wrong FieldGroup sizes for
some parsed structures. The new BinaryTokenizerContext does not have
these problems and is more general (but still needs more polishing
work).

Also polished many field names, comments, debug messages, and some code.

8 years agoSimplified variable-length vector (our "PStrings") contents extraction.
Alex Rousskov [Fri, 29 Apr 2016 00:45:48 +0000 (18:45 -0600)] 
Simplified variable-length vector (our "PStrings") contents extraction.

Earlier code required the caller to first create a PString object and
then extract its .data field to get to the vector contents as an SBuf.
However, that approach just increased code and logging noise because
that SBuf itself (including SBuf.length()) is all the caller may need!

8 years agoFixed parsing of server-sent SNI.
Alex Rousskov [Fri, 29 Apr 2016 00:27:45 +0000 (18:27 -0600)] 
Fixed parsing of server-sent SNI.

The old code could not handle an empty SNI extension that most servers
send. RFC 6066 prose instructs servers to send empty SNI extensions, and
the formal SNI grammar is apparently client-specific. We are not the
only ones being confused by that because there are severs that send
empty ServerNameLists, which are actually prohibited by the grammar.

8 years agoFixed SNI parsing. The old code could only handle single-name SNI lists.
Alex Rousskov [Thu, 28 Apr 2016 19:12:10 +0000 (13:12 -0600)] 
Fixed SNI parsing. The old code could only handle single-name SNI lists.

... and, hence, failed to parse empty SNI lists with misleading
"needMore" errors.

8 years agoShared memory corruption when storing multi-slot (>32KB) shm misses.
Alex Rousskov [Mon, 25 Apr 2016 23:04:05 +0000 (17:04 -0600)] 
Shared memory corruption when storing multi-slot (>32KB) shm misses.

This is a regression I introduced in trunk r14584 (Bug 7: Update
cached entries on 304 responses).

8 years agoCleanup: remove use of MEM_DLINK_NODE for custom link-list
Amos Jeffries [Mon, 25 Apr 2016 08:27:42 +0000 (20:27 +1200)] 
Cleanup: remove use of MEM_DLINK_NODE for custom link-list

... implementation and replaces it all with a std::queue.

Also, de-duplicates the *Dequeue() functions by merging them into helper
class as a single nextRequest() getter method.

8 years agoBe careful with parsed TLS handshake details. They may be missing.
Alex Rousskov [Sat, 23 Apr 2016 04:57:06 +0000 (22:57 -0600)] 
Be careful with parsed TLS handshake details. They may be missing.

TODO: Convert HandshakeParser::details pointer into an always-available
object?

8 years agoRemoved HandshakeParser::parseError and hid/renamed its parseDone.
Alex Rousskov [Sat, 23 Apr 2016 04:46:12 +0000 (22:46 -0600)] 
Removed HandshakeParser::parseError and hid/renamed its parseDone.

The presence of two "persistent" parsing outcomes (done and error) was
confusing to the callers: Which one do I check and when? The adjusted
interface uses exceptions for errors and a false parseHandshake() return
value to signal "need more data". This simplifies the API and untangles
the calling code quite a bit.

8 years agoFixed "make distcheck".
Alex Rousskov [Fri, 22 Apr 2016 17:35:53 +0000 (11:35 -0600)] 
Fixed "make distcheck".

8 years agoRemoved unused methods.
Alex Rousskov [Fri, 22 Apr 2016 17:11:54 +0000 (11:11 -0600)] 
Removed unused methods.

8 years agoFix typo in rev.14651
Amos Jeffries [Fri, 22 Apr 2016 16:57:29 +0000 (04:57 +1200)] 
Fix typo in rev.14651

8 years agoFixed "make check".
Alex Rousskov [Fri, 22 Apr 2016 16:47:59 +0000 (10:47 -0600)] 
Fixed "make check".

8 years agoCleanup: convert late initialized objects to MEMPROXY_CLASS
Amos Jeffries [Fri, 22 Apr 2016 11:39:23 +0000 (23:39 +1200)] 
Cleanup: convert late initialized objects to MEMPROXY_CLASS

Convert all the objects using the libmem "old API" for as-needed pools
to using the MEMPROXY_CLASS() API which is better designed for late
initialization.

8 years agoCode polishing. No functionality changes.
Alex Rousskov [Fri, 22 Apr 2016 05:17:07 +0000 (23:17 -0600)] 
Code polishing. No functionality changes.

8 years agoDo not allocate TlsDetails until throwing isSslv2Record() returns.
Alex Rousskov [Fri, 22 Apr 2016 04:51:24 +0000 (22:51 -0600)] 
Do not allocate TlsDetails until throwing isSslv2Record() returns.

If isSslv2Record() throws InsufficientInput, then we must re-parse these
first few bytes later. Nil details triggers that parsing.

8 years agoMerged two identical methods together. No functionality changes.
Alex Rousskov [Fri, 22 Apr 2016 03:48:37 +0000 (21:48 -0600)] 
Merged two identical methods together. No functionality changes.

8 years agoLack of data at EOF is a parsing error, not a "need more" condition.
Alex Rousskov [Fri, 22 Apr 2016 00:12:20 +0000 (18:12 -0600)] 
Lack of data at EOF is a parsing error, not a "need more" condition.

Branch changes did not distinguish "need more and can expect more" from
"need more but got everything" situations. When one of the inner parsers
that deal with "complete" buffers failed, it would throw
InsufficientInput exception and the higher level parsing code would
interpret that as a sign that more SSL records/fragments are needed,
resulting in stuck transactions instead of parsing errors.

Unlike parsing errors, stuck transactions cannot be bypassed or ignored
so the difference is important.

8 years ago4.0.9 SQUID_4_0_9
Amos Jeffries [Wed, 20 Apr 2016 13:19:37 +0000 (01:19 +1200)] 
4.0.9

8 years agoPrep for 4.0.9 and 3.5.17
Amos Jeffries [Wed, 20 Apr 2016 12:10:08 +0000 (00:10 +1200)] 
Prep for 4.0.9 and 3.5.17

8 years agoFix several ESI element construction issues
Amos Jeffries [Tue, 19 Apr 2016 20:07:52 +0000 (08:07 +1200)] 
Fix several ESI element construction issues

* Do not wrap active logic in assert().

* Fix localbuf array bounds checking.

* Add Must() conditions to verify array writes will succeed

8 years agoDocs: add release notes for on_unsupported_protocol and request_start_timeout
Amos Jeffries [Tue, 19 Apr 2016 15:18:53 +0000 (03:18 +1200)] 
Docs: add release notes for on_unsupported_protocol and request_start_timeout

8 years agoBug 4495: Unknown SSL option SSL_OP_NO_TICKET
Amos Jeffries [Tue, 19 Apr 2016 10:40:05 +0000 (22:40 +1200)] 
Bug 4495: Unknown SSL option SSL_OP_NO_TICKET

8 years agoStop parsing response prefix after discovering an "HTTP/0.9" response.
Alex Rousskov [Mon, 18 Apr 2016 19:08:25 +0000 (13:08 -0600)] 
Stop parsing response prefix after discovering an "HTTP/0.9" response.

Otherwise, our "X-Transformed-From: HTTP/0.9" headers are going to
be ignored, and the rest of the received bytes are going to be parsed
(and modified!) as an HTTP/1 response header, followed by message body.

8 years agoPrevent %O use by deny_info leading to header smuggling
Amos Jeffries [Sun, 17 Apr 2016 11:49:54 +0000 (23:49 +1200)] 
Prevent %O use by deny_info leading to header smuggling

8 years agocachemgr.cgi: use dynamic MemBuf for internal content generation
Amos Jeffries [Sat, 16 Apr 2016 12:01:03 +0000 (00:01 +1200)] 
cachemgr.cgi: use dynamic MemBuf for internal content generation

Using a fixed size buffer limits how big content lines can be. Modern
HTTP is fast reaching the point where such limits are problematic.
Also fixes incorrect uses of snprintf() by removing them.

8 years agoFixed "unused variable" compilation error.
Alex Rousskov [Fri, 15 Apr 2016 18:03:53 +0000 (12:03 -0600)] 
Fixed "unused variable" compilation error.

8 years agoOptimization/simplification: Do not parse unused challenge in v2 Handshake.
Alex Rousskov [Fri, 15 Apr 2016 17:52:20 +0000 (11:52 -0600)] 
Optimization/simplification: Do not parse unused challenge in v2 Handshake.

Also removed pointless commit() and #if-0d code.

8 years agoHandshake Error: ccs received early
Christos Tsantilas [Thu, 14 Apr 2016 17:31:46 +0000 (20:31 +0300)] 
Handshake Error: ccs received early

Some servers cause an SSL handshake error with peek and splice.
The problem is related to the TLS Session Tickets extension handling. Squid
expects always a Tls Session Tickets extension, included in server hello
message, to assume that the ticket accepted and the session is a resumed
session, which is not always true.

This is a Measurement Factory project

8 years agoPartial revert of rev.14638
Amos Jeffries [Thu, 14 Apr 2016 12:01:02 +0000 (00:01 +1200)] 
Partial revert of rev.14638

It did not catch all cases of the SP it was intended to and the chunked
encoding parse will need significantly different changes pending IETF WG
discussions.

Keep the violationLevel() member, which will be useful in general.

8 years agoBug 4493: theObject->sharedMemorySize() == theSegment.size() exception
Alex Rousskov [Wed, 13 Apr 2016 22:36:12 +0000 (16:36 -0600)] 
Bug 4493: theObject->sharedMemorySize() == theSegment.size() exception

We should not expect the exact match because, as discovered during bug
3805 (r13947) fix, shared Segment::size() may exceed the originally
requested RAM amount due to stat() page rounding done by OSes like OS X.

Unfortunately, this rounding weakens the failed consistency check a lot.
TODO: Store the exact requested size and check against that as well.

8 years agoRefactored and probably sped up ServerBio reading.
Alex Rousskov [Wed, 13 Apr 2016 05:40:24 +0000 (23:40 -0600)] 
Refactored and probably sped up ServerBio reading.

I could not grok the logic of the ServerBio::read*() methods and
saw strange cache.log message sequences like this:

  | bio.cc(121) read: FD 13 read -1 <= 65535
  | bio.cc(126) read: error: 11 ignored: 1
  | bio.cc(139) readAndBuffer: read -1 bytes
  | bio.cc(266) read: Pass 5 bytes to openSSL as read
  | bio.cc(121) read: FD 13 read -1 <= 65535
  | bio.cc(126) read: error: 11 ignored: 1
  | bio.cc(139) readAndBuffer: read -1 bytes
  | bio.cc(266) read: Pass 4 bytes to openSSL as read

Initially, they looked like we were [incorrectly] subtracting -1 from
some buffer length (read -1, pass 5; read -1, pass 4). Now, I believe
they [just] indicated unnecessary network reads. The fixed sequence
looks similar to this (note no network reads):

  | bio.cc(289) giveBuffered: Pass 5 read bytes to openSSL
  | bio.cc(289) giveBuffered: Pass 4 read bytes to openSSL

The refactored ServerBio code starts in "parsing" state (SSL Hello
parsing is the primary ServerBio functionality). Only when that parsing
is over, ServerBio starts feeding OpenSSL with received bytes. This
internal logic allows us to hide parsing from callers and avoid the
confusing public holdRead API.

8 years agoThe "hold write" conditions are not important enough for debug level 1.
Alex Rousskov [Wed, 13 Apr 2016 05:36:05 +0000 (23:36 -0600)] 
The "hold write" conditions are not important enough for debug level 1.

... and are already reported at lower levels in ServerBio.

I was getting one of this messages for every(?) transaction in Polygraph
tests.

8 years agoFixed SSL server Hello parsing.
Alex Rousskov [Tue, 12 Apr 2016 18:28:03 +0000 (12:28 -0600)] 
Fixed SSL server Hello parsing.

We must parse cipher and compression method before extensions.

Also fixed extensions detection to match RFC 5246 algorithm.

8 years agoSourceFormat Enforcement
Source Maintenance [Tue, 12 Apr 2016 18:12:15 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoBug 4492: Handle SP padded size in chunked encoding
Amos Jeffries [Tue, 12 Apr 2016 15:07:13 +0000 (03:07 +1200)] 
Bug 4492: Handle SP padded size in chunked encoding

8 years agoRemoved ServerOptions "partial copy" copy constructor.
Alex Rousskov [Mon, 11 Apr 2016 16:34:29 +0000 (10:34 -0600)] 
Removed ServerOptions "partial copy" copy constructor.

AFAICT, the default copy constructor should work and the removed
explicit constructor was not copying the staticContext member, for no
documented reason (that I could find). It was also unused [in my tests].

If the partial copy constructor was abused for something useful, then a
different approach should be found -- the one that does not violate the
standard copy constructor post-conditions.

8 years agoAvoid startup/shutdown crashes [by avoiding static non-POD globals].
Alex Rousskov [Mon, 11 Apr 2016 15:14:58 +0000 (09:14 -0600)] 
Avoid startup/shutdown crashes [by avoiding static non-POD globals].

Squid crashes on startup when the parent process exit()s after fork()ing
the kid process. Squid may also crash on shutdown after exiting main().

In both cases, the crashes are build- and environment-specific. Many
environments show no problems at all. Even disabling compiler
optimizations may prevent crashes. When crashes do happen, their
symptoms (e.g., backtrace) point to problems during destruction of
global objects, but the details point to innocent objects (e.g., PortCfg
or SSL_CTX).

In some environments, the following malloc error is printed on console
before the crash: "corrupted double-linked list".

This change replaces two StatHist globals used for SBuf statistics
collection with always-available singletons. The replaced globals could
be destructed before the last SBuf object using them, leading to memory
corruption (that would eventually crash Squid).

There are probably more such globals.

8 years agoAdd a new error page token for unquoted external ACL messages.
Nathan Hoad [Sun, 10 Apr 2016 04:31:51 +0000 (16:31 +1200)] 
Add a new error page token for unquoted external ACL messages.

This is useful for external ACLs that send back messages that contain
actual HTML.

This work is submitted on behalf of Bloomberg L.P.

8 years agoBug 4465: Header forgery detection leads to crash
Alex Rousskov [Sat, 9 Apr 2016 15:35:34 +0000 (03:35 +1200)] 
Bug 4465: Header forgery detection leads to crash

8 years agoRecord SSL client and SSL server details (supported TLS version/requested
Christos Tsantilas [Fri, 8 Apr 2016 10:58:07 +0000 (13:58 +0300)] 
Record SSL client and SSL server details (supported TLS version/requested
TLS version) for fast-sni branch

Currently the fast-SNI-peek branch does not parse SSL client hello in
Client-First and Server-First bumping modes. This patch always "peeks" the
SSL client hello message and apply the squid TLS parser for Client-First
and Server-First modes to.

Also this patch moves the code which retrieves SSL server details from
PeekingPeerConnector class to PeerConnector class t retrieve details  for all
SSL server side connections.

8 years agoFix shm_open error message after rev.14625
Amos Jeffries [Fri, 8 Apr 2016 07:35:47 +0000 (19:35 +1200)] 
Fix shm_open error message after rev.14625

8 years agoBug 4405: assertion failed: comm.cc:554: "Comm::IsConnOpen(conn)"
Christos Tsantilas [Thu, 7 Apr 2016 16:36:10 +0000 (19:36 +0300)] 
Bug 4405: assertion failed: comm.cc:554: "Comm::IsConnOpen(conn)"

 It is possible that the connection will be closed somewhere inside
"clientTunnelOnError" call, inside ConnStateData::fakeAConnectRequest which
is called by ConnStateData::clientTunnelOnError or inside spliceOnError()
while trying to splice(). In this case the callers should be informed to abort
imediatelly, but instead continues, and try to set timeout handler on closed
connection.

This patch:
  - Modify ConnStateData::fakeAConnectRequest and ConnStateData::splice methods     to return boolean and false on error.
  - Does not close the connection inside ConnStateData::fakeAConnectRequest but
    instead return false and allow callers to close the connection if required.

This is a Measurement Factory project

8 years agoBug 4481: varyEvaluateMatch: Oops. Not a Vary match on second attempt
Amos Jeffries [Thu, 7 Apr 2016 13:10:28 +0000 (01:10 +1200)] 
Bug 4481: varyEvaluateMatch: Oops. Not a Vary match on second attempt

8 years agoBug 4482: Solaris GCC 5.2 warning in src/ip/Intercept.cc
Carsten Grzemba [Thu, 7 Apr 2016 12:03:53 +0000 (00:03 +1200)] 
Bug 4482: Solaris GCC 5.2 warning in src/ip/Intercept.cc

8 years agoFix missing variable type after rev.14625
Amos Jeffries [Thu, 7 Apr 2016 11:12:09 +0000 (23:12 +1200)] 
Fix missing variable type after rev.14625