Bug 221137
| Summary: | FreeBSD 11+ does not send ICMP redirects | ||
|---|---|---|---|
| Product: | Base System | Reporter: | igorr |
| Component: | kern | Assignee: | Andrey V. Elsukov <ae> |
| Status: | Closed FIXED | ||
| Severity: | Affects Many People | CC: | ae, emaste, eugen, gnn, mckay, net, rgrimes |
| Priority: | --- | Keywords: | regression |
| Version: | 11.1-STABLE | Flags: | koobs:
mfc-stable11?
koobs: mfc-stable10? |
| Hardware: | Any | ||
| OS: | Any | ||
| Attachments: | |||
|
Description
igorr
2017-08-01 08:04:09 UTC
You can add some security policy as a workaround, this disables the using of ip_tryforward(). But it is good to have ICMP redirects without special tweaks. Everything worked in 9.X, and I have checked that 10.3 also sends ICMP redirects. (In reply to igorr from comment #2) > But it is good to have ICMP redirects without special tweaks. > Everything worked in 9.X, and I have checked that 10.3 also sends ICMP > redirects. Yes, it was broken with ip_tryforward introduction. But I don't like the solution with M_SKIP_FIREWALL. This flag can be used by pfil module and clearing it can break something. I prefer to reintroduce sysctl variable to disable tryforward. (In reply to Andrey V. Elsukov from comment #3) We already have sysctl net.inet.ip.redirect with default value of 1 that is broken now. We could change default to 0 (keep fastforwarding on by default) and disable fast path if net.inet.ip.redirect changed to 1. I have used M_SKIP_FIREWALL to avoid reintroducing packet into pfil_run_hooks(). As ICMP redirects are not very common, and if there are no much trouble with reprocessing packet in input firewall rules we can remove M_SKIP_FIREWALL from my patch. Or use some different flag. Also I have not found any places referencing to M_SKIP_FIREWALL except ip_output.c. And it is called after ip_input(). So with my patch ip_tryforward() will be used most of the time. And in rare cases when ICMP redirect should be sent it will not be used. But I have nothing against using net.inet.ip.redirect as flag for disablinmg fast path and enabling ICMP redirects. Created attachment 185012 [details]
New patch for rev 322037. Uses net.inet.ip.redirect as condition to use ip_tryforward()
I have made new patch which uses sysctl net.inet.ip.redirect to decide whenever use ip_tryforward().
Also it changes default value for net.inet.ip.redirect to 0.
Can somebody check my patch and if everything is good commit it? Created attachment 196187 [details]
Plausible icmp redirect fix for ipv4 and ipv6
I'm surprised that this bug is still unfixed after a year and quite surprised that it is marked "Affects Only Me". ICMP redirects are still in all the RFCs and are not deprecated, so FreeBSD's recent inability to generate them affects everyone who wishes to be RFC compliant.
I have attached a plausible minimalist fix. When net.inet.ip.redirect is set (net.inet6.ip6.redirect for ipv6) fast path forwarding (which lacks redirect generation ability) is not attempted. This means the standard code is used and ICMP redirects are generated.
Anyone wishing to have fast path forwarding at the cost of never generating ICMP redirects can disable redirects using sysctl.
This might be considered a strong position to take, but the alternative (ignoring an obvious bug) seems to me to be a stronger position with no up side.
I think this issue should be resolved before 12 ships. Please let your thoughts be known.
CC base r290383 committer A commit references this bug: Author: ae Date: Tue Aug 14 07:54:14 UTC 2018 New revision: 337736 URL: https://svnweb.freebsd.org/changeset/base/337736 Log: Restore ability to send ICMP and ICMPv6 redirects. It was lost when tryforward appeared. Now ip[6]_tryforward will be enabled only when sending redirects for corresponding IP version is disabled via sysctl. Otherwise will be used default forwarding function. PR: 221137 Submitted by: mckay@ MFC after: 2 weeks Changes: head/sys/netinet/ip_input.c head/sys/netinet6/ip6_input.c Thank you Andrey A commit references this bug: Author: ae Date: Tue Aug 28 07:24:10 UTC 2018 New revision: 338343 URL: https://svnweb.freebsd.org/changeset/base/338343 Log: MFC r337736: Restore ability to send ICMP and ICMPv6 redirects. It was lost when tryforward appeared. Now ip[6]_tryforward will be enabled only when sending redirects for corresponding IP version is disabled via sysctl. Otherwise will be used default forwarding function. PR: 221137 Changes: _U stable/11/ stable/11/sys/netinet/ip_input.c stable/11/sys/netinet6/ip6_input.c Fixed in stable/11 and head/. Thanks! |
