Bug 12826 - Core 164: squid >=5 crashes on literal IPv6 addresses
Summary: Core 164: squid >=5 crashes on literal IPv6 addresses
Status: CLOSED FIXED
Alias: None
Product: IPFire
Classification: Unclassified
Component: squid (show other bugs)
Version: 2
Hardware: unspecified All
: Will only affect a few users Crash
Assignee: Matthias Fischer
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-31 12:51 UTC by Brundi
Modified: 2023-01-29 08:00 UTC (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brundi 2022-03-31 12:51:24 UTC 
Squid has been updated to v5.4.1 in core update 164.

Since version 5, squid crashes due to an assertion failure, when parsing/resolving a literal IPv6 address in the HTTP host if IPv6 handling has been disabled (as is the case with IPFire v2).
This is a known bug upstream: https://bugs.squid-cache.org/show_bug.cgi?id=5154
There has been work on a fix, but no news since the beginning of 2022: https://github.com/squid-cache/squid/pull/947

A manually compiled binary of the previous squid release v4.16 works fine on core update 164.

More details:
In my network, telegram-desktop appears to use literal IPv6 addresses to connect to the telegram servers.
After updating to core update 164, the web proxy would die in a matter of seconds and not come back up due to too many failed restarts.
Manually starting squid with `squid -N` reveals the failing assertion:

[root@ipfire ~]# squid -N
2022/03/31 14:39:12| WARNING: BCP 177 violation. Detected non-functional IPv6 loopback.
assert "false" at line 663
Ip::Address invalid? with isIPv4()=F, isIPv6()=T
ADDRESS: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
squid: Address.cc:663: void Ip::Address::getAddrInfo(addrinfo*&, int) const: Assertion `false' failed.
Aborted

The log shows one of telegram's IP addresses: http://[2001:67c:4e8:f002::b]/api
Comment 1 Michael Tremer 2022-03-31 13:26:48 UTC 
Oh wow. This is bad code.

@Matthias: Would you please have a look at this?
Comment 2 Matthias Fischer 2022-03-31 16:16:32 UTC 
I did take look at it, but unfortunately I can't promise much yet.

As written above and as I see it, the current state since Jan 18 is "waiting-for author (author action is expected (and usually required)".

If I had, I could try to compile a test version with an unofficial patch but I only found some code fragments yet.

All proposed changes seem to be "outdated"...
Comment 3 Michael Steinel 2022-09-28 11:13:41 UTC 
we had the same problem with WIN10 clients testing ipv6 connectivity:

--snip--

/usr/sbin/squid -N  -d 9

...

2022/09/27 12:24:16| Accepting HTTP Socket connections at conn2 local=192.168.3.254:3128 remote=[::] FD 11 flags=9
2022/09/27 12:24:17| storeLateRelease: released 0 objects
2022/09/27 12:24:29| DNS error while resolving ipv6.msftconnecttest.com: No valid address records
    current master transaction: master115
2022/09/27 12:24:29| DNS error while resolving ipv6.msftconnecttest.com: No valid address records
    current master transaction: master115
assert "false" at line 663
Ip::Address invalid? with isIPv4()=F, isIPv6()=T
ADDRESS: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
squid: Address.cc:663: void Ip::Address::getAddrInfo(addrinfo*&, int) const: Assertion `false' failed.
Aborted
--snap--

since squid was not restarting our users are without internet access ... 

my workaround is to add the following in squid.conf:

acl to_ipv6 dst ipv6
acl from_ipv6 src ipv6
...
http_access deny to_ipv6
http_access deny from_ipv6
Comment 4 Michael Tremer 2022-09-28 13:25:46 UTC 
Ah I hate assertions. They should be banned from production code. Catch your problems properly instead of sacrificing the whole application.

However, that workaround seems to be very sensible to me.

@Matthias: Would you like to work on a patch for this?

I wonder how not more people have reported this problem.
Comment 5 Michael Steinel 2022-09-28 13:39:29 UTC 
meanwhile I added the workaround in the appropriate file:

cat /var/ipfire/proxy/advanced/acls/include.acl

# prevent ipv6 requests to avoid crash in squid > 5.x
acl to_ipv6 dst ipv6
acl from_ipv6 src ipv6
http_access deny to_ipv6
http_access deny from_ipv6

I recommend to use it in squid.conf per default as IPfire only supports ipv4 !
Comment 6 Matthias Fischer 2022-09-28 16:43:16 UTC 
Hi,

oh my...thats what I needed... ;-)

Adding these lines to 'squid.conf' can only be done through 'proxy.cgi', which is indeed a bit tricky. Lots of 'if'...'else'...

'squid.conf' is initially empty (0 Bytes) and gets (re)written during the every "save" action through the GUI.

Which leaves the question for me: at which position in 'squid.conf' do we want to insert these lines so they will be added - in the appropriate place - under *all* circumstances?

I can try to edit 'proxy.cgi' so they are placed at the position where the contents of 'include.acl' would be placed. Before or after. As Michael (Steinel) wrote, this seems to work.

Other - or better - ideas?
Comment 7 Michael Steinel 2022-09-28 17:15:09 UTC 
@Matthias: I would add it after the include.acl stuff is included
because as far as I know always the last "http_access deny" line matches