]> git.ipfire.org Git - thirdparty/squid.git/log
thirdparty/squid.git
7 years agoHappy Eyeballs: Deliver DNS resolution results to peer selection ASAP.
Alex Rousskov [Wed, 12 Jul 2017 05:04:41 +0000 (23:04 -0600)] 
Happy Eyeballs: Deliver DNS resolution results to peer selection ASAP.

To make eyeballs happy, DNS code must deliver each lookup result to the
IP cache and, ultimately, to upper layers of ipcache_nbgethostbyname()
callers. This requires changing two interfaces:

1. between the DNS and the IP cache (the IDNSCB callback);
2. between the IP cache and peer selection code (the IPH callback).

The IDNSCB callback is now called after every usable A and AAAA lookup
instead of waiting for both answers. The IPH callback now has a sister
API for incremental delivery: The Dns::IpReceiver class.

To safely handle incremental delivery of IP addresses to the IP cache, I
upgraded ipcache_addrs from an equivalent of a C POD to a C++ CachedIps
container. The encapsulation allowed me to clearly separate the two IP
cache iteration APIs:

* All IPs (used by, e.g., ACL matching and host verification code) and
* just the "good" IPs (used only for peer selection for now).

CachedIps stores IPs together with their good/bad status in a single
std::vector. Eventually, the CachedIp element may be extended to store
TTL. The following implementation alternatives were considered and
rejected (at least for now) while optimizing for the "a few (and usually
just one), usually good IPs" case:

* Using std::list or std::deque storage would consume more RAM[1] for
  the common case of one (or few) IPs per name and slowed down IPs
  iteration code.
  [1] http://info.prelert.com/blog/stl-container-memory-usage
* Separating IP from its status, like the old code did, would make it
  easier to mismatch IP and its status, make it harder to add more
  metadata like per-IP TTL, and only save memory when storing many IPs
  per name.

The drawback of the selected "all IP-related info in one place" approach
is that we need smart iterators (e.g., the added GoodIpsIterator) or a
visitor API.

I added a new interface class for the incremental notification about
newly found IP addresses (Dns::IpReceiver) instead of adding second
IPH-like function pointer because we cannot safely call cbdata-protected
functions multiple times for the same cbdata object -- only
cbdataReferenceValidDone() dereferences the opaque pointer properly, and
that function cannot be called repeatedly. CbcPointer solves that
problem (but requires a class). Class methods also allow for more
precise notifications, with fewer ifs in the recipient code.

The new IpCacheLookupForwarder class hides the differences between the
old C-style IPH callbacks and the new Dns::IpReceiver. Eventually, we
may be able to move all lookup-specific data/methods into
IpCacheLookupForwarder, away from the IP cache entries where that info
is useless at best.

mgr:ipcache no longer reports "IPcache Entries In Use" but that info is
now available as "cbdata ipcache_entry" row in mgr:mem.

Do not cache IPv6 /etc/hosts addresses when IPv6 support is disabled.
This change simplified code, made it more consistent (we did not cache
AAAA records), and fixed ipcacheCycleAddr() and ipcacheMarkAllGood()
that were clearing supposed-to-be-permanent "bad (IPv6 disabled)" marks.

Also fixed two DNS TTL bugs. Squid now uses minimum TTL among all used
DNS records[2]. Old ipcacheParse() was trying to do the same but:
* could overwrite a zero TTL with a positive value
* took into account TTLs from unused record types (e.g., CNAME).
[2] Subject to *_dns_ttl limits in squid.conf, as before.

Also fixed "delete xstrdup" (i.e., malloc()ed) pointer in bracketed IP
parsing code (now moved to Ip::Address::FromHost()).

Also prohibited duplicate addresses from entering the IP cache. Allowing
duplicates may be useful for various hacks, but the IP cache code
assumes that cached IPs are unique and fails to mark bad repeated IPs.

Also fixed sending Squid Announcements to unsupported/disabled IPv6
addresses discovered via /etc/hosts.

Also slightly optimized dstdomain when dealing with IP-based host names:
The code now skips unnecessary Ip::Address to ipcache_addrs conversion.
This simplification may also help remove the ipcacheCheckNumeric() hack.
The bracketed IP parsing code was moved to Ip::Address::fromHost(). It
still needs a lot of love.

7 years agoTLS: move X509_NAME_STACK_Pointer to Security::ServerOptions
Amos Jeffries [Mon, 10 Jul 2017 15:11:52 +0000 (03:11 +1200)] 
TLS: move X509_NAME_STACK_Pointer to Security::ServerOptions

This Pointer is only used by the Server port options.

No GnuTLS support added in this patch, just a straight shuffle
of the OpenSSL code.

7 years agoBug 1961 extra: Convert the URL::parse method API to take const URI strings
Amos Jeffries [Sun, 9 Jul 2017 08:16:33 +0000 (20:16 +1200)] 
Bug 1961 extra: Convert the URL::parse method API to take const URI strings

The input buffer is no longer truncated when overly long. All callers have
been checked that they handle the bool false return value in ways that do
not rely on that truncation.

Callers that were making non-const copies of buffers specifically for the
parsing stage are altered not to do so. This allows a few data copies and
allocations to be removed entirely, or delayed to remove from error handling
paths.

While checking all the callers of Http::FromUrl several places were found to
be using the "raw" URL string before the parsing and validation was done. The
simplest in src/mime.cc is already applied to v5 r15234. A more complicated
redesign in src/store_digest.cc is included here for review. One other marked
with an "XXX: polluting ..." note.

Also, added several TODO's to mark code where class URL needs to be used when
the parser is a bit more efficient.

Also, removed a leftover definition of already removed urlParse() function.

7 years agoFix ignored-qualifiers warning in class String
Amos Jeffries [Sat, 8 Jul 2017 19:28:42 +0000 (07:28 +1200)] 
Fix ignored-qualifiers warning in class String

