| Summary: | Core 164: squid >=5 crashes on literal IPv6 addresses | ||
|---|---|---|---|
| Product: | IPFire | Reporter: | Brundi <bernd> |
| Component: | squid | Assignee: | Matthias Fischer <matthias.fischer> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | Crash | ||
| Priority: | Will only affect a few users | CC: | michael.tremer, steinel |
| Version: | 2 | ||
| Hardware: | unspecified | ||
| OS: | All | ||
|
Description
Brundi
2022-03-31 12:51:24 UTC
Oh wow. This is bad code. @Matthias: Would you please have a look at this? 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"...
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
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. 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 ! 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? @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 Done. As Michael mentioned, I placed it behind the custom includes. => https://patchwork.ipfire.org/project/ipfire/patch/20220930190556.1435-1-matthias.fischer@ipfire.org/ => https://git.ipfire.org/?p=people/mfischer/ipfire-2.x.git;a=commit;h=aec892178f7ca63a8084c2fa327e60ea93066937 |