]> git.ipfire.org Git - thirdparty/squid.git/log
thirdparty/squid.git
4 months agoRemove ConnStateData::pinning.peer (#2034)
Eduard Bagdasaryan [Sat, 22 Mar 2025 23:56:58 +0000 (23:56 +0000)] 
Remove ConnStateData::pinning.peer (#2034)

Use ConnStateData::pinning.serverConnection.getPeer() instead of
copying: pinning.peer does not need to outlive pinning.serverConnection.

4 months agontlm_sspi_auth: Fix missing base64 symbol linkage (#2031)
Francesco Chemolli [Sat, 22 Mar 2025 22:30:29 +0000 (22:30 +0000)] 
ntlm_sspi_auth: Fix missing base64 symbol linkage (#2031)

Solve build error:

```
ld: ntlm_sspi_auth.o: in function `token_decode':
    undefined reference to `nettle_base64_decode_init'
    undefined reference to `nettle_base64_decode_update'
    undefined reference to `nettle_base64_decode_final'
```

4 months agoRemove HttpRequest::peer_host (#2032)
Eduard Bagdasaryan [Thu, 20 Mar 2025 19:44:48 +0000 (19:44 +0000)] 
Remove HttpRequest::peer_host (#2032)

HttpRequest::peer_host was added in 2009 commit 9ca29d23 so that
httpFixupAuthentication() could pass copied raw CachePeer::host pointer
value to peer_proxy_negotiate_auth(). Unfortunately, raw peer_host
pointer (to CachePeer::host memory) becomes dangling when CachePeer is
reconfigured away. Instead of maintaining this problematic field, we can
safely obtain the same CachePeer::host value from HttpStateData::_peer.

4 months agoDo not reuse an idle connection to a removed cache_peer (#2028)
Eduard Bagdasaryan [Tue, 18 Mar 2025 14:58:48 +0000 (14:58 +0000)] 
Do not reuse an idle connection to a removed cache_peer (#2028)

Sending new requests to a removed cache_peer contradicts current Squid
configuration and even exposes Squid code that forgets to check
CachePeer validity to dangling pointers. We will address the latter
concern separately.

4 months agoMingGW: do not build ext_edirectory_userip_acl (#2017)
Francesco Chemolli [Sun, 16 Mar 2025 23:25:46 +0000 (23:25 +0000)] 
MingGW: do not build ext_edirectory_userip_acl (#2017)

ext_edirectory_userip_acl uses API that are too
different from what is provided by windlap.h, do not build it

4 months agoCI: Enable eCAP and Valgrind in layer build tests if possible (#1996)
Eduard Bagdasaryan [Sun, 16 Mar 2025 22:03:22 +0000 (22:03 +0000)] 
CI: Enable eCAP and Valgrind in layer build tests if possible (#1996)

Now these optional features are enabled during applicable layer tests if
their packages appear to be available on the build system. This should
help prevent regressions like the one fixed in recent commit 53ed1a9.

4 months agonegotiate_sspi_auth: Fix command debugging (-v) (#2025)
Francesco Chemolli [Sun, 16 Mar 2025 14:15:56 +0000 (14:15 +0000)] 
negotiate_sspi_auth: Fix command debugging (-v) (#2025)

Terminate helper_command buffer before using it as a c-string. Supported
helper commands have two characters.

This change also reduces MinGW build errors.

4 months agoMinGW: do not build negotiate_wrapper (#2026)
Francesco Chemolli [Sat, 15 Mar 2025 06:21:20 +0000 (06:21 +0000)] 
MinGW: do not build negotiate_wrapper (#2026)

MinGW is missing fork(2). Deeper changes will be
needed to build this helper

4 months agoPortability: add include guards to ldap_backend.cc (#2021)
Francesco Chemolli [Sat, 15 Mar 2025 05:00:49 +0000 (05:00 +0000)] 
Portability: add include guards to ldap_backend.cc (#2021)

In the non-Windows case, we unconditinoally
include some system headers. Add conditions
for improved portability

Fixes error:
```
src/auth/digest/LDAP/ldap_backend.cc:49:10:
   fatal error: lber.h: No such file or directory
```

4 months agoMinGW: fix store/Controller.cc build (#2019)
Francesco Chemolli [Sat, 15 Mar 2025 03:38:36 +0000 (03:38 +0000)] 
MinGW: fix store/Controller.cc build (#2019)

    error: 'WNOHANG' was not declared in this scope

4 months agoMinGW: add mkdir adapter (#2018)
Francesco Chemolli [Fri, 14 Mar 2025 17:21:31 +0000 (17:21 +0000)] 
MinGW: add mkdir adapter (#2018)

On Windows, mkdir only takes one argument.
compat/mswindows.h has an adapter, add it to
compat/mingw.h as well.

Solves error:

```
UFSSwapDir.cc:617:26: error: too many arguments
    to function 'int mkdir(const char*)'
mingw/include/io.h:282:15: note: declared here
    int __cdecl mkdir (const char *);
```

4 months agoFix Mem::Segment::open() stub to fix build without shm_open() (#2016)
Francesco Chemolli [Fri, 14 Mar 2025 15:55:44 +0000 (15:55 +0000)] 
Fix Mem::Segment::open() stub to fix build without shm_open() (#2016)

When the environment does not HAVE_SHM, Ipc::Mem::Segment::open() method
definition does not match its declaration. Make it match.

    src/ipc/mem/Segment.cc:346:1: error: no declaration matches
    void Ipc::Mem::Segment::open()
    src/ipc/mem/Segment.h:37:10: note: candidate is:
    void Ipc::Mem::Segment::open(bool)

4 months agoMinGW: Fix ext_ad_group_acl build errors (#2005)
Francesco Chemolli [Fri, 14 Mar 2025 02:09:45 +0000 (02:09 +0000)] 
MinGW: Fix ext_ad_group_acl build errors (#2005)

- In C++, win32 APIs want references, not pointers
- Fix const mismatch
- Other minor polishing touches

4 months agoMinGW: add libnettle to negotiate_sspi_auth (#2024)
Francesco Chemolli [Fri, 14 Mar 2025 00:40:13 +0000 (00:40 +0000)] 
MinGW: add libnettle to negotiate_sspi_auth (#2024)

libnettle is needed to build negotiate_sspi_auth.

This change fixes many errors similar to:

    negotiate_sspi_auth.cc:126: undefined reference to
        nettle_base64_decode_init

4 months agoMaintenance: remove compat/psignal (#2022)
Francesco Chemolli [Thu, 13 Mar 2025 23:08:32 +0000 (23:08 +0000)] 
Maintenance: remove compat/psignal (#2022)

psignal(3) is not used anywhere in squid.
Remove the also-unused bundled implementation

4 months agoMinGW: fix aio compatibility layer (#2020)
Francesco Chemolli [Thu, 13 Mar 2025 21:21:48 +0000 (21:21 +0000)] 
MinGW: fix aio compatibility layer (#2020)

The AIO Windows compatibilty layer is also
necessary on mingw

Problems fixed:

```
DiskIO/AIO/async_io.h:58:18:
    error: field 'aq_e_aiocb' has incomplete type 'aiocb'
DiskIO/AIO/async_io.h:58:12:
     note: forward declaration of 'struct aiocb'

DiskIO/AIO/AIODiskFile.cc:
    In member function
    'virtual void AIODiskFile::read(ReadRequest*)':
src/DiskIO/AIO/AIODiskFile.cc:134:9:
    error: 'aio_read' was not declared in this scope;
    did you mean 'file_read' ?
```

4 months agoFix tls-dh support for DHE parameters with OpenSSL v3+ (#1949)
Andreas Weigel [Thu, 13 Mar 2025 11:30:28 +0000 (11:30 +0000)] 
Fix tls-dh support for DHE parameters with OpenSSL v3+ (#1949)

    # When applying tls-dh=prime256v1:dhparams.pem configuration:
    WARNING: Failed to decode EC parameters 'dhparams.pem'

    # When forcing the use of FFDHE with something like
    # openssl s_client -tls1_2 -cipher DHE-RSA-AES256-SHA256 -connect...
    ERROR: failure while accepting a TLS connection on:
        SQUID_TLS_ERR_ACCEPT+TLS_LIB_ERR=A0000C1+TLS_IO_ERR=1

Squid `https_port ... tls-dh=curve:dhparams.pem` configuration is
supposed to support _both_ ECDHE and FFDHE key exchange mechanisms (and
their cipher suites), depending on client-supported cipher suites. ECDHE
mechanism should use the named curve (e.g., `prime256v1`), and FFDHE
mechanism should use key exchange parameters loaded from the named PEM
file (e.g., `ffdhe4096` named group specified in RFC 7919).

When 2022 commit 742236c added support for OpenSSL v3 APIs, new
loadDhParams() code misinterpreted curve name presence in `tls-dh` value
as an indication that the named parameters file contains ECDHE
parameters, setting OSSL_DECODER_CTX_new_for_pkey() type parameter to
"EC", and (when parameter file specified FFDHE details) triggering the
WARNING message quoted above.

Squid should not expect additional ECDHE parameters when the elliptic
curve group is already fully specified by naming it at the start of
`tls-dh` value. Squid now reverts to earlier (v4) behavior, where
the two mechanisms can coexist and can be configured separately as
described above:

    $ openssl s_client -tls1_2 -cipher DHE-RSA-AES256-SHA256 -connect...
    Server Temp Key: DH, 4096 bits

    $ openssl s_client -connect...
    Server Temp Key: ECDH, prime256v1, 256 bits

Furthermore, updateContextEecdh() code in commit 742236c continued  to
load parsed parameters using old SSL_CTX_set_tmp_dh() call but should
have used SSL_CTX_set0_tmp_dh_pkey() API because the type of parsed
parameters (i.e. DhePointer) have changed from DH to EVP_PKEY pointer.
This second bug affected configurations with and without an explicit
curve name in `tls-dh` value.

Also report a failure to load parsed parameters into TLS context.

4 months agoRemove basic_smb_lm_auth helper (#2014)
Francesco Chemolli [Tue, 11 Mar 2025 16:34:23 +0000 (16:34 +0000)] 
Remove basic_smb_lm_auth helper (#2014)

This helper implementation is based on an old snapshot and adaptation of
Samba code. Samba project offers a more secure and better maintained
tool.

4 months agoRemove ntlm_smb_lm_auth helper (#2015)
Francesco Chemolli [Mon, 10 Mar 2025 10:13:00 +0000 (10:13 +0000)] 
Remove ntlm_smb_lm_auth helper (#2015)

This helper implementation is based on an old snapshot and adaptation of
Samba code. Samba project offers a more secure and better maintained
tool.

4 months agoRevert "Remove basic_smb_lm_auth helper" (#2013)
Francesco Chemolli [Sun, 9 Mar 2025 09:06:31 +0000 (09:06 +0000)] 
Revert "Remove basic_smb_lm_auth helper" (#2013)

Commit f53a024a40 contained spurious changes. Revert the change

4 months agoRemove basic_smb_lm_auth helper (#2009)
Francesco Chemolli [Thu, 6 Mar 2025 00:24:22 +0000 (00:24 +0000)] 
Remove basic_smb_lm_auth helper (#2009)

This helper implementation is based on an old snapshot and adaptation of
Samba code. Samba project offers a more secure and better maintained
tool.

4 months agoRemove ext_lm_group_acl helper (#2011)
Francesco Chemolli [Tue, 4 Mar 2025 19:06:59 +0000 (19:06 +0000)] 
Remove ext_lm_group_acl helper (#2011)

Lan Manager (LM) is an obsolete variant of the SMB protocol.
No product on the market has supported it for several years now,
in favour of NTLMv1 , NTLMv2 and Kerberos.

4 months agoMinGW: use nameless unions in ext_ad_group_acl (#2004)
Francesco Chemolli [Tue, 4 Mar 2025 15:15:06 +0000 (15:15 +0000)] 
MinGW: use nameless unions in ext_ad_group_acl (#2004)

ext_ad_group_acl was written in 2008 in C, and
it used the C variant of the Win32 API.
It was then ported to C++, but the API callers were
not updated to the C++ version of the API.
With more modern compilers, and
Squid enforcing more strict types and error handling,
it is no longer compiling.

This is part 1 of 2 of the fixes to make the helper build
again, the scope is to update Win32 API callers so they
use the C++ version of the API

Examples of fixed errors:

    error: 'IADs' {aka 'struct IADs'} has no member named 'lpVtbl'
    error: 'VARIANT' {aka 'struct tagVARIANT'} has no member named 'n1'

4 months agoMinGW: Fix winldap.h detection (#2010)
Francesco Chemolli [Sun, 2 Mar 2025 21:24:42 +0000 (21:24 +0000)] 
MinGW: Fix winldap.h detection (#2010)

<winldap.h> requires <windows.h> to be previously
included in order to compile.

    configure:38466: checking for winldap.h
    wincrypt.h:5051:254: error: 'PSYSTEMTIME' has not been declared

4 months agoNoNewGlobals for OpenSSL-related structures (#2006)
Eduard Bagdasaryan [Fri, 28 Feb 2025 15:48:30 +0000 (15:48 +0000)] 
NoNewGlobals for OpenSSL-related structures (#2006)

These changes were anticipated in Bug 5390 fix (recent commit c565067):
https://bugs.squid-cache.org/show_bug.cgi?id=5390#c16

They eliminate all known OpenSSL-related globals:

* Security::ProxyOutgoingConfig
* Ssl::SquidUntrustedCerts
* Ssl::TheGeneratorRequests
* Ssl::TheGlobalContextStorage

Also applied AAA and range-based `for` loop upgrades to modified lines.

5 months agoCI: Do not classify "no failures" stats as test-build errors (#2001)
Francesco Chemolli [Mon, 24 Feb 2025 19:01:52 +0000 (19:01 +0000)] 
CI: Do not classify "no failures" stats as test-build errors (#2001)

CppUnit tests emit a lot of "FAIL: 0" and "XFAIL: 0" lines, which are
incorrectly classified as errors by the test-builds.sh. Filter these
messages out as they are not indicative of problems.

5 months agoCI: Revert recent "show build commands on terminal" change (#2000)
Francesco Chemolli [Mon, 24 Feb 2025 14:26:00 +0000 (14:26 +0000)] 
CI: Revert recent "show build commands on terminal" change (#2000)

GitHub Actions UI does not handle large amounts of console output with
collapsable `::group::` sections well.
For example, UI may truncate console output if a collapsable `::group::`
section gets too many log lines. In some cases, GitHub does not report
truncation at all, resulting in misleading console output. In other, UI
warns: "This step has been truncated due to its large size. Download the
full logs from the menu once the workflow run has completed."

This change reverts recent commit e5a66fc26d.

5 months agoFix GCC v13 LTO build [-Walloc-size-larger-than=] (#1929)
Julien [Sun, 23 Feb 2025 20:06:06 +0000 (20:06 +0000)] 
Fix GCC v13 LTO build [-Walloc-size-larger-than=] (#1929)

    store/Disks.cc:690: error: argument 1 value 18446744073709551615
        exceeds maximum object size 9223372036854775807
        [-Werror=alloc-size-larger-than=]
    const auto tmp = new SwapDir::Pointer[swap->n_allocated];

    pconn.cc:43:53: error: argument 1 value 18446744073709551615 ...
    theList_ = new Comm::ConnectionPointer[capacity_];

Tested on Ubuntu 24.04 and GCC v13.2.0.

5 months agoNoNewGlobals for iocb_table (#1998)
Eduard Bagdasaryan [Thu, 20 Feb 2025 13:35:33 +0000 (13:35 +0000)] 
NoNewGlobals for iocb_table (#1998)

During shutdown, iocb_table global was deleted, but the corresponding
cleanup code ignored some IoCallback members, triggering misleading
Ipc::UdsSender memory leak reports from Valgrind.

This change uses NoNewGlobals design to address the above problem, but
this code needs a lot more refactoring to address other old problems
associated with Comm initialization.

5 months agoCI: show build commands on terminal (#1965)
Francesco Chemolli [Sun, 16 Feb 2025 23:46:20 +0000 (23:46 +0000)] 
CI: show build commands on terminal (#1965)

test-builds.sh records build output in per-layer logs, and we are
already collecting those logs. Send build output to standard output as
well, to expose it in GitHub Actions user interface. This new detailed
output is enclosed in a "layer ... output" group to keep emphasizing
problematic output lines extracted by test-builds.sh.

Also enclose MacOS build setup output in a group, for similar reasons.

5 months agoNoNewGlobals for cbdata_htable (#1991)
Eduard Bagdasaryan [Wed, 5 Feb 2025 12:58:59 +0000 (12:58 +0000)] 
NoNewGlobals for cbdata_htable (#1991)

This fix also reduces memory leak false positives
reported by Valgrind.

5 months agoAdd v8 release notes
Francesco Chemolli [Sun, 2 Feb 2025 23:34:10 +0000 (23:34 +0000)] 
Add v8 release notes

5 months agoMaster is now v8
Francesco Chemolli [Sun, 2 Feb 2025 23:26:16 +0000 (23:26 +0000)] 
Master is now v8

5 months agoPrep for v7.0.1 (#1988)
Francesco Chemolli [Sun, 2 Feb 2025 15:49:05 +0000 (15:49 +0000)] 
Prep for v7.0.1 (#1988)

Add v7.0.1 ChangeLog section.
Incorporate changes from v6.8 to v6.13 (inclusive).

6 months agoFix eCAP build broken by Adaptation::Config::metaHeaders change (#1987)
Eduard Bagdasaryan [Tue, 21 Jan 2025 13:19:47 +0000 (13:19 +0000)] 
Fix eCAP build broken by Adaptation::Config::metaHeaders change (#1987)

2024 master/v7 commit 984577ac replaced Adaptation::Config::metaHeaders
data member with a function but did not update metaHeaders users in eCAP
code.

6 months agoFix GCC v14 [-Wanalyzer-null-dereference] warnings in Kerberos (#1983)
huaraz [Fri, 17 Jan 2025 17:08:18 +0000 (17:08 +0000)] 
Fix GCC v14 [-Wanalyzer-null-dereference] warnings in Kerberos (#1983)

    src/acl/external/kerberos_ldap_group/support_sasl.cc:190:17: error:
    dereference of NULL 'defs' [CWE-476] [-Wanalyzer-null-dereference]

    src/auth/negotiate/kerberos/negotiate_kerberos_pac.cc:235:19: error:
    dereference of NULL 'Rids' [CWE-476] [-Wanalyzer-null-dereference]

6 months agoBug 5091: document that changes to workers require restart (#1980)
Francesco Chemolli [Wed, 15 Jan 2025 07:49:13 +0000 (07:49 +0000)] 
Bug 5091: document that changes to workers require restart (#1980)

6 months agoBug 5481: Fix GCC v14 build [-Wmaybe-uninitialized] (#1982)
Alex Rousskov [Tue, 14 Jan 2025 19:22:36 +0000 (19:22 +0000)] 
Bug 5481: Fix GCC v14 build [-Wmaybe-uninitialized] (#1982)

Successful xstrtoui(start,end) calls do initialize `end`, but we can
polish this code and avoid triggering that GCC warning at the same time.

6 months agoImprove Tunnel Server RESPONSE dumps (#1975)
Eduard Bagdasaryan [Mon, 13 Jan 2025 19:53:36 +0000 (19:53 +0000)] 
Improve Tunnel Server RESPONSE dumps (#1975)

Level-2 "Tunnel Server RESPONSE:..." debugs() incorrectly assumed that
its readBuf parameter contained hdr_sz header bytes. In reality, by the
time code reached that debugs(), readBuf no longer had any header bytes
(and often had no bytes at all). Besides broken header dumps, this bug
could lead to problems that Valgrind reports as "Conditional jump or
move depends on uninitialised value" in DebugChannel::writeToStream().

This fix mimics HttpStateData::processReplyHeader() reporting code,
including its known problems. Future changes should address those
problems and reduce code duplication across at least ten functions
containing similar "decorated" level-2 message dumps.

6 months agoBug 5311: clarify configuration byte units (#1979)
Francesco Chemolli [Thu, 9 Jan 2025 14:58:13 +0000 (14:58 +0000)] 
Bug 5311: clarify configuration byte units (#1979)

6 months agoBug 5093: List http_port params that https_port/ftp_port lack (#1977)
Francesco Chemolli [Tue, 7 Jan 2025 23:20:48 +0000 (23:20 +0000)] 
Bug 5093: List http_port params that https_port/ftp_port lack (#1977)

To avoid documentation duplication, current https_port and ftp_port
directive descriptions reference http_port directive instead of
detailing their own supported parameters. For https_port, this solution
creates a false impression that the directive supports all http_port
options. Our ftp_port documentation is better but still leaves the
reader guessing which options are actually supported.

This change starts enumerating http_port configuration parameters that
ftp_port and https_port directives do _not_ support. Eventually, Squid
should reject configurations with unsupported listening port options.

6 months agoCI: Add workflow_dispatch to workflows that still lack it (#1976)
Francesco Chemolli [Tue, 31 Dec 2024 23:18:39 +0000 (23:18 +0000)] 
CI: Add workflow_dispatch to workflows that still lack it (#1976)

Manually triggering a workflow rerun is handy when troubleshooting. Our
coverity-scan.yaml workflow already has a workflow_dispatch trigger.

6 months agoWork around some mgr:forward accounting/reporting bugs (#1969)
Alex Rousskov [Tue, 31 Dec 2024 21:59:05 +0000 (21:59 +0000)] 
Work around some mgr:forward accounting/reporting bugs (#1969)

In modern code, FwdReplyCodes[0][i] is usually zero because n_tries is
usually at least one at logReplyStatus() call time. This leads to
mgr:forward report showing nothing but table heading (i.e. no stats)

Also improve `try#N` heading:data match by skipping FwdReplyCodes[0]
reporting (there is still no `try#0` heading) and adding a previously
missing `try#9` heading

6 months agoClarify --enable-ecap failure on missing shared library support (#1968)
Alex Rousskov [Tue, 31 Dec 2024 20:40:46 +0000 (20:40 +0000)] 
Clarify --enable-ecap failure on missing shared library support (#1968)

    checking if libtool supports shared libraries... no
    checking whether to build shared libraries... no
    configure: error: eCAP support requires loadable modules.
        Please do not use --disable-shared with --enable-ecap.

After 2022 commit 5a2409b7, our advice for handling the above error
became misleading in environments that do not --disable-shared
explicitly but lack shared libraries support for other reasons

6 months agoDo not lookup IP addresses of X509 certificate subject CNs (#1967)
Alex Rousskov [Tue, 31 Dec 2024 19:22:21 +0000 (19:22 +0000)] 
Do not lookup IP addresses of X509 certificate subject CNs (#1967)

A true-vs-false `nodns` parameter value bug in a recent commit 22b2a7a0
caused, in some environments, significant startup delays and/or runtime
stalls because getaddrinfo(3) performed blocking DNS lookups when
parsing common names of X509 certificate subjects. Squid parses CNs when
loading configured and validating received certificates. Other side
effects may have included Squid-generated certificates having wrong
alternative subject names and/or wrong certificate validation results.

Negative names and context-disassociated boolean constants strike again!
Fortunately, associated problematic Ip::Address::lookupHostIP() will be
replaced when the existing Ip::Address::Parse() TODO is addressed.

6 months agoREQMOD stuck when adapted request (body) is not forwarded (#1966)
Alex Rousskov [Tue, 31 Dec 2024 17:27:40 +0000 (17:27 +0000)] 
REQMOD stuck when adapted request (body) is not forwarded (#1966)

Squid forwards request bodies using BodyPipe objects. A BodyPipe object
has two associated agents: producer and consumer. Those agents are set
independently, at different processing stages. If BodyPipe consumer is
not set, the producer may get stuck waiting for BodyPipe buffer space.
When producer creates a BodyPipe, it effectively relies on some code
somewhere to register a consumer (or declare a registration failure),
but automatically tracking that expectation fulfillment is impractical

For REQMOD transactions involving adapted request bodies, including ICAP
204 transactions, Client::startRequestBodyFlow() sets body consumer. If
that method is not called, there will be no consumer, and REQMOD may get
stuck. Many `if` statements can block request forwarding altogether or
block a being-forwarded request from reaching that method. For example,
adapted_http_access and miss_access denials block request forwarding

Without REQMOD, request processing can probably get stuck for similar
lack-of-consumer reasons, but regular requests ought to be killed by
various I/O or forwarding timeouts. There are no such timeouts for those
REQMOD transactions that are only waiting for request body consumer to
clear adapted BodyPipe space (e.g., after all ICAP 204 I/Os are over).
Relying on timeouts is also very inefficient

For a `mgr:mem` observer, stuck REQMOD transactions look like a ModXact
memory leak. A `mgr:jobs` report shows ModXact jobs with RBS(1) status

6 months agoMaintenance: remove HERE (#1972)
Francesco Chemolli [Tue, 31 Dec 2024 14:58:49 +0000 (14:58 +0000)] 
Maintenance: remove HERE (#1972)

HERE has been deprecated for a while; there is only
one remaining instance of it.

7 months agoReport cache_peer context in probe and standby pool messages (#1960)
Eduard Bagdasaryan [Fri, 20 Dec 2024 20:56:36 +0000 (20:56 +0000)] 
Report cache_peer context in probe and standby pool messages (#1960)

The absence of the usual "current master transaction:..." detail in
certain errors raises "Has Squid lost the current transaction context?"
red flags:

    ERROR: Connection to peerXyz failed

In some cases, Squid may have lost that context, but for cache_peer TCP
probes, Squid has not because those probes are not associated with
master transactions. It is difficult to distinguish the two cases
because no processing context is reported.  To address those concerns,
we now report current cache_peer probing context (instead of just not
reporting absent master transaction context):

    ERROR: Connection to peerXyz failed
        current cache_peer probe: peerXyzIP

When maintaining a cache_peer standy=N connection pool, Squid has and
now reports both contexts, attributing messages to pool maintenance:

    ERROR: Connection to peerXyz failed
        current cache_peer standby pool: peerXyz
        current master transaction: master1234

The new PrecomputedCodeContext class handles both reporting cases and
can be reused whenever the cost of pre-computing detailCodeContext()
output is acceptable.

7 months agoCI: Add OpenBSD build tests for staged commits (#1964)
Francesco Chemolli [Tue, 17 Dec 2024 15:06:29 +0000 (15:06 +0000)] 
CI: Add OpenBSD build tests for staged commits (#1964)

Use a GitHub-hosted VM to create OpenBSD test environment. Requires
GitHub repository configuration that permits the use of
`vmactions/openbsd-vm@v1` Action.

We have not enabled ccache optimization for OpenBSD tests because we
do not know how to copy updated ccache files from VM back into runner.

7 months agoCI: Add GitHub Actions workflow for periodic Coverity Scan (#1958)
Francesco Chemolli [Mon, 16 Dec 2024 18:19:18 +0000 (18:19 +0000)] 
CI: Add GitHub Actions workflow for periodic Coverity Scan (#1958)

Implement a weekly scheduled GitHub Actions workflow to run Coverity
Scan (i.e. cov-build). Currently, we run Coverity Scan using Jenkins.

The new job uses the Squid Project pre-made docker image because
installing the tools required to use free Coverity Scan service cannot
be easily automated at the moment.

The job only runs for the official Squid Project repository.

7 months agoPortability: remove explicit check for libdl (#1963)
Francesco Chemolli [Mon, 16 Dec 2024 14:49:55 +0000 (14:49 +0000)] 
Portability: remove explicit check for libdl (#1963)

OpenBSD does not have libdl, as it has dlopen() in libc.
It is not really needed, and force-requiring the presence of libdl
causes ./configure to fail on openbsd:

    checking for dlopen in -ldl... no
    configure: error: Required library 'dl' not found

7 months agoFix CachePeer.cc:101 "!tcp_up" assertion after cache_peer death (#1962)
Eduard Bagdasaryan [Fri, 13 Dec 2024 15:27:13 +0000 (15:27 +0000)] 
Fix CachePeer.cc:101 "!tcp_up" assertion after cache_peer death (#1962)

getFirstUpParent() could return a down (or otherwise disqualified)
cache_peer. Broken by 2023 commit 2e24d0b.

Refactored function structure to improve code quality, partially
addressing "use our return/reporting style" TODO in getDefaultParent().

7 months agoMaintenance: Remove change-minimizing MemObject::Io hack (#1957)
Shailesh [Fri, 13 Dec 2024 09:35:27 +0000 (09:35 +0000)] 
Maintenance: Remove change-minimizing MemObject::Io hack (#1957)

Added in 2018 commit 4310f8b0.

7 months agoRemove GNU'ism in release notes Makefile (#1959)
Amos Jeffries [Tue, 10 Dec 2024 08:40:39 +0000 (08:40 +0000)] 
Remove GNU'ism in release notes Makefile (#1959)

Targets containing % wildcard is a GNU automake custom feature.

7 months agoMaintenance: use void return for commSetTimeout functions (#1956)
Francesco Chemolli [Mon, 2 Dec 2024 22:47:57 +0000 (22:47 +0000)] 
Maintenance: use void return for commSetTimeout functions (#1956)

On some systems (e.g. Linux), time_t is a signed 64-bit integer. int may
be a signed 32-bit integer, resulting in possible truncation errors.

Detected by Coverity. CID 1547031: Use of 32-bit time_t (Y2K38_SAFETY).

7 months agoNoNewGlobals for Comm::AcceptLimiter::Instance_ (#1954)
Francesco Chemolli [Sun, 1 Dec 2024 11:34:00 +0000 (11:34 +0000)] 
NoNewGlobals for Comm::AcceptLimiter::Instance_ (#1954)

Inspired by Coverity. CID 1441988: Useless call (USELESS_CALL).

7 months agoMaintenance: improved stat5minClientRequests() naming (#1951)
Eduard Bagdasaryan [Sun, 1 Dec 2024 01:48:10 +0000 (01:48 +0000)] 
Maintenance: improved stat5minClientRequests() naming (#1951)

stat5minClientRequests() was to meant to return the number of recent
client requests. However, the function did not provide implied 5 minute
precision. It returned, roughly speaking, the number of requests during
the last 0-6 minutes. The new, less strict function name and boolean
type avoid this false precision implication.

Also removed unused stat5minCPUUsage().

7 months agoBug 5390: Non-POD SquidConfig::ssl_client::sslContext exit crash (#1952)
Eduard Bagdasaryan [Sat, 30 Nov 2024 03:04:33 +0000 (03:04 +0000)] 
Bug 5390: Non-POD SquidConfig::ssl_client::sslContext exit crash (#1952)

Squid may crash when the SquidConfig global is auto-destructed after
main() ends. Since SquidConfig global is used by cleanup code, we should
keep its fields alive, essentially emulating "No New Globals" policy
effects. This surgical fix will be followed up with more changes to
address general OpenSSL cleanup problems exposed by this bug.

This bug fix facilitates backporting by using FuturePeerContext shim.

8 months agoTreat responses to collapsed requests as fresh (#1927)
Alex Rousskov [Wed, 27 Nov 2024 02:32:02 +0000 (02:32 +0000)] 
Treat responses to collapsed requests as fresh (#1927)

Squid transactions involving collapsed requests receive their responses
as Store cache hits. Cache hits undergo mandatory freshness validation
checks and, if those checks detect a stale response, may trigger
revalidation (e.g., an If-Modified-Since request to the origin server).
This logic results in a still-being-delivered-to-Squid response
triggering its own revalidation if that response is deemed stale on
arrival by collapsed request (e.g., has a Cache-Control: max-age=0
header field).

HTTP RFC 9111 Section 4.7 briefly mentions collapsed requests but is
ambiguous with regard to validation of responses to collapsed requests.
IETF HTTP Working Group chair believes that specs are unclear, and that
these responses should not be treated as cache hits (in primary cases):
https://lists.w3.org/Archives/Public/ietf-http-wg/2024JanMar/0095.html

This change follows the above advice and prevents arguably excessive
freshness checks for responses to collapsed requests. This change is
collapsed-forwarding specific: It does not prevent freshness checks for
responses that were, at the time of a hit request, either fully cached
or still receiving response body bytes.

After this change, clientReplyContext can no longer collapse twice, once
after initial Store lookup and then again during refresh, because the
first collapse now precludes refresh.

8 months agoDo not TLS close_notify when resetting a TCP connection (#1944)
Eduard Bagdasaryan [Sun, 24 Nov 2024 20:19:03 +0000 (20:19 +0000)] 
Do not TLS close_notify when resetting a TCP connection (#1944)

8 months agoRemove krb5_get_max_time_skew portability hack (#1875)
Amos Jeffries [Sat, 23 Nov 2024 18:39:06 +0000 (18:39 +0000)] 
Remove krb5_get_max_time_skew portability hack (#1875)

This function has been available in Kerberos libraries
for over ten years now.

8 months agoSimplified quick_abort_pct code and improved its docs (#1921)
Shailesh Vashishth [Sat, 23 Nov 2024 13:41:03 +0000 (13:41 +0000)] 
Simplified quick_abort_pct code and improved its docs (#1921)

Instead of ignoring quick_abort_pct settings that would, together with
other conditions, abort a pending download of a 99-byte or smaller
response, Squid now honors quick_abort_pct for all response sizes. Most
Squids are not going to be affected by this change because default
quick_abort_min settings (16KB) prevent aborts of 99-byte responses even
before quick_abort_pct is checked.

Due to conversion from integer to floating point math, this change may
affect responses larger than 99 bytes as well, but these effects ought
to be limited to cases where the decision is based on a tiny difference
(e.g., receiving 1% more bytes would have triggered full download). In
most such cases, the decision could probably go either way due to
response header size fluctuations anyway.

Also updated quick_abort_pct documentation, primarily to clarify a
misleading statement: Squid did not and does not treat 16KB or smaller
responses specially in this context. The original statement was probably
based on quick_abort_min _default_ setting of 16KB, but statement
phrasing and placement hid that connection.

8 months agoDocumentation: Fixed reference to doc/debug-messages.dox file (#1953)
Alex Rousskov [Fri, 22 Nov 2024 23:08:29 +0000 (23:08 +0000)] 
Documentation: Fixed reference to doc/debug-messages.dox file (#1953)

Reported by Matus UHLAR on the squid-users mailing list.
Broken since inception in 2021 commit c59baaa8.

8 months agoReplace MEM_DREAD_CTRL pool with MEMPROXY class pool (#1924)
Amos Jeffries [Thu, 21 Nov 2024 10:45:23 +0000 (10:45 +0000)] 
Replace MEM_DREAD_CTRL pool with MEMPROXY class pool (#1924)

Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.

8 months agoCI: Add FreeBSD build tests for staged commits (#1948)
Francesco Chemolli [Wed, 20 Nov 2024 16:00:30 +0000 (16:00 +0000)] 
CI: Add FreeBSD build tests for staged commits (#1948)

Use a GitHub-hosted VM to create FreeBSD test environment.

We have not enabled ccache optimization for FreeBSD tests because we
do not know how to copy updated ccache files from VM back into runner.

8 months agoRefactor peerRefreshDNS() to clarify its (void*)1 logic (#1950)
Eduard Bagdasaryan [Tue, 19 Nov 2024 22:43:59 +0000 (22:43 +0000)] 
Refactor peerRefreshDNS() to clarify its (void*)1 logic (#1950)

Creating a raw pointer with 1 as an address/value raised red flags, and
it was difficult to interpret tricky peerRefreshDNS() logic correctly.

8 months agoBug 5363: Handle IP-based X.509 SANs better (#1793)
Tony Walker [Sat, 16 Nov 2024 22:10:39 +0000 (22:10 +0000)] 
Bug 5363: Handle IP-based X.509 SANs better (#1793)

Most X.509 Subject Alternate Name extensions encountered by Squid are
based on DNS domain names. However, real-world servers (including
publicly available servers that use vanity IP addresses) also use
IP-based SANs. Squid mishandled IP-based SANs in several ways:

* When generating certificates for servers targeted by their IP
  addresses, addAltNameWithSubjectCn() used that target IP as a
  DNS-based SAN, resulting in a frankenstein DNS:[ip] SAN value that
  clients ignored when validating a Squid-generated certificate.

* When validating a received certificate, Squid was ignoring IP-based
  SANs. When Subject CN did not match the requested IP target, Squid
  only looked at DNS-based SANs, incorrectly failing validation.

* When checking certificate-related ACLs like ssl::server_name,
  matchX509CommonNames() ignored IP-based SANs, not matching
  certificates containing ACL-listed IP addresses.

Squid now recognizes and generates IP-based SANs.

Squid now attempts to match IP-based SANs with ACL-listed IP addresses,
but the success of that attempt depends on whether ACL IP parameters are
formatted the same way inet_ntop(3) formats those IP addresses: Matching
is still done using c-string/domain-based ::matchDomainName() (for
ssl::server_name) and string-based regexes (for ssl::server_name_regex).
Similar problems affect dstdomain and dstdomain_regex ACLs. A dedicated
fix is needed to stop treating IPs as domain names in those contexts.

This change introduces partial support for preserving IP-vs-domain
distinction in parsed/internal Squid state rather than converting both
to a string and then assuming that string is a DNS domain name.

8 months agoext_time_quota_acl: restore debug level feature and argument (#1936)
Francesco Chemolli [Wed, 13 Nov 2024 23:34:15 +0000 (23:34 +0000)] 
ext_time_quota_acl: restore debug level feature and argument (#1936)

Commit 4878cfe567 modernized the time quota ACL helper,
but removed the '-d debuglevel' feature, reducing it to a binary
toggle.

8 months agoAnnotate PoolMalloc memory in valgrind builds (#1946)
Alex Rousskov [Wed, 13 Nov 2024 18:16:08 +0000 (18:16 +0000)] 
Annotate PoolMalloc memory in valgrind builds (#1946)

MemPoolMalloc code (i.e. memory_pools code used by default) was missing
VALGRIND_MAKE_MEM_*() calls. Similar calls do exist in MemPoolChunked
code (i.e. code enabled by setting MEMPOOLS environment variable to 1).

Even with these markings, "memory_pools on" configuration is still not
quite compatible with Valgrind leak reporting suppressions: In some
cases, Valgrind may still incorrectly suppress leak reporting (or report
leaks that should have been suppressed) because Valgrind associates leak
suppressions with memory _allocators_ while buggy code may leak memory
allocated by others. The long-term solution (if it exists) requires
upgrading these markings to VALGRIND_MEMPOOL_*() API targeting memory
pools, but that requires a serious effort, especially when dealing with
MemPoolChunked complexities. The added markings help detect non-leak
violations and improve PoolMalloc/MemPoolChunked code symmetry.

8 months agoUpdate HTTP status codes (#1942)
Amos Jeffries [Tue, 12 Nov 2024 00:00:38 +0000 (00:00 +0000)] 
Update HTTP status codes (#1942)

Also remove comments that replicate what the registry documents.

8 months agoCI: Cancel obsolete concurrent GitHub Actions workflow jobs (#1940)
Francesco Chemolli [Mon, 11 Nov 2024 18:24:25 +0000 (18:24 +0000)] 
CI: Cancel obsolete concurrent GitHub Actions workflow jobs (#1940)

If a new push happens to a staging branch or a PR branch, continuing to
run now-obsolete tests is pointless and wasteful. However, we do want to
finish any jobs running on previous master branch commits, so that every
master branch commit has full test results.

8 months agoCI: Use ccache-action repo maintained by its original author (#1943)
Alex Rousskov [Mon, 11 Nov 2024 16:17:09 +0000 (16:17 +0000)] 
CI: Use ccache-action repo maintained by its original author (#1943)

    squid-cache/ccache-action@v1.2.14 is not allowed to be used in ...

For recent commit 627cca6d, we cloned hendrikmuhs/ccache-action
repository because GitHub Actions prohibited usage of that repository in
Squid Project CI tests. Cloning worked around that restriction, but we
did not realize that there are other solutions, and that cloning forces
all other Squid repositories to either clone squid-cache/ccache-action
or permit squid-cache/ccache-action execution by other means.

To reduce the number of obscure repositories Squid Project and others
have to deal with, it is better to adjust repository configuration to
allow well-known hendrikmuhs/ccache-action in "Actions permissions" at
https://github.com/.../squid/settings/actions

8 months agoReduce SO_LINGER code duplication (#1941)
Eduard Bagdasaryan [Fri, 8 Nov 2024 13:23:28 +0000 (13:23 +0000)] 
Reduce SO_LINGER code duplication (#1941)

Also prevent nil connection pointer dereference and setsockopt() calls
with negative FD in comm_reset_close() and old_comm_reset_close().
It is unknown whether such bugs could be triggered before these changes.

Also removed fde::flags::nolinger as unused since 1999 commit 2391a162.

8 months agoCI: Add ccache to GitHub Actions build tests (#1935)
Francesco Chemolli [Tue, 5 Nov 2024 22:21:26 +0000 (22:21 +0000)] 
CI: Add ccache to GitHub Actions build tests (#1935)

Depending on an individual build test, adding ccache currently reduces
build test time 35-60% (e.g., from 6m42s to 4m18s and from 17m to 7m).

Problematic ccache entries can be removed using GitHub API. For example:

    gh -R squid-cache/squid cache list
    gh -R squid-cache/squid cache delete <cache-key|"all">

Do not add ccache to MacOS build tests because I had mixed experience
with that optimization on that OS.

8 months agoFix systemd startup sequence to require active Local Filesystem (#1937)
Amos Jeffries [Tue, 5 Nov 2024 16:46:41 +0000 (16:46 +0000)] 
Fix systemd startup sequence to require active Local Filesystem (#1937)

Squid requires Local Filesystem to be active. While uncommon, it
may in some cases be incomplete or delayed. Ensure that the
dependency is explicitly listed to prevent failure from Squid
early initialization.

This change resolves Debian Bug 956581:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=956581

8 months agoNil request dereference in ACLExtUser and SourceDomainCheck ACLs (#1931)
Eduard Bagdasaryan [Tue, 5 Nov 2024 14:21:19 +0000 (14:21 +0000)] 
Nil request dereference in ACLExtUser and SourceDomainCheck ACLs (#1931)

ACLExtUser-based ACLs (i.e. ext_user and ext_user_regex) dereferenced a
nil request pointer when they were used in a context without a request
(e.g., when honoring on_unsupported_protocol).

SourceDomainCheck-based ACLs (i.e. srcdomain and srcdom_regex) have a
similar bug, although we do not know whether broken slow ACL code is
reachable without a request (e.g., on_unsupported_protocol tests cannot
reach that code until that directive starts supporting slow ACLs). This
change does not start to require request presence for these two ACLs to
avoid breaking any existing configurations that "work" without one.

8 months agoBug 5383: handleNegotiationResult() level-2 debugs() crash (#1856)
Alex Rousskov [Sun, 3 Nov 2024 08:14:55 +0000 (08:14 +0000)] 
Bug 5383: handleNegotiationResult() level-2 debugs() crash (#1856)

Writing a nil c-string to an std::ostream object results in undefined
behavior. When Security::IoResult::errorDescription is nil, that UB
leads to crashes for some STL implementations. These changes avoid UB
while making higher-level reporting code simpler and safer.

This change alters affected level-1 ERROR test lines a little, including
removing duplicated connection information from clientNegotiateSSL()
message (cache_log_message id=62). That duplication existed because
Squid reports the same Connection info automatically via CodeContext.

New WithExtras() mechanism may be useful for other "low-level debugging
and level-0/1 details for admins ought to differ" cases as well. Today,
the only known debugging context for Security::IoResult is
Security::PeerConnector::suspendNegotiation(), but that is likely to
change as we upgrade TLS callers to library-independent wrappers beyond
the current Security::Accept() and Security::Connect() pair.

8 months agoReport all refreshCheck() outcomes and entry gist (#1932)
Alex Rousskov [Fri, 1 Nov 2024 23:38:08 +0000 (23:38 +0000)] 
Report all refreshCheck() outcomes and entry gist (#1932)

All other refreshCheck() return statements already have a debugs().
Reporting StoreEntry gist helps find the right calls in busy proxy logs.

8 months agoDisplay Linux variant at ./configure time (#1928)
Francesco Chemolli [Fri, 1 Nov 2024 14:11:41 +0000 (14:11 +0000)] 
Display Linux variant at ./configure time (#1928)

Linux standards offer an /etc/os-release file containing
information about the operating system.

Parse that file and print an identifier of the Linux version at
configure time.

8 months agoCI: Replace Fedora 39 with 41 (#1926)
Francesco Chemolli [Wed, 30 Oct 2024 12:35:57 +0000 (12:35 +0000)] 
CI: Replace Fedora 39 with 41 (#1926)

Fedora 39 will become EOL on 2024-11-12. Fedora 41 has been released.

9 months agoFtp::Gateway may segfault in level-3 double-complete debugs() (#1923)
Eduard Bagdasaryan [Mon, 28 Oct 2024 09:51:42 +0000 (09:51 +0000)] 
Ftp::Gateway may segfault in level-3 double-complete debugs() (#1923)

Ftp::Gateway::completeForwarding() must check data.conn pointer before
dereferencing it. Long-term, we should improve Comm::ConnectionPointer
printing to safely report Connection::id (where available). This minimal
fix just mimics existing Ftp::Relay::abortOnData() solution.

9 months agoUse GitHub Actions for more OS-specific build tests (#1917)
Francesco Chemolli [Fri, 25 Oct 2024 17:36:26 +0000 (17:36 +0000)] 
Use GitHub Actions for more OS-specific build tests (#1917)

Test staged commits using some of our docker images from Jenkins tests.
The added tests (see staged.yaml) were easier to adopt; we are also
working on enabling GitHub Actions for FreeBSD and some other images.

We do not run these added tests for PR commits because existing Ubuntu
tests (defined in pr.yaml) already expose the vast majority of build
problems, and we are worried that running a lot more tests for each PR
push event would consume too much GitHub resources and significantly
increase noise in PR checks summary, obscuring often-easier-to-handle
failures detected by Ubuntu tests.

Also postpone MacOS tests until PR staging. On GitHub, MacOS runners are
x10 more expensive than Linux runners. We use cheaper runners for fast
feedback while still checking MacOS build before each merged commit.

This change does not increase GitHub CI wait time for PR push tests
because those checks are dominated by the unchanged 35min CodeQL-tests
job. However, it reduces total CPU time used (from 2h 30m to 2h) because
we no longer perform 3 MacOS tests for PR push events.

This change doubles GitHub CI wait time for staged commit tests (from
35m to 1h) and drastically increases total CPU time used (from 2h to
17h), primarily due to 84 added docker-based linux-distros checks.

GitHub does not provide any riscv64 runners and free Linux/arm64
runners. Our initial attempts to virtualize Linux/arm64 tests on
MacOS/arm64 runners and to make virtualized FreeBSD and OpenBSD tests
work on Linux/x64 were not successful. Thus, we still rely on Jenkins
for Linux/riscv64, Linux/arm64, FreeBSD/x64, and OpenBSD/x64 tests.

9 months agoReplace MEM_DWRITE_Q pool with MEMPROXY class pool (#1873)
Amos Jeffries [Sat, 19 Oct 2024 09:44:07 +0000 (09:44 +0000)] 
Replace MEM_DWRITE_Q pool with MEMPROXY class pool (#1873)

Replacing memory allocate and destruct with C++ new/delete
and in-class initialization.

9 months agoMaintenance: Fix EventScheduler::timeRemaining() comment (#1916)
Alex Rousskov [Sun, 13 Oct 2024 21:33:34 +0000 (21:33 +0000)] 
Maintenance: Fix EventScheduler::timeRemaining() comment (#1916)

9 months agoBug 5449: Ignore SP and HTAB chars after chunk-size (#1914)
uhliarik [Fri, 11 Oct 2024 03:31:19 +0000 (03:31 +0000)] 
Bug 5449: Ignore SP and HTAB chars after chunk-size (#1914)

Prior to 2023 commit 951013d0, Squid accepted Transfer-Encoding chunks
with chunk-size followed by spaces or tabs (before CRLF). This HTTP
syntax violation was allowed to address Bug 4492 (fixed in 2017 commit
26f0a359). This change restores that fix functionality. FWIW, our
research shows that nginx and httpd also accept similar input.

9 months agoMaintenance: Remove unused file_write_mbuf() (#1913)
Amos Jeffries [Thu, 10 Oct 2024 05:47:43 +0000 (05:47 +0000)] 
Maintenance: Remove unused file_write_mbuf() (#1913)

9 months agoMaintenance: Remove unused clientBeginRequest() (#1910)
Alex Rousskov [Tue, 8 Oct 2024 16:03:03 +0000 (16:03 +0000)] 
Maintenance: Remove unused clientBeginRequest() (#1910)

This ESI-only function became unused since recent commit 5eb89ef3. It
has enough problematic code and comments to justify dedicated removal.

9 months agoFix validation of Digest auth header parameters (#1906)
Francesco Chemolli [Mon, 7 Oct 2024 08:13:17 +0000 (08:13 +0000)] 
Fix validation of Digest auth header parameters (#1906)

Insufficient validation of Digest authentication parameters resulted in
a DigestCalcHA1() call that dereferenced a nil pointer.

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ where it was filed as
"strlen(NULL) Crash Using Digest Authentication".

9 months agoProhibit bad --enable-linux-netfilter combinations (#1893)
Alex Rousskov [Sat, 5 Oct 2024 16:18:33 +0000 (16:18 +0000)] 
Prohibit bad --enable-linux-netfilter combinations (#1893)

Since 2009 commit 51f4d36b, Squid declared that "all transparent build
options are independent, and may be used in any combination". That
declaration was accurate from a "successful build" point of view, but
Ip::Intercept::LookupNat() (and its precursors) started mishandling at
least two combinations of options as detailed below.

LookupNat() tries up to four lookup methods (until one succeeds):

1. NetfilterInterception(): --enable-linux-netfilter; SO_ORIGINAL_DST
2. IpfwInterception(): --enable-ipfw-transparent; getsockname()
3. PfInterception(): --enable-pf-transparent; getsockname() or /dev/pf
4. IpfInterception(newConn): --enable-ipf-transparent; ioctl(SIOCGNATL)

The first method -- NetfilterInterception() -- fails to look up the true
destination address of an intercepted connection when, for example, the
client goes away just before the lookup. In those (relatively common in
busy environments) cases, the intended destination address cannot be
obtained via getsockname(), but LookupNat() proceeds calling other
methods, including the two methods that may rely on getsockname().

Methods 2 and 3 may rely on a previous getsockname() call to provide
true destination address, but getsockname() answers are not compatible
with what NetfilterInterception() must provide -- the destination
address returned by getsockname() is Squid's own http(s)_port address.
When Squid reaches problematic code now encapsulated in a dedicated
UseInterceptionAddressesLookedUpEarlier() function, Squid may end up
connecting to its own http(s)_port! Such connections may cause
forwarding loops and other problems. In SslBump deployments, these loops
form _before_ Forwarded-For protection can detect and break them.

These problems can be prevented if an admin does not enable incompatible
combinations of interception lookup methods. The relevant code is
correctly documented as "Trust the user configured properly", but that
statement essentially invalidates our "may be used in any combination"
design assertion and leads to runtime failures when user configured
improperly. Those runtime failures are difficult to triage because they
lack signs pointing to a build misconfiguration.

This change bans incompatible NetfilterInterception()+getsockname()
combinations at compile time: Squid ./configured with
--enable-linux-netfilter cannot use --enable-ipfw-transparent or
(--enable-pf-transparent --without-nat-devpf).

TODO: Ban incompatible combinations at ./configure time as well!

We have considered and rejected an alternative solution where all
./configure option combinations are still allowed, but LookupNat()
returns immediately on NetfilterInterception()/SO_ORIGINAL_DST failures.
That solution is inferior to build-time bans because an admin may think
that their Squid uses other configured lookup method(s) if/as needed,
but Squid would never reach them in --enable-linux-netfilter builds.

The only viable alternative is to specify lookup methods in squid.conf,
similar to the existing "tproxy" vs. "intercept" http(s)_port modes. In
that case, squid.conf will be validated for incompatible method
combinations (if combinations are supported at all), and LookupNat()
will only use configured method(s).

9 months agoDo not mark successful FTP PUT entries with ENTRY_BAD_LENGTH (#1904)
Eduard Bagdasaryan [Mon, 23 Sep 2024 09:31:36 +0000 (09:31 +0000)] 
Do not mark successful FTP PUT entries with ENTRY_BAD_LENGTH (#1904)

Since 2021 commit ba3fe8d, we explicitly mark complete responses and
treat all other responses as truncated. That commit missed a case where
the FTP server responds with 226 or 250 code after receiving the upload.
The bug affects HTTP PUT requests using ftp URI scheme.

Incorrect truncation marking adds an unwanted WITH_CLIENT %err_detail to
ERR_FTP_PUT_CREATED transaction records in access.log:

    201 PUT ftp://... ERR_FTP_PUT_CREATED/FTP_REPLY_CODE=226+WITH_CLIENT

Fixed code logs:

    201 PUT ftp://... ERR_FTP_PUT_CREATED/FTP_REPLY_CODE=226

10 months agoBug 5428: Warn if pkg-config is not found (#1902)
Francesco Chemolli [Mon, 23 Sep 2024 02:09:29 +0000 (02:09 +0000)] 
Bug 5428: Warn if pkg-config is not found (#1902)

Squid builds without pkg-config, but results are likely to surprise
administrators because many optional features will not be
default-enabled despite properly installed libraries.

10 months agoRemove Edge Side Include (ESI) protocol (#1905)
Amos Jeffries [Sat, 21 Sep 2024 20:27:11 +0000 (20:27 +0000)] 
Remove Edge Side Include (ESI) protocol (#1905)

ESI feature has a number of bugs and security vulnerabilities.
It is also rarely used and a survey of active community members
has not revealed a need to keep maintaining this code.

10 months agoUse ERR_ACCESS_DENIED for HTTP 403 (Forbidden) errors (#1899)
Alex Rousskov [Fri, 20 Sep 2024 09:35:04 +0000 (09:35 +0000)] 
Use ERR_ACCESS_DENIED for HTTP 403 (Forbidden) errors (#1899)

... when request authentication fails. Do not use
ERR_CACHE_ACCESS_DENIED for those "permanent" errors.

Default ERR_CACHE_ACCESS_DENIED is meant for cases where the user is
likely to eventually gain access (e.g., by supplying credentials). Its
default text says "not currently allowed... until you have authenticated
yourself". When the error page was added in 1998 commit cb69b4c7 it was
only used for HTTP 407 errors. The same logic was preserved when that
code was refactored in 1999 commit 1cfdbcf0, but exceptions started to
creep in, perhaps accidentally, since 2011 when HTTP 403 case was added
in commit 2f1431ea that introduced USE_AUTH macro. 2011 commit 21512911
added a similar "not possible to authenticate" SslBump case.

Other HTTP 403 (Forbidden) cases already use ERR_ACCESS_DENIED or a
similar "permanent" error (e.g., ERR_FORWARDING_DENIED or ERR_TOO_BIG).

It is still possible to customize the returned error page via deny_info.

10 months agoFix ENTRY_ABORTED assertion in sendClientOldEntry() (#1903)
Alex Rousskov [Tue, 17 Sep 2024 22:04:21 +0000 (22:04 +0000)] 
Fix ENTRY_ABORTED assertion in sendClientOldEntry() (#1903)

    FATAL: assertion failed: client_side_reply.cc:392:
    "!EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)"

The replaced assertion was wrong because a stale entry may be aborted
while we are revalidating it. The exact real-world conditions that
triggered this assertion are unknown, but many conditions lead to
aborted entries. The assertion can be triggered in lab tests using a hit
transaction that collapses on a miss transaction that runs into storage
problems (e.g., a memory cache that ran out of usable space).

Also adjusted cacheHit() to avoid similar problems. We have not
reproduced them, but no code maintains the asserted invariant on the
cacheHit() path either. Moreover, async-called cacheHit() initiates
processExpired() that leads to problematic sendClientOldEntry() call, so
seeing an aborted StoreEntry at cacheHit() time is probably a matter of
sufficient concurrency levels and asynchronous callback delays.

10 months agoBug 5293: Security::CreateClientSession uses wrong TLS options (#1895)
Alex Rousskov [Tue, 10 Sep 2024 01:32:59 +0000 (01:32 +0000)] 
Bug 5293: Security::CreateClientSession uses wrong TLS options (#1895)

When establishing a TLS connection to an origin server _through_ a
cache_peer, Security::CreateClientSession() used cache_peer's
Security::PeerOptions instead of global ProxyOutgoingConfig (i.e.
tls_outgoing_options). Used cache_peer's PeerOptions are unrelated to
the (tunneled) TLS connection in question (and are currently empty
because Squid does not support TLS inside TLS -- the cache_peer accepts
plain HTTP connections).

This TLS context:options mismatch exists in both OpenSSL and GnuTLS
builds, but it currently does not affect OpenSSL builds where
CreateSession() gets TLS options from its Security::Context parameter
rather than its (unused) Security::PeerOptions parameter.

The mismatch exists on both PeekingPeerConnector/SslBump and
BlindPeerConnector code paths.

This minimal change pairs TLS context with its TLS options. Long-term,
the added FuturePeerContext shim (that stores references to independent
context and options objects) should be replaced with a PeerContext class
that encapsulates those two objects. We may also be able to avoid
re-computing GnuTLS context from options and to simplify code by
preventing PeerConnector construction in Squid builds that do not
support TLS. That refactoring should be done separately because it
triggers many changes unrelated to Bug 5293.

Also removed updateSessionOptions() from
PeekingPeerConnector::initialize() because Security::CreateSession(),
called by our parent initialize() method, already sets session options.
It is easier to remove that call/code than keep it up to date.
Security::BlindPeerConnector does not updateSessionOptions().

10 months agoLimit Server::inBuf growth (#1898)
Eduard Bagdasaryan [Mon, 9 Sep 2024 16:41:50 +0000 (16:41 +0000)] 
Limit Server::inBuf growth (#1898)

After a ReadNow() call, the buffer length must not exceed accumulation
limits (e.g., client_request_buffer_max_size). SBuf::reserve() alone
cannot reliably enforce those limits because it does not decrease SBuf
space; various SBuf manipulations may lead to excessive SBuf space. When
filled by ReadNow(), that space exceeds the limit.

This change uses documented CommIoCbParams::size trick to limit how much
Comm::ReadNow() may read, obeying SQUID_TCP_SO_RCVBUF (server-to-Squid)
and client_request_buffer_max_size (client-to-Squid) accumulation limit.

10 months agoBug 5417: An empty annotation value does not match (#1896)
Alex Rousskov [Thu, 5 Sep 2024 17:46:20 +0000 (17:46 +0000)] 
Bug 5417: An empty annotation value does not match (#1896)

Helpers may return annotations with empty values:

    OK team_=""

A note ACL may be configured to match an annotation with an empty value:

    configuration_includes_quoted_values on
    acl emptyTeam note team_ ""

However, that emptyTeam ACL did not match the above helper annotation:

* AppendTokens() split an empty annotation value into an empty vector
  instead of a vector with a single empty entry. That "never match an
  empty value received from the helper" bug was probably introduced in
  2017 commit 75d47340 when it replaced an "always try to match an empty
  value, even when it was not received from the helper" bug in
  ACLNoteStrategy::matchNotes().

* ACLStringData::match(SBuf v) never matched an empty value v. That bug
  was probably introduced in 2015 commit 76ee67ac that mistook a nil
  c-string pointer for an empty c-string.

10 months agoBug 5405: Large uploads fill request buffer and die (#1887)
Alex Rousskov [Sun, 1 Sep 2024 00:39:34 +0000 (00:39 +0000)] 
Bug 5405: Large uploads fill request buffer and die (#1887)

    maybeMakeSpaceAvailable: request buffer full
    ReadNow: ... size 0, retval 0, errno 0
    terminateAll: 1/1 after ERR_CLIENT_GONE/WITH_CLIENT
    %Ss=TCP_MISS_ABORTED

This bug is triggered by a combination of the following two conditions:

* HTTP client upload fills Squid request buffer faster than it is
  drained by an origin server, cache_peer, or REQMOD service. The buffer
  accumulates 576 KB (default 512 KB client_request_buffer_max_size + 64
  KB internal "pipe" buffer).

* The affected server or service consumes a few bytes after the critical
  accumulation is reached. In other words, the bug cannot be triggered
  if nothing is consumed after the first condition above is met.

Comm::ReadNow() must not be called with a full buffer: Related
FD_READ_METHOD() code cannot distinguish "received EOF" from "had no
buffer space" outcomes. Server::readSomeData() tried to prevent such
calls, but the corresponding check had two problems:

* The check had an unsigned integer underflow bug[^1] that made it
  ineffective when inBuf length exceeded Config.maxRequestBufferSize.
  That length could exceed the limit due to reconfiguration and when
  inBuf space size first grew outside of maybeMakeSpaceAvailable()
  protections (e.g., during an inBuf.c_str() call) and then got filled
  with newly read data. That growth started happening after 2020 commit
  1dfbca06 optimized SBuf::cow() to merge leading and trailing space.
  Prior to that commit, Bug 5405 could probably only affect Squid
  reconfigurations that lower client_request_buffer_max_size.

* The check was separated from the ReadNow() call it was meant to
  protect. While ConnStateData was waiting for the socket to become
  ready for reading, various asynchronous events could alter inBuf or
  Config.maxRequestBufferSize.

This change fixes both problems.

This change also fixes Squid Bug 5214.

[^1]: That underflow bug was probably introduced in 2015 commit 4d1376d7
while trying to emulate the original "do not read less than two bytes"
ConnStateData::In::maybeMakeSpaceAvailable() condition. That condition
itself looks like a leftover from manual zero-terminated input buffer
days that ended with 2014 commit e7287625. It is now removed.

10 months agoReject config with unknown directives before committing to it (#1897)
Alex Rousskov [Wed, 28 Aug 2024 21:01:15 +0000 (21:01 +0000)] 
Reject config with unknown directives before committing to it (#1897)

Ideally, we want to reject configurations with unknown directives before
applying any configuration changes that correspond to known directives,
but current apply-as-you-parse architecture makes that impractical.
Pending smooth reconfiguration refactoring will make that possible, but
we can make a step towards that ideal future now.

Rejecting bad configurations before calling configDoConfigure() reduces
the set of configuration errors that Squid can detect in one execution
(because configDoConfigure() error-checking code is not reached), but
that small reduction is a lesser evil compared to running
configDoConfigure() with a clearly broken config, especially when we are
going to kill Squid anyway. While many legacy parse_foo() functions do
apply significant changes before configDoConfigure(), we cannot easily
prevent that (for now). We can easily prevent configDoConfigure().

11 months agoUse git to extract default build-info (when enabled) (#1892)
Francesco Chemolli [Sat, 24 Aug 2024 23:30:23 +0000 (23:30 +0000)] 
Use git to extract default build-info (when enabled) (#1892)

Have configure option --enable-build-info[=yes]
refer to git instead of bazaar to extract
information about what is being built to include
in the output of `squid -v`