7 years agoCleanup: update InstanceId to C++11
Amos Jeffries [Sat, 8 Jul 2017 19:14:49 +0000 (07:14 +1200)] 
Cleanup: update InstanceId to C++11

... fixing some ignored-qualifiers warnings

7 years agoCleanup: remove many unused-parameter warnings from tests and stubs
Amos Jeffries [Sat, 8 Jul 2017 18:20:41 +0000 (06:20 +1200)] 
Cleanup: remove many unused-parameter warnings from tests and stubs

7 years agoValidate mime icon URL before allocating store entries
Amos Jeffries [Sat, 8 Jul 2017 10:04:48 +0000 (22:04 +1200)] 
Validate mime icon URL before allocating store entries

7 years agoRemove unnecessary assert in NetDB
Amos Jeffries [Sat, 8 Jul 2017 10:00:57 +0000 (22:00 +1200)] 
Remove unnecessary assert in NetDB

7 years agoIgnore HTCP packets with invalid URI
Amos Jeffries [Sat, 8 Jul 2017 09:58:09 +0000 (21:58 +1200)] 
Ignore HTCP packets with invalid URI

8 years agoFix typo in SPONSORS listing
Amos Jeffries [Thu, 6 Jul 2017 04:20:10 +0000 (16:20 +1200)] 
Fix typo in SPONSORS listing

The website is auto-generated from this file. It needs to have accurate whitespace.

8 years agoPrep for 4.0.21
Amos Jeffries [Sun, 2 Jul 2017 08:06:33 +0000 (20:06 +1200)] 
Prep for 4.0.21

8 years agoext_file_userip_acl: merge 'deny all' example into example.conf file
Amos Jeffries [Fri, 30 Jun 2017 05:47:19 +0000 (17:47 +1200)] 
ext_file_userip_acl: merge 'deny all' example into example.conf file

8 years agoAdd missing copyrght blurb on apparmor profile
Amos Jeffries [Thu, 29 Jun 2017 13:07:10 +0000 (01:07 +1200)] 
Add missing copyrght blurb on apparmor profile

