Restored the natural order of the following two notifications:
* BodyConsumer::noteMoreBodyDataAvailable() and
* BodyConsumer::noteBodyProductionEnded() or noteBodyProducerAborted().
Commit b599471 unintentionally reordered those two notifications. Client
kids (and possibly other BodyConsumers) relied on the natural order to
end their work. If an HttpStateData job was done with the Squid-to-peer
connection and only waiting for the last adapted body bytes, it would
get stuck and leak many objects. This use case was not tested during b599471 work.
Amish [Wed, 2 Jan 2019 11:51:45 +0000 (11:51 +0000)]
basic_ldap_auth: Return BH on internal errors; polished messages (#347)
Basic LDAP auth helper now returns BH instead of ERR in case of errors
other than LDAP_SECURITY_ERROR, per helper guidelines.
Motivation: I have a wrapper around Basic LDAP auth helper. If an LDAP
server is down, then the helper returns BH, and the wrapper uses
a fallback authentication source.
Also converted printf() to SEND_*() macros and reduced message
verbosity.
Systems which have been partially 'IPv6 disabled' may allow
sockets to be opened and used but missing the IPv6 loopback
address.
Implement the outstanding TODO to detect such failures and
disable IPv6 support properly within Squid when they are found.
This should fix bug 4915 auth_param helper startup and similar
external_acl_type helper issues. For security such helpers are
not permitted to use the machine default IP address which is
globally accessible.
Fail Rock swapout if the disk dropped some of the write requests (#352)
Detecting dropped writes earlier is more than a TODO: If the last entry
write was successful, the whole entry becomes available for hits
immediately. IpcIoFile::checkTimeouts() that runs every 7 seconds
(IpcIoFile::Timeout) would eventually notify Rock about the timeout,
allowing Rock to release the failed entry, but that notification may
be too late.
The precise outcome of hitting an entry with a missing on-disk slice is
unknown (because the bug was detected by temporary hit validation code
that turned such hits into misses), but SWAPFAIL is the best we could
hope for.
Initialize StoreMapSlice when reserving a new cache slot (#350)
Rock sets the StoreMapSlice::next field when sending a slice to disk. To
avoid writing slice A twice, Rock allocates a new slice B to prime
A.next right before writing A. Scheduling A's writing and, sometimes,
lack of data to fill B create a gap between B's allocation and B's
writing (which sets B.next). During that time, A.next points to B, but
B.next is untouched.
If writing slice A or swapout in general fails, the chain of failed
entry slices (now containing both A and B) is freed. If untouched B.next
contains garbage, then freeChainAt() adds "random" slices after B to the
free slice pool. Subsequent swapouts use those incorrectly freed slices,
effectively overwriting portions of random cache entries, corrupting the
cache.
How did B.next get dirty in the first place? freeChainAt() cleans the
slices it frees, but Rock also makes direct noteFreeMapSlice() calls.
Shared memory cache may have avoided this corruption because it makes no
such calls.
Ipc::StoreMap::prepFreeSlice() now clears allocated slices. Long-term,
we may be able to move free slice management into StoreMap to automate
this cleanup.
Also simplified and polished slot allocation code a little, removing the
Rock::IoState::reserveSlotForWriting() middleman. This change also
improves the symmetry between Rock and shared memory cache code.
Before this fix, Squid sometimes logged the following error:
BUG: Worker I/O pop queue for ... overflow: ...
The bug could result in truncated hit responses, reduced hit ratio, and,
combined with buggy lost I/O handling code (GitHub PR #352), even cache
corruption.
The bug could be triggered by the following sequence of events:
* Disker dequeues one I/O request from the worker push queue.
* Worker pushes more I/O requests to that disker, reaching 1024 requests
in its push queue (QueueCapacity or just "N" below). No overflow here!
* Worker process is suspended (or is just too busy to pop I/O results).
* Disker satisfies all 1+N requests, adding each to the worker pop queue
and overflows that queue when adding the last processed request.
This fix limits worker push so that the sum of all pending requests
never exceeds (pop) queue capacity. This approach will continue to work
even if diskers are enhanced to dequeue multiple requests for seek
optimization and/or priority-based scheduling.
Pop queue and push queue can still accommodate N requests each. The fix
appears to reduce supported disker "concurrency" levels from 2N down to
N pending I/O requests, reducing queue memory utilization. However, the
actual reduction is from N+1 to N: Since a worker pops all its satisfied
requests before queuing a new one, there could never be more than N+1
pending requests (N in the push queue and 1 worked on by the disker).
We left the BUG reporting and handling intact. There are no known bugs
in that code now. If the bug never surfaces again, it can be replaced
with code that translates low-level queue overflow exception into a
user-friendly TextException.
Alex Rousskov [Tue, 8 Jan 2019 15:14:18 +0000 (15:14 +0000)]
Fix BodyPipe/Sink memory leaks associated with auto-consumption (#348)
Auto-consumption happens (and could probably leak memory) in many cases,
but this leak was exposed by an eCAP service that blocked or replaced
virgin messages.
The BodySink job termination algorithm relies on body production
notifications. A BodySink job created after the body production had
ended can never stop and, hence, leaks (leaking the associated BodyPipe
object with it). Such a job is also useless: If production is over,
there is no need to free space for more body data! This change avoids
creating such leaking and useless jobs.
Amos Jeffries [Sun, 6 Jan 2019 13:22:19 +0000 (13:22 +0000)]
Bug 4875 pt2: GCC-8 compile errors with -O3 optimization (#288)
GCC-8 warnings exposed at -O3 optimization causes its
own static analyzer to detect optimized code is eliding
initialization on paths that do not use the
configuration variables.
Refactor the parseTimeLine() API to return the parsed
values so that there is no need to initialize anything prior
to parsing.
Fixed forward_max_tries documentation and implementation (#277)
Before 1c8f25b, FwdState::n_tries counted the total number of forwarding
attempts, including pinned and persistent connection retries. Since that
revision, it started counting just those retries. What should n_tries
count? The counter is used to honor the forward_max_tries directive, but
that directive was documented to limit the number of _different_ paths
to try. Neither 1c8f25b~1 nor 1c8f25b code matched that documentation!
Continuing to count just pinned and persistent connection retries (as in 1c8f25b) would violate any reasonable forward_max_tries intent and admin
expectations. There are two ways to fix this problem, synchronizing code
and documentation:
* Count just the attempts to use a different forwarding path, matching
forward_max_tries documentation but not what Squid has ever done. This
approach makes it difficult for an admin to limit the total number of
forwarding attempts in environments where, say, the second attempt is
unlikely to succeed and will just incur wasteful delays (Squid bug
4788 report is probably about one of such use cases). Also,
implementing this approach may be more difficult because it requires
adding a new counter for retries and, for some interpretations of
"different", even a container of previously visited paths.
* Count all forwarding attempts (as before 1c8f25b) and adjust
forward_max_tries documentation to match this historical behavior.
This approach does not have known unique flaws.
Also fixed FwdState::n_tries off-by-one comparison bug discussed during
Squid bug 4788 triage.
Also fixed admin concern behind Squid bug 4788 "forward_max_tries 1 does
not prevent some retries": While the old forward_max_tries documentation
actually excluded pconn retries, technically invalidating the bug
report, the admin now has a knob to limit those retries.
chi-mf [Tue, 30 Oct 2018 04:48:40 +0000 (04:48 +0000)]
Fix netdb exchange with a TLS cache_peer (#307)
Squid uses http-scheme URLs when sending netdb exchange (and possibly
other) requests to a cache_peer. If a DIRECT path is selected for that
cache_peer URL, then Squid sends a clear text HTTP request to that
cache_peer. If that cache_peer expects a TLS connection, it will reject
that request (with, e.g., error:transaction-end-before-headers),
resulting in an HTTP 503 or 504 netdb fetch error.
Workaround this by adding an internalRemoteUri() parameter to indicate
whether https or http URL scheme should be used. Netdb fetches from
CachePeer::secure peers now get an https scheme and, hence, a TLS
connection.
chi-mf [Thu, 25 Oct 2018 13:33:06 +0000 (13:33 +0000)]
Update netdb when tunneling requests (#314)
Updating netdb on tunneled transactions (e.g., CONNECT requests) is
especially important for origin servers that are only reached via
tunnels. Without updates, requests for such sites may always through a
cache_peer, even if a direct connection to them is much faster.
Logging client "handshake" bytes is useful in at least two contexts:
* Runtime traffic bypass and bumping/splicing decisions. Identifying
popular clients like Skype for Business (that uses a TLS handshake but
then may not speak TLS) is critical for handling their traffic
correctly. Squid does not have enough ACLs to interrogate most TLS
handshake aspects. Adding more ACLs may still be a good idea, but
initial sketches for SfB handshakes showed rather complex
ACLs/configurations, _and_ no reasonable ACLs would be able to handle
non-TLS handshakes. An external ACL receiving the handshake is in a
much better position to analyze/fingerprint it according to custom
admin needs.
* A logged handshake can be used to analyze new/unusual traffic or even
trigger security-related alarms.
The current support is limited to cases where Squid was saving handshake
for other reasons. With enough demand, this initial support can be
extended to all protocols and port configurations.
flozilla [Wed, 24 Oct 2018 12:12:01 +0000 (14:12 +0200)]
Fix memory leak when parsing SNMP packet (#313)
SNMP queries denied by snmp_access rules and queries with certain
unsupported SNMPv2 commands were leaking a few hundred bytes each. Such
queries trigger "SNMP agent query DENIED from..." WARNINGs in cache.log.
Certificate fields injection via %D in ERR_SECURE_CONNECT_FAIL (#306)
%ssl_subject, %ssl_ca_name, and %ssl_cn values were not properly escaped
when %D code was expanded in HTML context of the ERR_SECURE_CONNECT_FAIL
template. This bug affects all ERR_SECURE_CONNECT_FAIL page templates
containing %D, including the default template.
Other error pages are not vulnerable because Squid does not populate %D
with certificate details in other contexts (yet).
Thanks to Nikolas Lohmann [eBlocker] for identifying the problem.
TODO: If those certificate details become needed for ACL checks or other
non-HTML purposes, make their HTML-escaping conditional.
Eneas Queiroz [Wed, 10 Oct 2018 16:45:29 +0000 (16:45 +0000)]
Allow compilation with minimal OpenSSL (#281)
Updated use of OpenSSL deprecated API, so that Squid can be compiled
with OpenSSL built with the OPENSSL_NO_DEPRECATED option. Such OpenSSL
builds are useful for saving storage space on embedded systems.
Also added compat/openssl.h -- a centralized OpenSSL portability shim.
Including it is now required before #including openssl/*.h headers.
chi-mf [Wed, 10 Oct 2018 07:50:52 +0000 (07:50 +0000)]
Fixed %USER_CA_CERT_xx and %USER_CERT_xx crashes (#301)
The bug was introduced in 4e56d7f6 when the formatting code was moved
into Format::Format::assemble() where the old "format" loop variable is
a Format data member with the right type but (usually) the wrong value.
Bug 4893: Malformed %>ru URIs for CONNECT requests (#299)
Commit bec110e (a.k.a. v4 commit fbbd5cd5) broke CONNECT URI logging
because it incorrectly assumed that URI::absolute() supports all URIs.
As the result, Squid logged CONNECT URLs as "://host:port".
Also fixed a similar wrong assumption in ACLFilledChecklist::verifyAle()
which may affect URL-related ACL checks for CONNECT requests, albeit
only in already buggy cases where Squid warns about "ALE missing URL".
Bug 4885: Excessive memory usage when running out of descriptors (#291)
TcpAcceptor now stops listening when it cannot accept due to FD limits.
We also no longer defer/queue the same limited TcpAcceptor multiple
times. These changes prevent unbounded memory growth and improve
performance of Squids running out of file descriptors. They should have
no impact on other Squids.
Bug 4875 pt1: GCC-8 compile errors with -O3 optimization (#287)
Use xstrncpy instead of strncat for String appending
Our xstrncpy() is safer, not assuming the existing char*
is nul-terminated and accounting explicitly for the
nul-terminator byte.
GCC-8 -O3 optimizations were exposing a strncat() output
truncation of the terminator when insufficient space was
available in the String buffer.
We suspect the GCC error to be a false-positive for -O3
builds and, even it it is accurate, these changes should
not affect builds with lower optimization levels.
This change also fixes icc builds: Commit 39cca4e missed noexcept
specification for nothrow variants of new and delete operators,
and the icc compiler did not like that.
Furthermore, we can simplify the replacements because, according
to cppreference, with C++11, "replacing the throwing single object
allocation functions is sufficient to handle all [allocations and
deallocations]".
Bug 4877: Add missing text about external_acl_type %DATA changes (#276)
Conversion of external_acl_type to using logformat macros was
not quite seamless. The %DATA macro now expands to a dash '-' to
fix helpers using it explicitly from receiving incorrect number
of fields (and misaligned input) on their input lines.
Unfortunately that also results in the implicit use of that
macro expanding to non-whitespace ('-'). That small fact was not
documented in the initial v4 release notes and config texts.
Bug 4716: Blank lines in cachemgr.conf are not skipped (#274)
The default cachemgr.conf contains three lines other than
comments. Two of them are blank, the third is "localhost".
These blank lines show up in the "Cache Server" list in the
CGI output.
Amos Jeffries [Tue, 7 Aug 2018 13:00:02 +0000 (13:00 +0000)]
Update systemd dependencies in squid.service (#264)
The network.target is not sufficient to guarantee network
interfaces and IPs are assigned and available. Particularly when
systemd is not in charge of the IP assignment itself.
Use network-online.target as well, which should ensure network
is properly configured and online before starting Squid.
Packing reply headers into StoreEntry/ShmWriter directly means numerous
tiny append() calls which involve expensive mem_node/slice searches. For
example, every two-byte ": " and CRLF delimiter is packed separately.
When dealing with an HTTP request header that Squid can parse but that
contains request URI length exceeding the 8K limit, Squid should log the
URL (prefix) instead of a dash. Logging the URL helps with triaging
these unusual requests. The older %ru (LFT_REQUEST_URI) was already
logging these huge URLs, but %>ru (LFT_CLIENT_REQ_URI) was logging a
dash. Now both log the URL (or its prefix).
As a side effect, %>ru now also logs error:request-too-large,
error:transaction-end-before-headers and other Squid-specific
pseudo-URLs, as appropriate.
Also refactored request- and URI-recording code to reduce chances of
similar inconsistencies reappearing in the future.
Also, honor strip_query_terms in %ru for large URLs. Not stripping query
string in %ru was a security problem.
Also fixed a bug with "redirected" flag calculation in
ClientHttpRequest::handleAdaptedHeader(). In general, http->url and
request->url should not be compared directly, because the latter always
passes through uri_whitespace cleanup, while the former does not.
Also fixed a bug with possibly wrong %ru after redirection:
ClientHttpRequest::log_uri was not updated in this case.
Also initialize AccessLogEntry::request and AccessLogEntry::notes ASAP.
Before this change, these fields were initialized in
ClientHttpRequest::doCallouts(). It is better to initialize them just
after the request object is created so that ACLs, running before
doCallouts(), could have them at hand. There are at least three such
ACLs: force_request_body_continuation, spoof_client_ip and
spoof_client_ip.
Also synced %ru and %>ru documentation with the current code.
Bug 4843 pt3: GCC-8 fixes and refactoring (#172) (#256)
GCC-8 enables a lot more warnings related to unsafe coding
practices. The old Squid code contains a lot of risky buffer
size assumptions and implicit assumptions about C-string strcat,
strncat and snprintf changes when operating on those buffers -
many can result in output truncation. Squid's use of -Werror
makes these many issues all go from warnings to outright
compile failures.
Rather than just extending the char* buffer sizes not to
truncate this work seeks to actually remove the issues
permanently by converting to SBuf and updated Squid coding
styles.
The C++1z compilers (GCC-8 and Clang 4.0) are beginning to warn
about C functions memset/memcpy/memmove being used on class
objects which lack "trivial copy" constructor or assignment
operator - their use is potentially unsafe where anything more
complex than trivial copy/blit is required. A number of classes
in Squid are safely copied or initialized with those functions
for now but again the -Werror makes these hard errors.
Completing affected objects conversion from C to C++ code avoids
any deeply hidden issues or adding compiler exceptions to
silence the warnings.
Optimization: Do not create/configure ACLFilledChecklist in vain (#232)
While client_db is required for client-side pools to work, it may be
enabled for other reasons, without any client-side pools configured. We
should not create and configure useless ACLFilledChecklist objects
because those operations are already not trivial today and have a
a tendency of becoming more expensive with time.
Optimization: Fewer epoll(2) system calls when closing a socket (#235)
Squid was calling epoll(2) twice to clear a socket interest. One call is
more than enough: Technically, close(2) is supposed to clear epoll(2)
registration for us, but I did not risk relying on that.
In other environments, socket interest changes are pooled together
before being submitted to the OS, so Squid was doing a bit of extra
work, but not making (many) extra system calls AFAICT.
Also fixed (previously unused) Comm::ResetSelect() on these platforms:
* epoll(2): The old resetting code did not clear our interest AFAICT.
* kqueue(2): The old resetting code made no sense to me at all.
* poll(2): There was no code at all.
* select(Win32): There was no code at all.
Even though Comm::ResetSelect() implementation is now the same for all
platforms, I did not make that code platform-agnostic because it is
possible to optimize it further in platform-specific ways.
Alex Rousskov [Wed, 4 Jul 2018 15:59:26 +0000 (15:59 +0000)]
Documented when helper requests get queued (#238)
I had to change introductory paragraphs in several directives so that
the new documentation can refer to "numberofchildren". I fixed a few
spelling/grammar problems in changed paragraphs and edited them a bit
for consistency, but they need more work.
When an HTTPS or SSL-Bump port is configured without a cert=
parameter it results in a segmentation fault. Detect that
occurance and add the required FATAL error message instead for
these configurations where cert= is a parameter rather than an
option.
Our project terminology for config settings is;
"parameter"
- a required setting. Print a FATAL error message if missing.
"option"
- an optional setting. Ignored or default value if missing.
Bug 4223: fixed retries of failed re-forwardable transactions (#211)
This change fixes Store to delay writing (and, therefore, sharing) of
re-triable errors (e.g., 504 Gateway Timeout responses) to clients:
once we start sharing the response with a client, we cannot re-try the
transaction. Since ENTRY_FWD_HDR_WAIT flag purpose is to delay response
sharing with Store clients, this patch fixes and clarifies its usage:
1. Removes unconditional clearing from startWriting().
2. Adds a conditional clearing to StoreEntry::write().
3. Sets it only for may-be-rejected responses.
(2) adds ENTRY_FWD_HDR_WAIT clearing to detect responses that filled the
entire read buffer and must be shared now with the clients because
they can no longer be retried with the current re-forwarding mechanisms
(which rely on completing the bad response transaction first) and will
get stuck. Such large re-triable error responses (>16KB with default
read_ahead_gap) should be uncommon. They were getting stuck prior to
master r12501.1.48. That revision started clearing ENTRY_FWD_HDR_WAIT in
StoreEntry startWriting() unconditionally, allowing all errors to be
sent to Store clients without a delay, and effectively disabling
retries.
Mgr::Forwarder was always setting ENTRY_FWD_HDR_WAIT, probably mimicking
similarly aggressive FwdState behavior that we are now removing. Since
the forwarder never rewrites the entry content, it should not need to
set that flag. The forwarder and associated Server classes must not
communicate with the mgr client during the client-Squid connection
descriptor handoff to Coordinator, but ENTRY_FWD_HDR_WAIT is not the
right mechanism to block such Squid-client communication. The flag
does not work well for this purpose because those Server classes may
(and do?) substitute the "blocked" StoreEntry with another one to
write an error message to the client.
Also moved ENTRY_FWD_HDR_WAIT clearance from many StoreEntry::complete()
callers to that method itself. StoreEntry::complete() is meant to be the
last call when forming the entry. Any post-complete entry modifications
such as retries are prohibited.
Amos Jeffries [Tue, 5 Jun 2018 06:11:29 +0000 (06:11 +0000)]
Bug 4831: filter chain certificates for validity when loading (#187)
51e09c08a5e6c582e7d93af99a8f2cfcb14ea9e6 adding
GnuTLS support required splitting the way
certificate chains were loaded. This resulted in the
leaf certificate being added twice at the prefix of a
chain in the serverHello.
It turns out that some recipients validate strictly that the
chain delivered by a serverHello does not contain extra
certificates and reject the handshake if they do.
This patch implements the XXX about filtering certificates
for chain sequence order and self-sign properties, added
in the initial PR. Resolving the bug 4831 regression and also
reporting failures at startup/reconfigure for admins.
Also, add debug display of certificate names for simpler
detection and administrative fix when loaded files fail
these tests.
Bug 4855: re-enable querying private entries for HTCP/ICP (#214)
This was broken since 4310f8b: HTCP/ICP misused Store as a storage of
private entries during queries (e.g., see
neighborsUdpPing()/neighborsUdpAck()). A smarter HTCP/ICP
implementation would maintain its own StoreEntry cache for this purpose
(just like the existing queried_keys array for cache keys). However,
fixing this is beyond this issue scope.
Amos Jeffries [Tue, 22 May 2018 12:55:35 +0000 (12:55 +0000)]
Bug 4707: purge tool does not obey --sysconfdir= build option (#210)
The purge tool was still using DEFAULT_SQUID_CONF macro from
before it was bundled with Squid, which had no connection to our
./configure script.
Update it to the DEFAULT_CONFIG_FILE macro used by other Squid
binaries and fix some existing issues with that macro's use by
binaries outside 'squid'.
Amos Jeffries [Sun, 20 May 2018 15:46:50 +0000 (15:46 +0000)]
Bug 4843 pt2: squidclient refactoring for GCC-8 (#208)
Replace fixed size buffers for mime header block and additional
custom headers. This fixes long standing issues with buffer
overflow from large custom header values which have become a
hard error in GCC-8.
Also improve snprintf() URL buffer limit handling and const
correctness for Transport::Write().
Bug 4811: supply AccessLogEntry (ALE) for more fast ACL checks. (#182)
Supplying ALE for fast ACL checks allows those checks to use ACLs that
assemble values from logformat %codes. Today, such ACLs are limited to
misplaced external ACLs (that should not be used with "fast"
directives!), but it is likely that fast ACLs like annotate_client will
eventually require ALE.
The "has" ACL documentation promises ALE for every transaction, but our
code does not deliver on that promise. This change fixes a dozen of
easy cases where ALE was available nearby. Also a non-trivial
cache_peer_access case was fixed, which proved to be more complex
because of the significant call depth of the peerAllowedToUse() check,
which is a known design problem of its own.
More cases need fixing, and the whole concept of ALE probably needs to
be revised because logformat %code expansion is needed in the
increasing number of contexts that have nothing to do with access
logging.
Also fixed triggering of (probably pointless) level-1 warnings:
* ALE missing adapted HttpRequest object
* ALE missing URL
With fix applied, any ACLChecklist with ALE synchronizes it at
'pre-check' stage without logging level-1 warnings. Warnings are
triggered only if for some reason this 'pre-check' synchronization was
bypassed.
Amos Jeffries [Sun, 13 May 2018 06:57:41 +0000 (06:57 +0000)]
Bug 4843 pt1: ext_edirectory_userip_acl refactoring for GCC-8 (#204)
Proposed changes to this helper to fix strcat / strncat buffer
overread / overflow issues.
The approach takes three parts:
* adds a makeHexString function to replace many for-loops
catenating bits of strings together with hex conversion into a
second buffer. Replacing with a snprintf() and buffer overflow
handling.
* a copy of Ip::Address::lookupHostIp to convert the input
string into IP address binary format, then generate the hex
string using the above new hex function instead of looped
sub-string concatenations across several buffers.
This removes all the "00" and "0000" strncat() calls and
allows far simpler code even with added buffer overflow
handling.
* replace multiple string part concatenations with a few simpler
calls to snprintf() for all the search_ip buffer constructions.
Adding buffer overflow handling as needed for the new calls.
huaraz [Sun, 6 May 2018 16:06:42 +0000 (16:06 +0000)]
Bug 4042: ext_kerberos_ldap_group: add -P principal option (#195)
Added a -P principal option to ext_kerberos_ldap_group to
select a principal from the keytab overwriting the automated
method which may make it more responsive.
Bug 4845: NegotiateSsl crash on aborting transaction (#201)
Security::PeerConnector::NegotiateSsl() might be called after the
Security::PeerConnector object is gone. This race condition is present
on both regular SSL and SslBump code paths, but sightings are rare.
This bug shares the underlying cause (and the solution) with bug 3505.
TODO: Adjust Comm::SetSelect() API to prevent future bugs like this.
Amos Jeffries [Sat, 5 May 2018 14:42:12 +0000 (14:42 +0000)]
Bug 4847 pt1: regression in proxy_auth ACL flags (#191)
r15058 "Support for --long-acl-options" in Squid-4.0.21
unintentionally removed the proxy_auth ACL support for -i/+i
flags. See bug report for details.
Fix proxy_auth ACL -i and +i flags no longer working by copying
RegexData flags registration, since ACLs for UserData all use
the same names and meanings.
Add documentation to indicate that ident and ext_user ACLs do
support -i/+i just like proxy_auth ACLs.
TODO: fix server_cert_fingerprint ACL which is still broken.
Amos Jeffries [Thu, 3 May 2018 15:52:04 +0000 (15:52 +0000)]
Bug 4852: regression in deny_info %R macro (#193)
SBuf::c_str() produces a temporary c-string which is not
guaranteed to survive, and does not survive as long as required
to print the deny_info URL. The HttpRequest::url path SBuf has
a much longer lifetime, so use a const reference to it instead.
Do not abuse argv[0] to supply roles and IDs to SMP kids (#176)
Use a newly added "--kid role-ID" command line option instead. Just like
argv[0], the new option is not meant for direct/human use.
This change allows exec(3)-wrapping tools like Valgrind to work with SMP
Squid: When launching kid processes, Valgrind does not pass Squid-formed
argv[0] to kid processes, breaking old kid role and ID detection code.
This change does not alter argv[0] of Squid processes. There is nothing
wrong with Squid-formed argv[0] values for Squid kids.
Also added a CommandLine class to support command line parsing without
code duplication. Squid needs to handle the new --kid option way before
the old mainParseOptions() handles the other options. The new class also
encapsulates argv manipulations, reducing main.cc pollution.
Avoid ssl/helper.cc "ssl_crtd" assertions on reconfiguration (#186)
Reconfiguration process consists of mainReconfigureStart() and
mainReconfigureFinish() steps separated by at least one main loop
iteration. Clearing a Squid global variable in mainReconfigureStart()
creates two problems for transactions that were started before
reconfiguration:
1. Transactions accessing that global _during_ reconfiguration loop
iteration(s) may be confused by the variable sudden disappearance.
2. Transactions accessing that global _after_ mainReconfigureFinish()
may be confused by the variable disappearance if reconfiguration
resulted in the global variable becoming nil.
To remove the first problem for ssl_crtd, external_acl, and redirecting
helpers, all of them are now reconfigured "instantly", during
mainReconfigureFinish().
To prevent crashes due to the second problem, Squid now generates helper
errors if the disappeared ssl_crtd or external_acl helpers are accessed
after reconfiguration. The admin is warned about such problems via
level-1 cache.log ERROR messages.
The second problem cannot be fully solved without storing (refcounted)
configuration globals inside each transaction that uses them. Such
serious changes are outside this small assertion-fixing project scope.
Reliable timestamp information is often critical for triage. We can use
the existing debugs() interface to add timestamps to FATAL messages. The
affected code already calls such risky functions as
storeDirWriteCleanLogs() so calling debugs() instead of printing
directly to files/syslog should not make things worse.
FATAL messages that were also logged to syslog (at LOG_ALERT level) are
still logged to syslog (at that same level, but now with the usual
Squid-generated prefix). Such syslog alerts can now be easily triggered
via a new ForceAlert() API.
Also treat segmentation faults, bus errors, and other signal-based
sudden deaths the same as most other FATAL errors -- log them to syslog.
Alex Rousskov [Thu, 12 Apr 2018 22:12:39 +0000 (22:12 +0000)]
Fixed Transient reader locking broken by 4310f8b (#161)
The closeForWriting() call comment is correct -- we must keep the lock,
but the optional argument was accidentally lost (when undoing the failed
attempt to remove all long-term transient locks in the feature branch).
Also polished stale comments (which led to the above bug discovery!).
Also polished addEntry() aspects that I have missed in 4310f8b.
The fixed leak was accompanied by these cache.log errors:
ERROR: worker I/O push queue for ... overflow: ...
I/O queue overflows during disk read requests log the same error but do
not leak memory. Repeated overflows during disk write requests could
eventually exhaust IPC shared memory:
ERROR: ... exception: run out of shared memory pages for IPC I/O
With IPC memory exhausted due to leaks, rock disk I/O stops forever.
projects / thirdparty / squid.git / log
