]> git.ipfire.org Git - thirdparty/squid.git/log
thirdparty/squid.git
7 years agoBug 4004 partial: Fix segfault via Ftp::Client::readControlReply
Christos Tsantilas [Wed, 30 Nov 2016 21:56:30 +0000 (10:56 +1300)] 
Bug 4004 partial: Fix segfault via Ftp::Client::readControlReply

Added nil dereference checks for Ftp::Client::ctrl.conn, including:
- Ftp::Client::handlePasvReply() and handleEpsvReply() that dereference
  ctrl.conn in DBG_IMPORTANT messages.
- Many functions inside FtpClient.cc and FtpGateway.cc files.

TODO: We need to find a better way to handle nil ctrl.conn. It is only
a matter of time when we forget to add another dereference check or
discover a place we missed during this change.

Also disabled forwarding of EPRT and PORT commands to origin servers.
Squid support for those commands is broken and their forwarding may
cause segfaults (bug #4004). Active FTP is still supported, of course.

This is a Measurement Factory project

7 years agoImprove debugs warnings when loading signing certs fails
Amos Jeffries [Wed, 30 Nov 2016 15:42:05 +0000 (04:42 +1300)] 
Improve debugs warnings when loading signing certs fails

7 years agoTLS: Make key= before cert= an error instead of quietly hiding the issue
Amos Jeffries [Tue, 15 Nov 2016 07:57:28 +0000 (20:57 +1300)] 
TLS: Make key= before cert= an error instead of quietly hiding the issue

This squid.conf setup is fatal in Squid-4. So best to fix these installations.
Even though Squdi-3 can cope with it.

7 years agoHonor SBufReservationRequirements::minSize regardless of idealSize.
Alex Rousskov [Mon, 14 Nov 2016 12:40:51 +0000 (01:40 +1300)] 
Honor SBufReservationRequirements::minSize regardless of idealSize.

  In a fully specified SBufReservationRequirements, idealSize would
  naturally match or exceed minSize. However, the idealSize default value
  (zero) may not. We should honor minSize regardless of idealSize, just as
  the API documentation promises to do.

  No runtime changes expected right now because the only existing user of
  SBufReservationRequirements sets .idealSize to CLIENT_REQ_BUF_SZ (4096)
  and .minSize to 1024.

7 years agoFix spelling for digest nonce cache maintenance event
Garri Djavadyan [Mon, 14 Nov 2016 10:54:34 +0000 (23:54 +1300)] 
Fix spelling for digest nonce cache maintenance event

7 years agoFix ssl::server_name ACL badly broken since inception.
Christos Tsantilas [Mon, 14 Nov 2016 10:51:24 +0000 (23:51 +1300)] 
Fix ssl::server_name ACL badly broken since inception.

The original server_name code mishandled all SNI checks and some rare
host checks:

* The SNI-derived value was pointing to an already freed memory storage.
* Missing host-derived values were not detected (host() is never nil).
* Mismatches were re-checked with an undocumented "none" value
  instead of being treated as mismatches.

Same for ssl::server_name_regex.

Also set SNI for more server-first and client-first transactions.

This is a Measurement Factory project.

7 years agoBug 3379: Combination of If-Match and a Cache Hit result in TCP Connection Failure
Garri Djavadyan [Fri, 11 Nov 2016 06:03:25 +0000 (19:03 +1300)] 
Bug 3379: Combination of If-Match and a Cache Hit result in TCP Connection Failure

7 years agoFix build issue after rev.14105
Amos Jeffries [Tue, 1 Nov 2016 11:22:31 +0000 (00:22 +1300)] 
Fix build issue after rev.14105

7 years agoHTTP/1.1: make Vary:* objects cacheable
Amos Jeffries [Sun, 30 Oct 2016 09:45:03 +0000 (22:45 +1300)] 
HTTP/1.1: make Vary:* objects cacheable

Under new clauses from RFC 7231 section 7.1.4 and HTTP response
containing header Vary:* (wifcard variant) can be cached, but
requires revalidation with server before each use.

Use the new mandatory revalidation flags to allow storing of any
wildcard Vary:* response.

Note that responses with headers like Vary:A,B,C,* are equivalent
to Vary:*. The cache key string for these objects is normalized.

7 years agoFix debug message in ACLChecklist::bannedAction()
Garri Djavadyan [Sun, 30 Oct 2016 09:40:25 +0000 (22:40 +1300)] 
Fix debug message in ACLChecklist::bannedAction()

7 years agoBug 4567: Strange IPv6 shown in access.log
Amos Jeffries [Sun, 30 Oct 2016 09:39:20 +0000 (22:39 +1300)] 
Bug 4567: Strange IPv6 shown in access.log

7 years agoCopyright: add some missing blurbs and contributor details
Amos Jeffries [Sun, 30 Oct 2016 09:38:16 +0000 (22:38 +1300)] 
Copyright: add some missing blurbs and contributor details

7 years agoBug 4627: fix generate-host-certificates and dynamic_cert_mem_cache_size docs
Amos Jeffries [Sat, 29 Oct 2016 23:26:28 +0000 (12:26 +1300)] 
Bug 4627: fix generate-host-certificates and dynamic_cert_mem_cache_size docs

For Squid-3 the fix is just to update the documentation.

7 years agoBug 4620: NetBSD build error with --enable-ipf-transparent
Takahiro Kambe [Tue, 25 Oct 2016 08:25:30 +0000 (21:25 +1300)] 
Bug 4620: NetBSD build error with --enable-ipf-transparent

On NetBSD sys/param.h must be included before netinet/ip_compat.h

7 years agoFix external_acl_type default children documentations
Amos Jeffries [Tue, 25 Oct 2016 08:23:49 +0000 (21:23 +1300)] 
Fix external_acl_type default children documentations

The max children has always been 5, not 20.

Also, make mgr:config report dumper actually hide only the real default
values. (sync with helper/ChildConfig.cc defaults)

7 years agoFix regression bug introduced by r14089.
Christos Tsantilas [Tue, 25 Oct 2016 08:19:49 +0000 (21:19 +1300)] 
Fix regression bug introduced by r14089.

Squid crashed because HttpMsg::body_pipe was used without check that it
was initialized. The message lacks body pipe when it has no body or
empty body.

7 years agoFix build with eCAP but without ICAP support.
Alex Rousskov [Sat, 15 Oct 2016 04:20:24 +0000 (17:20 +1300)] 
Fix build with eCAP but without ICAP support.

That is, when ./configured with --enable-ecap --disable-icap-client.

AccessLogEntry::icap requires ICAP_CLIENT, not just USE_ADAPTATION.

7 years ago3.5.22 SQUID_3_5_22
Amos Jeffries [Sun, 9 Oct 2016 19:57:39 +0000 (08:57 +1300)] 
3.5.22

7 years agoHTTP: MUST ignore a [revalidation] response with an older Date header.
Eduard Bagdasaryan [Sun, 9 Oct 2016 19:47:26 +0000 (08:47 +1300)] 
HTTP: MUST ignore a [revalidation] response with an older Date header.

Before this patch, Squid violated the RFC 7234 section 4 MUST
requirement: "When more than one suitable response is stored, a cache
MUST use the most recent response (as determined by the Date header
field)." This problem may be damaging in cache hierarchies where
parent caches may have different responses. Trusting the older response
may lead to excessive IMS requests, cache thrashing and other problems.

7 years agoOptimized/simplified buffering: Appending nothing is always possible.
Alex Rousskov [Sun, 9 Oct 2016 15:55:58 +0000 (04:55 +1300)] 
Optimized/simplified buffering: Appending nothing is always possible.

This change avoids CoW when appending an empty buffer to MemBlob or
SBuf, making "if !empty then append()" caller complications unnecessary.

7 years agoHide OpenSSL tricks from Valgrind far-reaching initialization errors.
Alex Rousskov [Sun, 9 Oct 2016 14:30:11 +0000 (03:30 +1300)] 
Hide OpenSSL tricks from Valgrind far-reaching initialization errors.

This change has no effect unless ./configured --with-valgrind-debug.

OpenSSL, including its Assembly code, contains many optimizations and
timing defenses that Valgrind misinterprets as uninitialized value
usage. Most of those tricks can be disabled by #defining PURIFY when
building OpenSSL, but some are not protected with PURIFY and most
OpenSSL libraries are (and should be) built without that #define.

To make matters worse, once Valgrind misdetects uninitialized memory, it
will complain about every usage of that memory. Those complaints create
a lot of noise, complicate triage, and effectively mask true bugs.
AFAICT, they cannot be suppressed by listing the source of that memory.

For example, this OpenSSL Assembly trick:
    Uninitialised value was created by a stack allocation
       at 0x556C2F7: aesni_cbc_encrypt (aesni-x86_64.s:2081)

Triggers many false errors like this one:
    Conditional jump or move depends on uninitialised value(s)
       by 0x750838: Debug::Finish()
       by 0x942E68: Http::One::ResponseParser::parse(SBuf const&)
       ...

This change marks OpenSSL-returned decrypted bytes as initialized. This
might miss some true OpenSSL bugs, but we should focus on Squid bugs.

7 years agoAvoid segfaults when debugging section 4 at level 9.
Alex Rousskov [Sun, 9 Oct 2016 12:36:16 +0000 (01:36 +1300)] 
Avoid segfaults when debugging section 4 at level 9.

The bug was probably added in r11496. It was exposed by Valgrind's
"Conditional jump or move depends on uninitialised value(s)" error.

7 years agoSourceFormat Enforcement
Source Maintenance [Sun, 9 Oct 2016 00:14:14 +0000 (00:14 +0000)] 
SourceFormat Enforcement

7 years agoBug 4302 pt2: IPFilter v5 transparent interception
Amos Jeffries [Sat, 8 Oct 2016 20:37:35 +0000 (09:37 +1300)] 
Bug 4302 pt2: IPFilter v5 transparent interception

7 years agoBug 4594: build failure with clang 3.9
Amos Jeffries [Sat, 24 Sep 2016 22:36:05 +0000 (11:36 +1300)] 
Bug 4594: build failure with clang 3.9

7 years agoBug 4471: revalidation doesn't work when expired cached object lacks Last-Modified.
Eduard Bagdasaryan [Fri, 23 Sep 2016 20:49:24 +0000 (08:49 +1200)] 
Bug 4471: revalidation doesn't work when expired cached object lacks Last-Modified.

The bug was caused by the fact that Squid used only Last-Modified header
value for evaluating entry's last modification time while making an
internal revalidation request. So, without Last-Modified it was not
possible to correctly fill If-Modified-Since header value. As a result,
the revalidation request was not initiated when it should be.

To fix this problem, we should generate If-Modified-Since based on other
data, showing how "old" the cached response is, such as Date and Age
header values. Both Date and Age header values are utilized while
calculating entry's timestamp. Therefore, we should use the timestamp if
Last-Modified is unavailable.

TODO: HttpRequest::imslen support looks broken/deprecated. Using this
field inside StoreEntry::modifiedSince() leads to several useless checks
and probably may cause other [hidden] problems.

7 years agoSourceFormat Enforcement
Source Maintenance [Fri, 23 Sep 2016 18:14:24 +0000 (18:14 +0000)] 
SourceFormat Enforcement

7 years agoBug 2833: Collapse internal revalidation requests (SMP-unaware caches)
Eduard Bagdasaryan [Fri, 23 Sep 2016 15:28:42 +0000 (03:28 +1200)] 
Bug 2833: Collapse internal revalidation requests (SMP-unaware caches)

... also address Bug 4311 and partially address Bug 2833 and Bug 4471.

Extend collapsed_forwarding functionality to internal revalidation
requests. This implementation does not support Vary-controlled cache
objects and is limited to SMP-unaware caching environments, where each
Squid worker knows nothing about requests and caches handled by other
workers. However, it also lays critical groundwork for future SMP-aware
collapsed revalidation support.

Prior to these changes, multiple concurrent HTTP requests for the same
stale cached object always resulted in multiple internal revalidation
requests sent by Squid to the origin server. Those internal requests
were likely to result in multiple competing Squid cache updates, causing
cache misses and/or more internal revalidation requests, negating
collapsed forwarding savings.

Internal cache revalidation requests are collapsed if and only if
collapsed_forwarding is enabled. There is no option to control just
revalidation collapsing because there is no known use case for it.

* Public revalidation keys

Each Store entry has a unique key. Keys are used to find entries in the
Store (both already cached/swapped_out entries and not). Public keys are
normally tied to the request method and target URI. Same request
properties normally lead to the same public key, making cache hits
possible. If we were to calculate a public key for an internal
revalidation request, it would have been the same as the public key of
the stale cache entry being revalidated. Adding a revalidation response
to Store would have purged that being-revalidated cached entry, even if
the revalidation response itself was not cachable.

To avoid purging being-revalidated cached entries, the old code used
private keys for internal revalidation requests. Private keys are always
unique and cannot accidentally purge a public entry. On the other hand,
for concurrent [revalidation] requests to find the store entry to
collapse on, that store entry has to have a public key!

We resolved this conflict by adding "scope" to public keys:

* Regular/old public keys have default empty scope (that does not affect
  key generation). The code not dealing with collapsed revalidation
  continues to work as before. All entries stored in caches continue to
  have the same keys (with an empty scope).

* When collapsed forwarding is enabled, collapsable internal
  revalidation requests get public keys with a "revalidation" scope
  (that is fed to the MD5 hash when the key is generated). Such a
  revalidation request can find a matching store entry created by
  another revalidation request (and collapse on it), but cannot clash
  with the entry being revalidated (because that entry key is using a
  different [empty] scope).

This change not only enables collapsing of internal revalidation
requests within one worker, but opens the way for SMP-aware workers to
share information about collapsed revalidation requests, similar to how
those workers already share information about being-swapped-out cache
entries.

After receiving the revalidation response, each collapsed revalidation
request may call updateOnNotModified() to update the stale entry [with
the same revalidation response!]. Concurrent entry updates would have
wasted many resources, especially for disk-cached entries that support
header updates, and may have purged being-revalidated entries due to
locking conflicts among updating transactions. To minimize these
problems, we adjusted header and entry metadata updating logic to skip
the update if nothing have changed since the last update.

Also fixed Bug 4311: Collapsed forwarding deadlocks for SMP Squids using
SMP-unaware caches. Collapsed transactions stalled without getting a
response because they were waiting for the shared "transients" table
updates. The table was created by Store::Controller but then abandoned (not
updated) by SMP-unaware caches. Now, the transients table is not created
at all unless SMP-aware caches are present. This fix should also address
complaints about shared memory being used for Squid instances without
SMP-aware caches.

A combination of SMP-aware and SMP-unaware caches is still not supported
and is likely to stall collapsed transactions if they are enabled. Note
that, by default, the memory cache is SMP-aware in SMP environments.

7 years agoBug 3819: "fd >= 0" assertion in file_write() during reconfiguration
Alex Rousskov [Fri, 23 Sep 2016 12:40:44 +0000 (00:40 +1200)] 
Bug 3819: "fd >= 0" assertion in file_write() during reconfiguration

Other possible assertions include "NumberOfUFSDirs" in UFSSwapDir and
"fd >= 0" in file_close(). And Fs::Ufs::UFSStoreState::drainWriteQueue()
may segfault while dereferencing nil theFile, although I am not sure
that bug is caused specifically by reconfiguration.

Since trunk r9181.3.1, reconfiguration is done in at least two steps:
First, mainReconfigureStart() closes/cleans/disables all modules
supporting hot reconfiguration and then, at the end of the event loop
iteration, mainReconfigureFinish() opens/configures/enables them again.
UFS code hits assertions if it has to log entries or rebuild swap.state
between those two steps. The tiny assertion opportunity window hides
these bugs from most proxies that are not doing enough disk I/O or are
not being reconfigured frequently enough.

Asynchronous UFS cache_dirs such as diskd were the most exposed, but
even blocking UFS caching code could probably hit [rebuild] assertions.

The swap.state rebuilding (always initiated at startup) probably did not
work as intended if reconfiguration happened during the rebuild time
because reconfiguration closed the swap.state file being rebuilt. We now
protect that swap.state file and delay rebuilding progress until
reconfiguration is over.

TODO: To squash more similar bugs, we need to move hot reconfiguration
support into the modules so that each module can reconfigure instantly.

7 years agoDo not leak url_rewrite_extras and store_id_extras on reconfigure/shutdown.
Alex Rousskov [Fri, 23 Sep 2016 11:22:48 +0000 (23:22 +1200)] 
Do not leak url_rewrite_extras and store_id_extras on reconfigure/shutdown.

TODO: We should not create unneeded extras either.

7 years agoDo reset $HOME if needed after r13435. Minimize putenv() memory leaks.
Alex Rousskov [Fri, 23 Sep 2016 11:11:48 +0000 (23:11 +1200)] 
Do reset $HOME if needed after r13435. Minimize putenv() memory leaks.

While r13435 is attributed to me, the wrong condition was not mine. This
is a good example of why "cmp() op 0" pattern is usually safer because
the "==" or "!=" operator "tells" us whether the strings are equal,
unlike "!cmp()" that is often misread as "not equal".

7 years agoBug 4228: ./configure bug/typo in r14394.
Alex Rousskov [Wed, 21 Sep 2016 02:09:36 +0000 (14:09 +1200)] 
Bug 4228: ./configure bug/typo in r14394.

The bug resulted in "test: too many arguments" error messages when
running ./configure and effectively replaced AC_MSG_ERROR() with
AC_MSG_WARN() for missing but required Heimdal and GNU GSS Kerberos
libraries.

7 years agoFix potential ICAP null pointer dereference after rev.14082
Eduard Bagdasaryan [Fri, 16 Sep 2016 18:50:04 +0000 (06:50 +1200)] 
Fix potential ICAP null pointer dereference after rev.14082

Adjusted Adaptation::Icap::ModXact::finalizeLogInfo(), fixing possible
"Null pointer dereference".

 Detected by Coverity Scan. Issue 1372977

7 years agoFix logged request size (%http::>st) and other size-related %codes.
Eduard Bagdasaryan [Fri, 16 Sep 2016 11:53:28 +0000 (23:53 +1200)] 
Fix logged request size (%http::>st) and other size-related %codes.

The %[http::]>st logformat code should log the actual number of
[dechunked] bytes received from the client. However, for requests with
Content-Length, Squid was logging the sum of the request header size and
the Content-Length header field value instead. The logged value was
wrong when the client sent fewer than Content-Length body bytes.

Also:

* Fixed %http::>h and %http::<h format codes for icap_log. The old code
  was focusing on preserving the "request header" (i.e. not "response
  header") meaning of the %http::>h logformat code, but since ICAP
  services deal with (and log) both HTTP requests and responses, that
  focus on the HTTP message kind actually complicates ICAP logging
  configuration and will eventually require introduction of new %http
  logformat codes that would be meaningless in access.log context.

  - In ICAP context, %http::>h should log to-be-adapted HTTP message
    headers embedded in an ICAP request (HTTP request headers in REQMOD;
    HTTP response headers in RESPMOD). Before this change, %http::>h
    logged constant/"FYI" HTTP request headers in RESPMOD.

  - Similarly, %http::<h should log adapted HTTP message headers
    embedded in an ICAP response (HTTP request headers in regular
    REQMOD; HTTP response headers in RESPMOD and during request
    satisfaction in REQMOD). Before this change, %http::<h logged "-" in
    REQMOD.

  In other words, in ICAP logging context, the ">" and "<" characters
  should indicate ICAP message kind (where the being-logged HTTP message
  is embedded), not HTTP message kind, even for %http codes.

  TODO: %http::>h code logs "-" in RESPMOD because AccessLogEntry does
  not store virgin HTTP response headers.

* Correctly initialize ICAP ALE HTTP fields related to HTTP message
  sizes for icap_log, including %http::>st, %http::<st, %http::>sh, and
  %http::>sh format codes.

* Initialize HttpMsg::hdr_sz in HTTP header parsing code. Among other
  uses, this field is needed to initialize HTTP fields inside ICAP ALE.

* Synced default icap_log format documentation with the current code.

7 years ago3.5.21 SQUID_3_5_21
Amos Jeffries [Thu, 8 Sep 2016 18:55:14 +0000 (06:55 +1200)] 
3.5.21

7 years agoHTTP: do not allow Proxy-Connection to override Connection header
Amos Jeffries [Thu, 8 Sep 2016 13:14:40 +0000 (01:14 +1200)] 
HTTP: do not allow Proxy-Connection to override Connection header

Proxy-Connection header is never actually valid, it is relevant in
HTTP/1.0 messages only when Connection header is missing and not
relevant at all in HTTP/1.1 messages.

This fixes part of the behaviour, making Squid use Connection header
for persistence (keep-alive vs close) checking if one is present
instead of letting Proxy-Connection override it.

TODO: Proxy-Connection still needs to be ignored completely when
      the message version is HTTP/1.1.

7 years agoSSL CN wildcard must only match a single domain component [fragment].
Christos Tsantilas [Thu, 8 Sep 2016 12:27:06 +0000 (00:27 +1200)] 
SSL CN wildcard must only match a single domain component [fragment].

When comparing the requested domain name with a certificate Common Name,
Squid expanded wildcard to cover more than one domain name label (a.k.a
component), violating RFC 2818 requirement[1]. For example, Squid
thought that wrong.host.example.com matched a *.example.com CN.

    [1] "the wildcard character * ... is considered to match any single
    domain name component or component fragment. E.g., *.a.com matches
    foo.a.com but not bar.foo.a.com".

In other contexts (e.g., ACLs), wildcards expand to all components.
matchDomainName() now accepts a mdnRejectSubsubDomains flag that selects
the right behavior for CN match validation.

The old boolean honorWildcards parameter replaced with a flag, for clarity
and consistency sake.

This patch also handles the cases where the host name consists only from dots
(eg malformed Host header or SNI info). The old code has undefined behaniour
in these cases. Moreover it handles the cases a certificate contain zero length
string as CN or alternate name.

This is a Measurement Factory project.

7 years agoSourceFormat Enforcement
Source Maintenance [Wed, 7 Sep 2016 18:14:33 +0000 (18:14 +0000)] 
SourceFormat Enforcement

7 years agoAdd missing updateLimits() to stub_HelperChildConfig.cc
Amos Jeffries [Wed, 7 Sep 2016 16:31:37 +0000 (04:31 +1200)] 
Add missing updateLimits() to stub_HelperChildConfig.cc

7 years agoHTTP: MUST always revalidate Cache-Control:no-cache responses.
Eduard Bagdasaryan [Wed, 7 Sep 2016 15:58:59 +0000 (03:58 +1200)] 
HTTP: MUST always revalidate Cache-Control:no-cache responses.

Squid MUST NOT use a CC:no-cache response for satisfying subsequent
requests without successful validation with the origin server. Squid
violated this MUST because Squid mapped both "no-cache" and
"must-revalidate"/etc. directives to the same ENTRY_REVALIDATE flag
which was interpreted as "revalidate when the cached response becomes
stale" rather than "revalidate on every request".

This patch splits ENTRY_REVALIDATE into:
- ENTRY_REVALIDATE_ALWAYS, for entries with CC:no-cache and
- ENTRY_REVALIDATE_STALE, for entries with other related CC directives
  like CC:must-revalidate and CC:proxy-revalidate.

Reusing ENTRY_CACHABLE_RESERVED_FOR_FUTURE_USE value for the more
forgiving ENTRY_REVALIDATE_STALE (instead of the more aggressive
ENTRY_REVALIDATE_ALWAYS) fixes the bug for the already cached entries -
they will be revalidated and stored with the correct flag on the next
request.

7 years agoFix logic error in rev.13930
Amos Jeffries [Wed, 17 Aug 2016 13:34:13 +0000 (01:34 +1200)] 
Fix logic error in rev.13930

 Using !=0 on both string compares means any login= value will permit
 40x responses through. Only PASS and PASSTHRU should be doing that.

 Detected by Coverity Scan. Issue 1364711

7 years agoBug 3025: Proxy-Authenticate problem using ICAP server
mkishi [Wed, 17 Aug 2016 05:48:29 +0000 (17:48 +1200)] 
Bug 3025: Proxy-Authenticate problem using ICAP server

7 years agoBug 4563: duplicate code in httpMakeVaryMark
Amos Jeffries [Wed, 17 Aug 2016 05:10:37 +0000 (17:10 +1200)] 
Bug 4563: duplicate code in httpMakeVaryMark

7 years agoFix SSL-Bump failure results in SEGFAULT
Amos Jeffries [Wed, 17 Aug 2016 02:58:28 +0000 (14:58 +1200)] 
Fix SSL-Bump failure results in SEGFAULT

7 years agoBug 4428: mal-formed Cache-Control:stale-if-error header
Amos Jeffries [Wed, 17 Aug 2016 02:55:01 +0000 (14:55 +1200)] 
Bug 4428: mal-formed Cache-Control:stale-if-error header

7 years agoSquid segfault via Ftp::Client::readControlReply().
Christos Tsantilas [Fri, 5 Aug 2016 14:59:33 +0000 (02:59 +1200)] 
Squid segfault via Ftp::Client::readControlReply().

Ftp::Client::scheduleReadControlReply(), which may called from the
asynchronous start() or readControlReply()/handleControlReply()
handlers, does not check whether the control connection is still usable
before using it.

This is a Measurement Factory project.

7 years agoSourceFormat Enforcement
Source Maintenance [Sat, 23 Jul 2016 12:13:51 +0000 (12:13 +0000)] 
SourceFormat Enforcement

7 years agoBug #4542: authentication credentials IP TTL updated incorrectly
Anonymous [Sat, 23 Jul 2016 07:19:30 +0000 (19:19 +1200)] 
Bug #4542: authentication credentials IP TTL updated incorrectly

7 years agoBug 4534: assertion failure in xcalloc when using many cache_dir
Amos Jeffries [Sat, 23 Jul 2016 07:16:20 +0000 (19:16 +1200)] 
Bug 4534: assertion failure in xcalloc when using many cache_dir

7 years ago3.5.20 SQUID_3_5_20
Amos Jeffries [Fri, 1 Jul 2016 11:36:16 +0000 (23:36 +1200)] 
3.5.20

7 years agoAssertion failed: Write.cc:38: "fd_table[conn->fd].flags.open"
Christos Tsantilas [Thu, 30 Jun 2016 21:09:12 +0000 (09:09 +1200)] 
Assertion failed: Write.cc:38: "fd_table[conn->fd].flags.open"

The Ftp::Server::stopWaitingForOrigin() notification may come after
Ftp::Server (or an external force) has started closing the control
connection but before the Ftp::Server job became unreachable for
notifications. Writing a response in this state leads to assertions.

Other, currently unknown paths may lead to the same write-after-close
problems. This change protects all asynchronous notification methods
(except the connection closure handler itself) from exposing underlying
code to a closing control connection. This is very similar to checking
for ERR_CLOSING in Comm handlers.

This is a Measurement Factory project.

7 years agoBug 4523: smblib compile fails on NetBSD
Ian Clark [Thu, 30 Jun 2016 20:53:30 +0000 (08:53 +1200)] 
Bug 4523: smblib compile fails on NetBSD

7 years agoDo not make bogus recvmsg(2) calls when closing UDS sockets.
Alex Rousskov [Thu, 30 Jun 2016 20:51:54 +0000 (08:51 +1200)] 
Do not make bogus recvmsg(2) calls when closing UDS sockets.

comm_empty_os_read_buffers() assumes that all non-blocking
FD_READ_METHODs can read into an opaque buffer filled with random
characters. That assumption is wrong for UDS sockets that require an
initialized msghdr structure. Feeding random data to recvmsg(2) leads to
confusing errors, at best. Squid does not log those errors, but they
are visible in, for example, strace:

  recvmsg(17, 0x7fffbb, MSG_DONTWAIT) = -1 EMSGSIZE (Message too long)

comm_empty_os_read_buffers() is meant to prevent TCP RST packets. The
function now ignores UDS sockets that are not used for TCP.

TODO: Useless reads may also exist for UDP and some TCP sockets.

7 years agoFix SEGFAULT parsing malformed adaptation service configuration
Amos Jeffries [Thu, 30 Jun 2016 20:47:53 +0000 (08:47 +1200)] 
Fix SEGFAULT parsing malformed adaptation service configuration

icap_service or ecap_service directives with missing field values need to
abort cleanly with an error.

Also, fix config parsing error messages in adaptation service parse to
use "ERROR:" prefix consistent with squid.conf general behaviour.

7 years agoFixed ConnStateData::In::maybeMakeSpaceAvailable() logic.
Alex Rousskov [Sat, 18 Jun 2016 13:36:07 +0000 (01:36 +1200)] 
Fixed ConnStateData::In::maybeMakeSpaceAvailable() logic.

This change fixes logic bugs that mostly affect performance: In micro-
tests, this change gives 10% performance improvement.

maybeMakeSpaceAvailable() is called with an essentially random in.buf.
The method must prepare in.buf for the next network read. The old code
was not doing that [well enough], leading to performance problems.

In some environments, in.buf often ends up having tiny space exceeding 2
bytes (e.g., 6 bytes). This happens, for example, when Squid creates and
parses a fake CONNECT request. The old code often left such tiny in.bufs
"as is" because we tried to ensure that we have at least 2 bytes to read
instead of trying to provide a reasonable number of buffer space for the
next network read. Tiny buffers naturally result in tiny network reads,
which are very inefficient, especially for non-incremental parsers.

I have removed the explicit "2 byte" space checks: Both the new and the
old code do not _guarantee_ that at least 2 bytes of buffer space are
always available, and the caller does not check that condition either.
If some other code relies on it, more fixes will be needed (but this
change is not breaking that guarantee -- either it was broken earlier or
was never fully enforced). In practice, only buffers approaching
Config.maxRequestBufferSize limit may violate this guarantee AFAICT, and
those buffers ought to be rare, so the bug, if any, remains unnoticed.

Another subtle maybeMakeSpaceAvailable() problem was that the code
contained its own buffer capacity increase algorithm (n^2 growth).
However, increasing buffer capacity exponentially does not make much
sense because network read sizes are not going to increase
exponentially. Also, memAllocStringmemAllocate() overwrites n^2 growth
with its own logic. Besides, it is buffer _space_, not the total
capacity that should be increased. More work is needed to better match
Squid buffer size for from-user network reads with the TCP stack buffers
and traffic patterns.

Both the old and the new code reallocate in.buf MemBlobs. However, the
new code leaves "reallocate or memmove" decision to the new
SBuf::reserve(), opening the possibility for future memmove
optimizations that SBuf/MemBlob do not currently support.

It is probably wrong that in.buf points to an essentially random MemBlob
outside ConnStateData control but this change does not attempt to fix that.

7 years agoBug 3579: assertion failed 'MemPools[type]' from dst_as ACL
Amos Jeffries [Sat, 18 Jun 2016 11:48:03 +0000 (23:48 +1200)] 
Bug 3579: assertion failed 'MemPools[type]' from dst_as ACL

7 years agoSourceFormat Enforcement
Source Maintenance [Thu, 16 Jun 2016 00:14:16 +0000 (00:14 +0000)] 
SourceFormat Enforcement

7 years agoDo not allow low-level debugging to hide important/critical messages.
Alex Rousskov [Wed, 15 Jun 2016 22:08:16 +0000 (10:08 +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 agoBug 4485: off-by-one out-of-bounds Parser::Tokenizer::int64() read errors
Eduard Bagdasaryan [Tue, 24 May 2016 20:05:03 +0000 (08:05 +1200)] 
Bug 4485: off-by-one out-of-bounds Parser::Tokenizer::int64() read errors

8 years agoIncrease debug level in a peek-and-splice related debug message
Christos Tsantilas [Sat, 21 May 2016 17:29:19 +0000 (05:29 +1200)] 
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 agoFix icons loading speed.
Alex Rousskov [Sat, 21 May 2016 15:52:02 +0000 (03:52 +1200)] 
Fix 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 agoFix OpenSSL detection on FreeBSD
Amos Jeffries [Sat, 21 May 2016 13:01:44 +0000 (01:01 +1200)] 
Fix OpenSSL detection on FreeBSD

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

8 years agoSourceFormat Enforcement
Source Maintenance [Tue, 17 May 2016 18:14:16 +0000 (18:14 +0000)] 
SourceFormat Enforcement

8 years agoSupport unified EUI format code in external_acl_type
Steve Hill [Tue, 17 May 2016 14:58:50 +0000 (02:58 +1200)] 
Support unified EUI format code in external_acl_type

Squid supports %>eui as a logformat specifier, which produces an EUI-48
for IPv4 clients and an EUI-64 for IPv6 clients.  However, This is not
allowed as a format specifier for the external ACLs, and you have to use
%SRCEUI48 and %SRCEUI64 instead.  %SRCEUI48 is only useful for IPv4
clients and %SRCEUI64 is only useful for IPv6 clients, so supporting
both v4 and v6 is a bit messy.

Adds the %>eui specifier for external ACLs and behaves in the same way
as the logformat specifier.

8 years ago3.5.19 SQUID_3_5_19
Amos Jeffries [Sun, 8 May 2016 12:41:25 +0000 (00:41 +1200)] 
3.5.19

8 years agoBug 4515: regression after CVE-2016-4554 patch
Amos Jeffries [Sun, 8 May 2016 12:41:07 +0000 (00:41 +1200)] 
Bug 4515: regression after CVE-2016-4554 patch

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 6 May 2016 12:14:15 +0000 (12:14 +0000)] 
SourceFormat Enforcement

8 years ago3.5.18 SQUID_3_5_18
Amos Jeffries [Fri, 6 May 2016 10:11:49 +0000 (22:11 +1200)] 
3.5.18

8 years agoFix SIGSEGV in ESIContext response handling
Amos Jeffries [Fri, 6 May 2016 09:39:48 +0000 (21:39 +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 agoBug 4455: SegFault from ESIInclude::Start
Amos Jeffries [Fri, 6 May 2016 08:24:29 +0000 (20:24 +1200)] 
Bug 4455: SegFault from ESIInclude::Start

8 years agoPrevent Squid forcing -b 2048 into the arguments for sslcrtd_program
Nathan Hoad [Fri, 6 May 2016 08:15:36 +0000 (20:15 +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 agoBug 4510: Removed stale comment about 32KB limit on shared memory cache entries.
Alex Rousskov [Fri, 6 May 2016 06:36:08 +0000 (18:36 +1200)] 
Bug 4510: Removed stale comment about 32KB limit on shared memory cache entries.

8 years agoBug 4509: EUI compile error on NetBSD
Leonardo Taccari [Fri, 6 May 2016 06:28:50 +0000 (18:28 +1200)] 
Bug 4509: EUI compile error on NetBSD

8 years agoBug 4498: URL-unescape the login-info after extraction from URI
Amos Jeffries [Fri, 6 May 2016 06:25:07 +0000 (18:25 +1200)] 
Bug 4498: URL-unescape the login-info after extraction from URI

8 years agoHandshake Error: ccs received early part2
Christos Tsantilas [Wed, 4 May 2016 08:13:12 +0000 (11:13 +0300)] 
Handshake Error: ccs received early part2

Squid currently does not handle SSL server responses that start with
an SSL Alert Record. Squid fails to parse the Server Hello message and
also fails to detect and handle resuming sessions.

This is a Measurement Factory project.

8 years agoBug 4501: HTTP/1.1: normalize Host header
Amos Jeffries [Mon, 2 May 2016 10:51:18 +0000 (22:51 +1200)] 
Bug 4501: 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 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, and invalid port values.

8 years agoRequire exact match in Host header name lookup
Amos Jeffries [Mon, 2 May 2016 03:39:35 +0000 (15:39 +1200)] 
Require exact match in Host header name lookup

8 years ago3.5.17 SQUID_3_5_17
Amos Jeffries [Wed, 20 Apr 2016 12:21:39 +0000 (00:21 +1200)] 
3.5.17

8 years agoBug 4495: Unknown SSL option SSL_OP_NO_TICKET
Amos Jeffries [Wed, 20 Apr 2016 11:56:48 +0000 (23:56 +1200)] 
Bug 4495: Unknown SSL option SSL_OP_NO_TICKET

8 years agonullptr is a C++11 feature
Amos Jeffries [Wed, 20 Apr 2016 10:14:06 +0000 (22:14 +1200)] 
nullptr is a C++11 feature

8 years agoFix several ESI element construction issues
Amos Jeffries [Wed, 20 Apr 2016 06:36:04 +0000 (18:36 +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 agoSourceFormat Enforcement
Source Maintenance [Wed, 20 Apr 2016 06:14:12 +0000 (06:14 +0000)] 
SourceFormat Enforcement

8 years agocachemgr.cgi: use dynamic MemBuf for internal content generation
Amos Jeffries [Wed, 20 Apr 2016 03:54:04 +0000 (15:54 +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 agoAdd chained certificates and signing certificate to peek-then-bumped connections.
Nathan Hoad [Tue, 19 Apr 2016 15:04:09 +0000 (03:04 +1200)] 
Add chained certificates and signing certificate to peek-then-bumped connections.

The scenario this patch addresses is when Squid is configured with an
intermediate signing CA certificate, and clients have the root CA installed on
their machines. What happens is that the generated certificates come down with
an unknown issuer (the intermediate signing certificate), with no
intermediates, so they are rejected. By adding the configured certificate chain
as old client-first mode did, the intermediate and root certificates come down
as well, resulting in the issuer being identified and the connection being
established "securely".

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

8 years agoHandshake Error: ccs received early: fix typo
Christos Tsantilas [Mon, 18 Apr 2016 15:28:15 +0000 (18:28 +0300)] 
Handshake Error: ccs received early: fix typo

8 years agoAvoid startup/shutdown crashes [by avoiding static non-POD globals].
Alex Rousskov [Sun, 17 Apr 2016 11:19:27 +0000 (23:19 +1200)] 
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 agoBug 4493: theObject->sharedMemorySize() == theSegment.size() exception
Alex Rousskov [Sun, 17 Apr 2016 11:02:41 +0000 (23:02 +1200)] 
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 agoBug 4465: Header forgery detection leads to crash
Alex Rousskov [Sun, 17 Apr 2016 10:54:24 +0000 (22:54 +1200)] 
Bug 4465: Header forgery detection leads to crash

8 years agoHandshake Error: ccs received early
Christos Tsantilas [Sun, 17 Apr 2016 10:43:27 +0000 (22:43 +1200)] 
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 agoBug 4482: Solaris GCC 5.2 warning in src/ip/Intercept.cc
Carsten Grzemba [Tue, 12 Apr 2016 06:52:39 +0000 (18:52 +1200)] 
Bug 4482: Solaris GCC 5.2 warning in src/ip/Intercept.cc

8 years agoBug 4468: NotNode (!acl) naming: Terminate the name before strncat(name).
Alex Rousskov [Tue, 12 Apr 2016 06:04:23 +0000 (18:04 +1200)] 
Bug 4468: NotNode (!acl) naming: Terminate the name before strncat(name).

The fix may reduce or even eliminate garbage in logged ACL names (at
least). The bug was exposed by valgrind's "Conditional jump or move
depends on uninitialised value(s)" error.

8 years agoBug 4483: ./configure garbles -Og option in CFLAGS
Carsten Grzemba [Mon, 11 Apr 2016 13:14:25 +0000 (01:14 +1200)] 
Bug 4483: ./configure garbles -Og option in CFLAGS

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

8 years agoBug 2460 partial: workaround deferred reads on shutdown and restart
Lubos Uhliarik [Mon, 11 Apr 2016 12:41:38 +0000 (00:41 +1200)] 
Bug 2460 partial: workaround deferred reads on shutdown and restart

8 years agoBug 4480: Regression in logformat [.width_max]
Amos Jeffries [Thu, 7 Apr 2016 04:43:45 +0000 (16:43 +1200)] 
Bug 4480: Regression in logformat [.width_max]

8 years ago3.5.16 SQUID_3_5_16
Amos Jeffries [Fri, 1 Apr 2016 21:00:41 +0000 (10:00 +1300)] 
3.5.16

8 years agopinger: drop capabilities on Linux
Yuriy M. Kaminskiy [Fri, 1 Apr 2016 20:38:29 +0000 (09:38 +1300)] 
pinger: drop capabilities on Linux

On linux, it is possible to install pinger helper with only CAP_NET_RAW
raised instead of full setuid-root:

 (setcap cap_net_raw+ep /path/to/pinger && chmod u-s /path/to/pinger) || :

However, pinger only drops setuid/setgid, and won't drop capabilities
after sockets are opened (when it is setuid-root, setuid(getuid()) also
drops capabilities, no code changes necessary; however, if it is only
setcap'ed, setuid() is no-op).

Fix is minimally tested, seems to work fine with both/either `setcap`
and `chmod u+s`; non-linux/non-libcap configurations should not be
affected).

* Also fixes errno debug outputs.

8 years agopinger: Fix select(2) to actually use max_fd
Yuriy M. Kaminskiy [Fri, 1 Apr 2016 06:23:24 +0000 (19:23 +1300)] 
pinger: Fix select(2) to actually use max_fd

8 years agoConvert Vary handling to SBuf
Amos Jeffries [Fri, 1 Apr 2016 06:15:31 +0000 (19:15 +1300)] 
Convert Vary handling to SBuf