8 years agoSourceFormat Enforcement
Source Maintenance [Mon, 26 Jun 2017 18:12:14 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoReduce "!Comm::MonitorsRead(serverConnection->fd)" assertions.
Christos Tsantilas [Mon, 26 Jun 2017 14:34:57 +0000 (17:34 +0300)] 
Reduce "!Comm::MonitorsRead(serverConnection->fd)" assertions.

* Protect Squid Client classes from new requests that compete with
  ongoing pinned connection use and
* resume dealing with new requests when those Client classes are done
  using the pinned connection.

Replaced primary ConnStateData::pinConnection() calls with a pair of
pinBusyConnection() and notePinnedConnectionBecameIdle() calls,
depending on the pinned connection state ("busy" or "idle").

Removed pinConnection() parameters that were not longer used or could be computed from the remaining parameters.

Removed ConnStateData::httpsPeeked() code "hiding" the originating
request and connection peer details while entering the first "idle"
state. The old (trunk r11880.1.6) bump-server-first code used a pair of
NULLs because "Intercepted connections do not have requests at the
connection pinning stage", but that limitation no longer applicable
because Squid always fakes (when intercepting) or parses (a CONNECT)
request now, even during SslBump step1.

The added XXX and TODOs are not directly related to this fix. They
were added to document problems discovered while working on this fix.

In v3.5 code, the same problems manifest as Read.cc
"fd_table[conn->fd].halfClosedReader != NULL" assertions.

This is a Measurement Factory project

8 years agoBug 1961 partial: move urlParse() into class URL
Amos Jeffries [Mon, 26 Jun 2017 02:14:42 +0000 (14:14 +1200)] 
Bug 1961 partial: move urlParse() into class URL

* rename local variables in urlParse() to avoid symbol
  clashes with class URL members and methods.

* move HttpRequest method assignment out to the single caller
  which actually needed it. Others all passed in the method
  which was already set on the HttpRequest object passed.

* removed now needless HttpRequest parameter of urlParse()

* rename urlParse as a class URL method

* make URL::parseFinish() private

* remove unnecessary CONNECT_PORT define

* add RFC documentation for 'CONNECT' URI handling

* fixed two XXX in URL-rewrite handling doing unnecessary
  HttpRequest object creation and destruction cycles on
  invalid URL-rewrite helper output.

8 years agoAdd copyright blurb to Solaris krb5 patch
Amos Jeffries [Mon, 26 Jun 2017 02:09:53 +0000 (14:09 +1200)] 
Add copyright blurb to Solaris krb5 patch

8 years agoAdd copyright boilerplate to TRANSLATORS file
Amos Jeffries [Mon, 26 Jun 2017 02:04:53 +0000 (14:04 +1200)] 
Add copyright boilerplate to TRANSLATORS file

8 years agoAdd copyright for 4.0.18 squid.conf regressions test
Amos Jeffries [Mon, 26 Jun 2017 01:56:18 +0000 (13:56 +1200)] 
Add copyright for 4.0.18 squid.conf regressions test

8 years agoMinimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED.
Alex Rousskov [Mon, 26 Jun 2017 00:10:34 +0000 (18:10 -0600)] 
Minimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED.

No functionality changes expected.

Added allow_t API to avoid direct comparisons with ACCESS_ALLOWED and
ACCESS_DENIED. Developers using direct comparisons eventually mishandle
exceptional ACCESS_DUNNO and ACCESS_AUTH_REQUIRED cases where neither
"allow" nor "deny" rule matched. The new API cannot fully prevent such
bugs, but should either led the developer to the right choice (usually
.allowed()) or alert the reviewer about an unusual choice (i.e.,
denied()).

The vast majority of checks use allowed(), but we could not eliminate
the remaining denied() cases ("miss_access" and "cache" directives) for
backward compatibility reasons -- previously "working" deployments may
suddenly start blocking cache misses and/or stop caching:
http://lists.squid-cache.org/pipermail/squid-dev/2017-May/008576.html

8 years agoBug 1961 partial: cleanup urlParseFinish()
Amos Jeffries [Sun, 25 Jun 2017 23:31:15 +0000 (11:31 +1200)] 
Bug 1961 partial: cleanup urlParseFinish()

Move the urlParseFinish() logic into a class URL method and remove its
dependency on HttpRequest objects.

Remove unnecessary urnParse() function.

8 years agoFix typo in rev.15039
Emmanuel Fuste [Sun, 25 Jun 2017 20:48:29 +0000 (08:48 +1200)] 
Fix typo in rev.15039

8 years agoFix --with-openssl --without-ssl-crtd build errors
Amos Jeffries [Sun, 25 Jun 2017 19:57:42 +0000 (07:57 +1200)] 
Fix --with-openssl --without-ssl-crtd build errors

The validation and cert generation helpers are independent.

8 years agoSourceFormat Enforcement
Source Maintenance [Sat, 24 Jun 2017 00:12:20 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoFixed mgr query handoff from the original recipient to Coordinator.
Alex Rousskov [Sat, 24 Jun 2017 00:01:51 +0000 (18:01 -0600)] 
Fixed mgr query handoff from the original recipient to Coordinator.

This bug has already been fixed once, in trunk r11164.1.61, but that fix
was accidentally undone shortly after, during significant cross-branch
merging activity combined with the Forwarder class split. The final
merge importing the associated code (trunk r11730) was buggy.

The bug (explained in r11164.1.61) leads to a race condition between

* Store notifying Server classes about the entry completion (which might
  trigger a bogus error message sent to the cache manager client while
  Coordinator sends its own valid response on the same connection!) and

* post-cleanup() connection closure handlers of Server classes silently
  closing everything (and leaving Coordinator the only responding
  process on that shared connection).

The bug probably was not noticed for so long because, evidently, the
latter actions tend to win in the current code.

8 years agoFixed message packing error handling in mgr and snmp SMP Forwarders.
Alex Rousskov [Fri, 23 Jun 2017 22:15:30 +0000 (16:15 -0600)] 
Fixed message packing error handling in mgr and snmp SMP Forwarders.

A missing "return" resulted in Forwarders sending garbage (or worse) to
Coordinator.

8 years agoCleanup: remove unused members and functions in stateful helper
Amos Jeffries [Tue, 20 Jun 2017 22:50:03 +0000 (10:50 +1200)] 
Cleanup: remove unused members and functions in stateful helper

8 years agobasic_ncsa_auth: fix hash listing wrap in man(8) page
Amos Jeffries [Tue, 20 Jun 2017 11:02:27 +0000 (23:02 +1200)] 
basic_ncsa_auth: fix hash listing wrap in man(8) page

'*' list bullet points must be indented with whitespace. If they are not the list is treated as a single wrapped paragraph.

8 years agoFix build errors with automake after rev.15194
Amos Jeffries [Mon, 19 Jun 2017 13:53:03 +0000 (01:53 +1200)] 
Fix build errors with automake after rev.15194

We cannot name files in src/ the same as files in sub-directories due to
certain automake still implicitly using -I. for search paths.

8 years agobug 4730: Squid may segfault while processes internal HTTP requests
Christos Tsantilas [Mon, 19 Jun 2017 10:19:28 +0000 (13:19 +0300)] 
bug 4730: Squid may segfault while processes internal HTTP requests

Squid segfaults when trying to access ClientHttpRequest::getConn() object for
HTTP requests without client connection (internal requests).

This is a Measurement Factory project

8 years agoPolishing touches meant to be included in r15206.
Alex Rousskov [Mon, 19 Jun 2017 00:33:39 +0000 (18:33 -0600)] 
Polishing touches meant to be included in r15206.

8 years agoSourceFormat Enforcement
Source Maintenance [Sun, 18 Jun 2017 18:12:07 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoAdd a basic apparmour profile
Simon Deziel [Sun, 18 Jun 2017 17:49:10 +0000 (05:49 +1200)] 
Add a basic apparmour profile

From Ubuntu, with some non-squid software references removed

8 years agoCleanup: remove many whitespace typos
Amos Jeffries [Sun, 18 Jun 2017 15:17:48 +0000 (03:17 +1200)] 
Cleanup: remove many whitespace typos

... caught by astyle 2.06 but ignored by 2.04

8 years agoFix option --foreground to implement expected behavior
Andreas Weigel [Sun, 18 Jun 2017 14:26:55 +0000 (02:26 +1200)] 
Fix option --foreground to implement expected behavior

... and allow usage of SMP mode with service supervisors that do not work
well with daemons.

Currently, --foreground behavior is counter-intuitive in that the launched
process, while staying in the foreground, forks another "master" process,
which will create additional children (kids), depending on the number of
configured workers/diskers.

Furthermore, sending a SIGINT/SIGTERM signal to this foreground process
terminates it, but leaves all the children running.

The behavior got introduced with v4 rev.14561.

From discussion on squid-dev, the following behavior is implemented:

* -N: The initial process is a master and a worker process.
  No kids.
  No daemonimization.

* --foreground: The initial process is the master process.
  One or more worker kids (depending on workers=N).
  No daemonimization.

* neither: The initial process double-forks the master process.
  One or more worker kids (depending on workers=N).
  Daemonimization.

The Release Notes for v4 were updated to reflect the corrected behavior.

8 years agoFix missing semicolon in rev.15203
Amos Jeffries [Sat, 17 Jun 2017 07:51:42 +0000 (19:51 +1200)] 
Fix missing semicolon in rev.15203

8 years agoSourceFormat Enforcement
Source Maintenance [Sat, 17 Jun 2017 00:12:06 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoCleanup: use EXIT_FAILURE and EXIT_SUCCESS instead of magic numbers in exit()
Amos Jeffries [Fri, 16 Jun 2017 18:38:19 +0000 (06:38 +1200)] 
Cleanup: use EXIT_FAILURE and EXIT_SUCCESS instead of magic numbers in exit()

... and return EXIT_SUCCESS from main() functions.

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 16 Jun 2017 18:12:15 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agotproxy does not spoof spliced connections
Christos Tsantilas [Fri, 16 Jun 2017 13:38:16 +0000 (16:38 +0300)] 
tproxy does not spoof spliced connections

Squid does not spoof client IP addresses when splicing after peeking/staring.

This is a Measurement Factory project

8 years agoBug 4112: ssl_engine does not accept cryptodev
Sven Eisenberg [Fri, 16 Jun 2017 12:15:30 +0000 (00:15 +1200)] 
Bug 4112: ssl_engine does not accept cryptodev

8 years agoCleanup: remove Vector template from adaptation headers
Amos Jeffries [Fri, 16 Jun 2017 12:04:22 +0000 (00:04 +1200)] 
Cleanup: remove Vector template from adaptation headers

class Vector no longer exists in Squid, no need to predefine it.

8 years agoFixed reusableReply() logic broken by v5 r15168.
Eduard Bagdasaryan [Tue, 13 Jun 2017 23:02:04 +0000 (17:02 -0600)] 
Fixed reusableReply() logic broken by v5 r15168.

8 years agoAdd missing includes after rev.15194
Amos Jeffries [Tue, 13 Jun 2017 08:33:33 +0000 (20:33 +1200)] 
Add missing includes after rev.15194

8 years agotransaction_initiator ACL for detecting various unusual transactions part2
Christos Tsantilas [Tue, 13 Jun 2017 06:46:58 +0000 (09:46 +0300)] 
transaction_initiator ACL for detecting various unusual transactions part2

Fix  "make distcheck" issues after r15194.

8 years agoSourceFormat Enforcement
Source Maintenance [Tue, 13 Jun 2017 00:12:19 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agotransaction_initiator ACL for detecting various unusual transactions
Christos Tsantilas [Mon, 12 Jun 2017 20:26:41 +0000 (23:26 +0300)] 
transaction_initiator ACL for detecting various unusual transactions

This ACL is essential in several use cases, including:

* After fetching a missing intermediate certificate, Squid uses the
  regular cache (and regular caching rules) to store the response. Squid
  deployments that do not want to cache regular traffic need to cache
  fetched certificates and only them.

  acl fetched_certificate transaction_initiator certificate-fetching
  cache allow fetched_certificate
  cache deny all

* Many traffic policies and tools assume the existence of an HTTP client
  behind every transaction. Internal Squid requests violate that
  assumption. Identifying internal requests protects external ACLs, log
  analyzers, and other mechanisms from the transactions they mishandle.

  acl skip_logging transaction_initiator internal
  access_log ... !skip_logging

The new transaction_initiator ACL classifies transactions based on their
initiator. Currently supported initiators are esi, certificate-fetching,
cache-digest, internal, client, and all. In the future, the same ACL
will be able to identify HTTP/2 push transactions using the "server"
initiator. See src/cf.data.pre for details.

This is a Measurement Factory project.

8 years agoRedesign urlParse API part2: fix compile errors after r15191
Christos Tsantilas [Mon, 12 Jun 2017 19:28:29 +0000 (22:28 +0300)] 
Redesign urlParse API part2: fix compile errors after r15191

8 years agoSourceFormat Enforcement
Source Maintenance [Mon, 12 Jun 2017 18:12:15 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoRedesign urlParse API
Christos Tsantilas [Mon, 12 Jun 2017 17:54:02 +0000 (20:54 +0300)] 
Redesign urlParse API

The urlParse modified to require an HttpRequest object as parameter and return
boolean value to indicate URL parsing status.

This is a Measurement Factory project

8 years agoCollapse security_file_certgen requests.
Christos Tsantilas [Mon, 12 Jun 2017 16:05:59 +0000 (19:05 +0300)] 
Collapse security_file_certgen requests.

Concurrent identical same-worker security_file_certgen (a.k.a. ssl_crtd)
requests are collapsed: The first such request goes through to one of
the helpers while others wait for that first request to complete,
successfully or otherwise. This optimization helps dealing with flash
crowds that suddenly send a large number of HTTPS requests to a small
group of origin servers.

Two certificate generation requests are considered identical if their
on-the-wire images are identical. This simple and fast approach covers
all certificate generation parameters, including all mimicked
certificate properties, and avoids hash collisions and poisoning.
Compared to collision- or poisoning-sensitive approaches that store raw
certificates and compare their signatures or fingerprints, storing
helper queries costs a few extra KB per pending helper request. That
extra RAM cost is worth the advantages and will be eliminated when
helper code switches from c-strings to SBufs.

This is a Measurement Factory project.

8 years agossl::server_name options to control matching logic.
Christos Tsantilas [Mon, 12 Jun 2017 15:40:16 +0000 (18:40 +0300)] 
ssl::server_name options to control matching logic.

Many popular servers use certificates with several "alternative subject
names" (SubjectAltName). Many of those names are wildcards. For example,
a www.youtube.com certificate currently includes *.google.com and 50+
other subject names, most of which are wildcards.

Often, admins want server_name to match any of the subject names. This
is useful to match any server belonging to a large conglomerate of
companies, all including some *.example.com name in their certificates.
The existing server_name functionality addresses this use case well.

The new ACL options address several other important use cases:

--consensus identifies transactions with a particular server when
  server's subject name is also present in certificates used by many other
  servers (e.g., matching transactions with a particular Google server but
  not with all Youtube servers).

--client-requested allows both (a) SNI-based matching even after
  Squid obtains the server certificate and (b) pinpointing a particular
  server in a group of different servers all using the same wildcard
  certificate (e.g., matching appengine.example.com but not
  www.example.com when the certificate for has *.example.com subject).

--server-provided allows matching only after Squid obtains the server
  certificate and matches any of the conglomerate parts.

Also this patch fixes squid to log client SNI when client-first bumping mode
is used too.

This is a Measurement Factory project.

8 years agoSupport for --long-acl-options
Christos Tsantilas [Mon, 12 Jun 2017 14:48:28 +0000 (17:48 +0300)] 
Support for --long-acl-options

The old single-letter ACL "flags" code was refactored to support long option
names (with option-specific value types) without significant
per-ACL-object performance/RAM overheads and without creating a global
registry for all possible options. This refactoring (unexpectedly)
resulted in removal of a lot of unreliable static initialization code.

Refactoring fixed ACL flags parsing code that was dangerously misinterpreting
-i and +i flags in several contexts. For example, each of the three cases
below was misinterpreted as if three domains were configured (e.g., "+i",
"-z", and "example.com") on each line instead of one domain ("example.com"):

        acl parsedAsThreeDomains dstdomain +i -i .example.com
        acl parsedAsThreeDomains dstdomain -i +i .example.com
        acl parsedAsThreeDomains dstdomain +i -z .example.com

TODO: Finish ACL::clone() removal. Then consider removing any unused
ACLData::clone() and associated ACL/ACLData copying methods.

This is a Measurement Factory project.

8 years agoMake GCC v6 happier after r15183. GCC thinks "if (x)" does not use x.
Alex Rousskov [Fri, 9 Jun 2017 18:19:00 +0000 (12:19 -0600)] 
Make GCC v6 happier after r15183. GCC thinks "if (x)" does not use x.

Same as r15186 but applied to tunnel.cc now. I hate code duplication.

8 years agoMake GCC v6 happier after r15183. GCC thinks "if (x)" does not use x.
Alex Rousskov [Fri, 9 Jun 2017 16:59:36 +0000 (10:59 -0600)] 
Make GCC v6 happier after r15183. GCC thinks "if (x)" does not use x.

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 9 Jun 2017 06:12:19 +0000 (06:12 +0000)] 
SourceFormat Enforcement

8 years agoAttempting to fix Jenkins "make test" (with clang?) after r15183.
Alex Rousskov [Fri, 9 Jun 2017 06:03:47 +0000 (00:03 -0600)] 
Attempting to fix Jenkins "make test" (with clang?) after r15183.

undefined reference: operator<<(std::ostream&, Comm::Connection const&)
make[5]: *** [tests/testACLMaxUserIP] Error 1

My test-builds.sh passed before (and still passes after) this change, so
I am just guessing that I picked the wrong stub_*comm.cc file in r15183.

8 years agoHappy Eyeballs: Use each fully resolved forwarding destination ASAP.
Alex Rousskov [Fri, 9 Jun 2017 04:38:40 +0000 (22:38 -0600)] 
Happy Eyeballs: Use each fully resolved forwarding destination ASAP.

Do not wait for all request forwarding destinations to be resolved. Use
each resolved destination as soon as it is needed. This change does not
improve or affect IPv4/IPv6 handling (Squid still waits for both DNS A
and AAAA answers when resolving a single destination name), but the peer
selection code can now deliver each IP address to the FwdState/tunneling
code without waiting for all other destination names to be DNS-resolved.
TODO: Deliver A and AAAA answers to the peer selection code ASAP.

This change speeds up forwarding in peering environments where peers may
need frequent DNS resolutions but that was not the motivation here.

This change is a step towards a more complete Happy Eyeballs support in
Squid. The general path can be roughly summarized as follows:

1. Squid has already supported: Use parallel A and AAAA queries.
2. This change: ASAP delivery of IPs from peer selection to FwdState.
3. The next step: ASAP delivery of IPs from DNS to peer selection.
4. A separate project may add: Use parallel TCP connections.

Also fixed missing cbdataReferenceDone(psstate->callback_data) calls
in three error handling cases. These cases probably leaked memory.

8 years agoFix missing return after rev.15176
Amos Jeffries [Fri, 9 Jun 2017 03:48:10 +0000 (15:48 +1200)] 
Fix missing return after rev.15176

 Detected by Coverity Scan. Issue 1412064

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 9 Jun 2017 00:12:17 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoClarified v5 r15168 code logic. No functionality changes.
Alex Rousskov [Thu, 8 Jun 2017 18:09:28 +0000 (12:09 -0600)] 
Clarified v5 r15168 code logic. No functionality changes.

Detected by Coverity Scan. Issue 1412065 (MISSING_BREAK).

8 years agoSourceFormat Enforcement
Source Maintenance [Wed, 7 Jun 2017 18:12:14 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoFix build on GCC-4 after rev.15177
Amos Jeffries [Wed, 7 Jun 2017 18:04:08 +0000 (06:04 +1200)] 
Fix build on GCC-4 after rev.15177

8 years agoTLS: Move the remaining SSL_SESSION cache callbacks to security/Session.*.
Amos Jeffries [Wed, 7 Jun 2017 15:57:21 +0000 (03:57 +1200)] 
TLS: Move the remaining SSL_SESSION cache callbacks to security/Session.*.

No GnuTLS additions here, or significant code changes. Most of this is a
straight cut-n-paste. Though it does make the slot lookup to auto in the
if-condition to simplify the callback code and removes some no longer
necessary comments as requested in audit.

8 years agoImprove config parsing of logformat definitions
Amos Jeffries [Wed, 7 Jun 2017 12:20:56 +0000 (00:20 +1200)] 
Improve config parsing of logformat definitions

Squid has for some time ignored custom definitions using the same name
as internally defined formats. And overwritten custom formats when there
was a repeated definition.

* Detect logformat duplicates and produce ERROR message indicating the
  format name, config line and action taken.

* Add some missing FATAL labels on parse abort when access_log has
  multiple logformat= options configured.

* Add missing FATAL error message when logformat lines has no name
  parameter (and thus no tokens either).

* Omit the default "logformat=squid" option from cachemgr config dumps.

8 years agoAdd missing ERROR label on cache_peer_access misconfiguration messages
Amos Jeffries [Tue, 6 Jun 2017 13:36:49 +0000 (01:36 +1200)] 
Add missing ERROR label on cache_peer_access misconfiguration messages

8 years agoSourceFormat Enforcement
Source Maintenance [Mon, 5 Jun 2017 00:12:15 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoBug 4492: Chunk extension parser is too pedantic.
Alex Rousskov [Sun, 4 Jun 2017 23:45:03 +0000 (17:45 -0600)] 
Bug 4492: Chunk extension parser is too pedantic.

Support HTTP/1 BWS when parsing HTTP and ICAP chunk extensions.
Per RFC 7230 Errata #4667, HTTP parsers MUST parse BWS in chunk-ext.
Per RFC 3507 and its extensions, ICAP agents generate BWS in chunk-ext.

Also discovered that our DelimiterCharacters() in pedantic mode is too
strict for many use cases: Most HTTP syntax rules allow both SP and HTAB
but pedantic DelimiterCharacters() only allows SP. Added
WhitespaceCharacters() to provide the more general set where it is
needed in new code (including BWS), but did not remove excessive
DelimiterCharacters() use.

8 years agoSourceFormat Enforcement
Source Maintenance [Sun, 4 Jun 2017 18:12:06 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoDo not die silently when dying via std::terminate().
Alex Rousskov [Sun, 4 Jun 2017 15:32:01 +0000 (09:32 -0600)] 
Do not die silently when dying via std::terminate().

Report exception failures that call std::terminate(). Exceptions unwind
stack towards main() and sooner or later get handled/reported by Squid.
However, exception _failures_ just call std::terminate(), which aborts
Squid without the stack unwinding. By default, a std::terminate() call
usually results in a silent Squid process death because some default
std::terminate_handler implementations do not say anything at all while
others write to stderr which Squid redirects to /dev/null by default.

Many different problems trigger std::terminate() calls. Most of them are
rare, but, after the C++11 migration, one category became likely in
Squid: A throwing destructor. Destructors in C++11 are implicitly
"noexcept" by default, and many old Squid destructors might throw.

These reporting changes do not bypass or eliminate any failures.

8 years agoSourceFormat Enforcement
Source Maintenance [Fri, 2 Jun 2017 00:12:13 +0000 (00:12 +0000)] 
SourceFormat Enforcement

8 years agoDo not respond with HTTP/304 to unconditional requests
Eduard Bagdasaryan [Thu, 1 Jun 2017 23:49:09 +0000 (17:49 -0600)] 
Do not respond with HTTP/304 to unconditional requests

... after internal revalidation. The original unconditional HttpRequest
was still marked (and processed) as conditional after internal
revalidation because the original (clear) Last-Modified and ETag values
were not restored (cleared) after the internal revalidation abused them.

TODO: Isolate the code converting the request into conditional one _and_
the code that undoes that conversion, to keep both actions in sync.

8 years agoCollapse internal revalidation requests (SMP-unaware caches), again.
Eduard Bagdasaryan [Thu, 1 Jun 2017 23:34:40 +0000 (17:34 -0600)] 
Collapse internal revalidation requests (SMP-unaware caches), again.

The security fix in v5 r14979 had a negative effect on collapsed
forwarding. All "private" entries were considered automatically
non-shareable among collapsed clients. However this is not true: there
are many situations when collapsed forwarding should work despite of
"private" entry status: 304/5xx responses are good examples of that.
This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
flag.

The suggested fix is not complete: To cover all possible situations, we
need to decide whether StoreEntry::shareableWhenPrivate is true or not
for all contexts where StoreEntry::setPrivateKey() is used. This patch
fixes only few important cases inside http.cc, making CF (as well
collapsed revalidation) work for some [non-cacheable] response status
codes, including 3xx, 5xx and some others.

The original support for internal revalidation requests collapsing
was in trink r14755 and referred to Squid bugs 2833, 4311, and 4471.

8 years agoPrep for 4.0.20 and 3.5.26
Amos Jeffries [Thu, 1 Jun 2017 12:10:54 +0000 (00:10 +1200)] 
Prep for 4.0.20 and 3.5.26

8 years agoCleanup: remove some unnecessary wrapper code in libauth.la
Amos Jeffries [Wed, 31 May 2017 03:03:31 +0000 (15:03 +1200)] 
Cleanup: remove some unnecessary wrapper code in libauth.la

Resolves two outstanding "TODO: remove wrapper" items.

* authenticateAddTrailer was completely unused.

* authenticateFixHeader was used but needless.

Also, makes the addReplyAuthHeader static start with upper-case
to comply with Squid coding guidelines.

8 years agoBug 4687: Wrong names of components in man page, section SEE ALSO
Luboš Uhliarik [Mon, 29 May 2017 15:50:02 +0000 (03:50 +1200)] 
Bug 4687: Wrong names of components in man page, section SEE ALSO

8 years agoTranslations: update .po and .pot to latest texts
Amos Jeffries [Mon, 29 May 2017 11:02:04 +0000 (23:02 +1200)] 
Translations: update .po and .pot to latest texts

Also, update the updater script for new helper locations after squid-4-14516

And fix one typo in the purge.8 manual.

8 years agoFix X509_up_ref build errors after r15160
Amos Jeffries [Mon, 29 May 2017 09:41:26 +0000 (21:41 +1200)] 
Fix X509_up_ref build errors after r15160

8 years agoTLS: recognise tls:: namespace on logformat tokens
Amos Jeffries [Mon, 29 May 2017 09:09:54 +0000 (21:09 +1200)] 
TLS: recognise tls:: namespace on logformat tokens

For forward-compatibility with the codes when they switch to TLS naming.
Leaving undocumented until that change actually happens.

8 years agoAdd OpenSSL library details to -v output
Amos Jeffries [Mon, 29 May 2017 00:06:55 +0000 (12:06 +1200)] 
Add OpenSSL library details to -v output

This is partially to meet the OpenSSL copyright requirement that binaries
mention when they are using the library, and partially for admin to see
which library their Squid is using when multiple are present in the system.

8 years agoBug 4662 pt4: better *_up_ref() detection
Amos Jeffries [Mon, 29 May 2017 00:03:34 +0000 (12:03 +1200)] 
Bug 4662 pt4: better *_up_ref() detection

Also, removes duplicate X509_up_ref definition in ssl/support.h

Also, updates LIBS for feature detect with custom library locations

8 years agoBug 4662 pt3: feature detect OpenSSL 1.1 const changes
Amos Jeffries [Sat, 27 May 2017 05:24:31 +0000 (17:24 +1200)] 
Bug 4662 pt3: feature detect OpenSSL 1.1 const changes

8 years agoBug 4662 pt2: feature detect OpenSSL 1.1 API functions
Amos Jeffries [Sat, 27 May 2017 01:03:47 +0000 (13:03 +1200)] 
Bug 4662 pt2: feature detect OpenSSL 1.1 API functions

8 years agoBug 4662: pt1: Feature detect the OpenSSL TLS_method() and similar functions
Amos Jeffries [Fri, 26 May 2017 20:55:48 +0000 (08:55 +1200)] 
Bug 4662: pt1: Feature detect the OpenSSL TLS_method() and similar functions

8 years agoSourceFormat Enforcement
Source Maintenance [Thu, 25 May 2017 18:12:07 +0000 (18:12 +0000)] 
SourceFormat Enforcement

8 years agoFixed Windows-specific code in r15148. Polished r15148 code.
Alex Rousskov [Thu, 25 May 2017 15:35:25 +0000 (09:35 -0600)] 
Fixed Windows-specific code in r15148. Polished r15148 code.

The Windows-specific part of File::synchronize() missed a curly brace.
Besides breaking compilation on Windows, it broke non-Windows code
formatting (--ignore-all-space is advised when looking at this commit).

Also polished r15148 code without changing its functionality. These
polishing touches were meant to be done during r15148 commit.

8 years agoSourceFormat Enforcement
Source Maintenance [Thu, 25 May 2017 06:12:04 +0000 (06:12 +0000)] 
SourceFormat Enforcement

8 years agoBug 4722: eCAP compile errors after v5 r15147
Amos Jeffries [Thu, 25 May 2017 05:27:08 +0000 (17:27 +1200)] 
Bug 4722: eCAP compile errors after v5 r15147

8 years agoSquid crashes when server-first bumping mode is used with openSSL-1.1.0 release
Christos Tsantilas [Wed, 24 May 2017 09:02:40 +0000 (12:02 +0300)] 
Squid crashes when server-first bumping mode is used with  openSSL-1.1.0 release

When OpenSSL-1.1.0 or later is used:
  - The SQUID_USE_SSLGETCERTIFICATE_HACK configure test is false
  - The SQUID_SSLGETCERTIFICATE_BUGGY configure test is true
  - Squid hits an assert(0) inside Ssl::verifySslCertificate when trying to
    retrieve a generated certificate from cache.

This is a Measurement Factory project

8 years agoSourceFormat Enforcement
Source Maintenance [Tue, 23 May 2017 12:12:15 +0000 (12:12 +0000)] 
SourceFormat Enforcement

8 years agoUse Http::ProtocolVersion in config parsing
Amos Jeffries [Tue, 23 May 2017 10:35:48 +0000 (22:35 +1200)] 
Use Http::ProtocolVersion in config parsing

8 years agoCreate PID file ASAP, before the shared memory segments.
Eduard Bagdasaryan [Tue, 23 May 2017 08:45:16 +0000 (20:45 +1200)] 
Create PID file ASAP, before the shared memory segments.

PID file is created right after configuration finalization, before the
allocation for any shared memory segments.

Late PID file creation allowed N+1 concurrent Squid instances to create
the same set of shared segments (overwriting each other segments),
resulting in extremely confusing havoc because the N instances would
later lose the race for the PID file (or some other critical resource)
creation and remove the segments. If that removal happened before a kid
of the single surviving instance started, that kid would fail to start
with open() errors in Segment.cc because the shared segment it tries to
open would be gone. Otherwise, that kid would fail to _restart_ after
any unrelated failures (possibly many days after the conflict), with
same errors, for the same reason.

Shared state corruption was also possible if different kids (of the
winning instance) opened (and started using) segments created (and
initialized) by different instances.

Situations with N+1 concurrent Squid instances are not uncommon because
many Squid service management scripts (or manual admin commands!)
* do not check whether another Squid is already running and/or
* incorrectly assume that "squid -z" does not daemonize.

This change finally makes starting N+1 Squid instances safe (AFAIK).

Also made daemonized and non-daemonized Squid create the PID file at the
same startup stage, reducing inconsistencies between the two modes.

8 years agoMake PID file check/creation atomic to avoid associated race conditions.
Eduard Bagdasaryan [Tue, 23 May 2017 08:31:54 +0000 (20:31 +1200)] 
Make PID file check/creation atomic to avoid associated race conditions.

After this change, if N Squid instances are concurrently started shortly
after time TS, then exactly one Squid instance (X) will run (and have
the corresponding PID file). If another Squid instance has already been
running (with the corresponding PID file) at TS, then X will be that
"old" Squid instance. If no Squid instances were running at TS, then X
will be one of those new N Squids started after TS.

Lack of atomic PID file operations caused unexpected Squid behavior:
* Mismatch between started Squid instance and stored PID file.
* Unexpected crashes due to failed allocation of shared resources,
  such as listening TCP ports or shared memory segments.

A new File class guarantees atomic PID file operations using locks. We
tried to generalize/reuse Ssl::Lock from the certificate generation
helper, but that was a bad idea: Helpers cannot use a lot of Squid code
(e.g., debugs(), TextException, SBuf, and enter_suid()), and the old
Ssl::Lock class cannot support shared locking without a major rewrite.

File locks on Solaris cannot work well (see bug #4212 comment #14), but
those problems do not affect PID file management code. Solaris- and
Windows-specific File code has not been tested and may not build.

Failure to write a PID file is now fatal. It used to be fatal only when
Squid was started with the -C command line option. In the increasingly
SMP world, running without a PID file leads to difficult-to-triage
errors. An admin who does not care about PID files should disable them.

Squid now exits with a non-zero error code if another Squid is running.

Also removed PID file rewriting during reconfiguration in non-daemon
mode. Squid daemons do not support PID file reconfiguration since trunk
r13867, but that revision (accidentally?) left behind half-broken
reconfiguration code for non-daemon mode. Fixing that code is difficult,
and supporting PID reconfigure in non-daemons is probably unnecessary.

Also fixed "is Squid running?" check when kill(0) does not have
permissions to signal the other instance. This does happen when Squid is
started (e.g., on the command line) by a different user than the user
Squid normally runs as or, perhaps, when the other Squid instance enters
a privileged section at the time of the check (untested). The bug could
result in undelivered signals or multiple running Squid instances.

These changes do not alter partially broken enter/leave_suid() behavior
of main.cc. That old code will need to be fixed separately!

PID file-related cache.log messages have changed slightly to improve
consistency with other DBG_IMPORTANT messages and to simplify code.
Squid no longer lies about creating a non-configured PID file. TODO:
Consider lowering the importance of these benign/boring messages.

* Terminal errors should throw instead of calling exit()

Squid used to call exit() in many PID-related error cases. Using exit()
as an error handling mechanism creates several problems:

1. exit() does not unwind the stack, possibly executing atexit()
   handlers in the wrong (e.g., privileged) context, possibly leaving
   some RAII-controller resources in bad state, and complicating triage;
2. Using exit() complicates code by adding a yet another error handling
   mechanism to the (appropriate) exceptions and assertions.
3. Spreading exit() calls around the code obscures unreachable code
   areas, complicates unifying exit codes, and confuses code checkers.

Long-term, it is best to use exceptions for nearly all error handling.
Reaching that goal will take time, but we can and should move in that
direction: The adjusted SquidMainSafe() treats exceptions as fatal
errors, without dumping core or assuming that no exception can reach
SquidMainSafe() on purpose. This trivial-looking change significantly
simplified (and otherwise improved) PID-file handling code!

The fatal()-related code suffers from similar (and other) problems, but
we did not need to touch it.

TODO: Audit catch(...) and exit() cases [in main.cc] to take advantage
of the new SquidMainSafe() code supporting the throw-on-errors approach.

8 years agoReplace String with SBuf for storage of header names in HttpHeaderEntry
Amos Jeffries [Tue, 23 May 2017 06:46:26 +0000 (18:46 +1200)] 
Replace String with SBuf for storage of header names in HttpHeaderEntry

In the process we gain SBuf methods for delByName and hasNamed.
Deprecating the char* versions.

8 years agoTests: Add stub_url.cc
Amos Jeffries [Tue, 23 May 2017 06:17:35 +0000 (18:17 +1200)] 
Tests: Add stub_url.cc

8 years agoDo not unconditionally revive dead peers after a DNS refresh.
Alex Rousskov [Mon, 22 May 2017 16:49:36 +0000 (10:49 -0600)] 
Do not unconditionally revive dead peers after a DNS refresh.

Every hour, peerRefreshDNS() performs a DNS lookup of all cache_peer
addresses. Before this patch, even if the lookup results did not change,
the associated peerDNSConfigure() code silently cleared dead peer
marking (CachePeer::tcp_up counter), if any.  Forcefully reviving dead
peers every hour can lead to transaction delays (and delays may lead to
failures) due to connection timeouts when using a still dead peer.

This patch starts standard TCP probing (instead of pointless dead peer
reviving), correctly refreshing peer state.  The primary goal is to
cover a situation where a DNS refresh changes the peer address list.
However, TCP probing may be useful for other situations as well and has
low overhead (that is why it starts unconditionally). For example,
probing may be useful when the DNS refresh changes the order of IP
addresses. It also helps detecting dead idle peers.

Also delay and later resume peer probing if peerDNSConfigure() is
invoked when peers are being probed. Squid should re-probe because the
current probes may use stale IP addresses and produce wrong results.

8 years agoFixed xstrndup() documentation, callers. Disclosed implementation bugs.
Alex Rousskov [Mon, 22 May 2017 16:42:28 +0000 (10:42 -0600)] 
Fixed xstrndup() documentation, callers. Disclosed implementation bugs.

xstrndup() does not work like strndup(3), and some callers got confused:

1. When n is the str length or less, standard strndup(str,n) copies all
   n bytes but our xstrndup(str,n) drops the last one. Thus, all callers
   must add one to the desired result length when calling xstrndup().
   Most already do, but it is often hard to see due to low code quality
   (e.g., one must remember that MAX_URL is not the maximum URL length).

2. xstrndup() also assumes that the source string is 0-terminated. This
   dangerous assumption does not contradict many official strndup(3)
   descriptions, but that lack of contradiction is actually a recently
   fixed POSIX documentation bug (i.e., correct implementations must not
   assume 0-termination): http://austingroupbugs.net/view.php?id=1019

The OutOfBoundsException bug led to truncated exception messages.

The ESI bug led to truncated 'literal strings', but I do not know what
that means in terms of user impact. That ESI fix is untested.

cachemgr.cc bug was masked by the fact that the buffer ends with \n
that is unused and stripped by the custom xstrtok() implementation.

TODO. Fix xstrndup() implementation (and rename the function so that
fixed callers do not misbehave if carelessly ported to older Squids).

8 years agoFix missong ERROR label on template loading errors
Amos Jeffries [Fri, 19 May 2017 13:37:24 +0000 (01:37 +1200)] 
Fix missong ERROR label on template loading errors

8 years agoBug 4653 - %st lies about tunneled traffic volumes
Christos Tsantilas [Thu, 18 May 2017 17:21:45 +0000 (20:21 +0300)] 
Bug 4653 - %st lies about tunneled traffic volumes

Squid-5 and squid-4 does not count the "HTTP/1.1 200 Connection Established"
header size for %<st formatting code.

This is a Measurement Factory project

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