|Summary:||route-to rule passes traffic when no targets are specified|
|Product:||Base System||Reporter:||Kajetan Staszkiewicz <vegeta>|
|Component:||kern||Assignee:||Gleb Smirnoff <glebius>|
|Severity:||Affects Only Me||CC:||vegeta|
Description Kajetan Staszkiewicz 2013-11-15 12:20:00 UTC
When rule's redirection pool is empty, a packet still gets forwarded but no redirection occurs. In case of using route-to the packet leaving the router will have original target MAC address and thus will be received by the rotuer itself again if public and internal interfaces are on the same network. Due to no TTL decrease in pf, this situation causes a network loop stealing bandwidth and CPU time. Redirection pool can be emptied by misconfiguration or by some automatic healthchecking tool which decides that all targets are dead and there is nobody to forward the traffic to. Fix: Pass the status of pf_map_addr to pf_set_rt_ifp and then to pf_create_state. If the status shows that pf_map_addr has failed, abort creation of the state and try to delete the src_node it could have created. The attached patch is for FreeBSD 10.0-BETA3 and was not yet tested under some bigger load, although the same approach works well for FreeBSD 9.1. I can provide the 9.1 patch too if requested. Patch attached with submission follows: How-To-Repeat: Empty a table and have some traffic hit a route-to rule using this table.
Comment 1 Gleb Smirnoff 2013-12-25 01:34:05 UTC
Responsible Changed From-To: freebsd-bugs->glebius Grab to not forget.
Comment 2 Kajetan Staszkiewicz 2014-08-10 17:48:47 UTC
Created attachment 145617 [details] Updated patch fixing the issue. Same thing as the previous patch but moved PFRES_MAPFAILED before PFRES_MAX, fixed enum for printing "map-failed" counter. Patch extracted from 10-STABLE sources.
Comment 3 Gleb Smirnoff 2014-08-11 08:35:16 UTC
Thanks! I will commit patch to head as is, but for stable/10 we have requirement not to change the ABI, and we can't grow struct pf_status. Thus, for stable/10 we will probably account this as PFRES_STATEINS.
Comment 4 Gleb Smirnoff 2014-08-11 08:56:28 UTC
Created attachment 145653 [details] improved patch Actually we can cut quite a lot of code here. Can you please review/test the attached patch?
Comment 5 commit-hook 2014-08-15 14:03:07 UTC
A commit references this bug: Author: glebius Date: Fri Aug 15 14:02:25 UTC 2014 New revision: 270022 URL: http://svnweb.freebsd.org/changeset/base/270022 Log: pf_map_addr() can fail and in this case we should drop the packet, otherwise bad consequences including a routing loop can occur. Move pf_set_rt_ifp() earlier in state creation sequence and inline it, cutting some extra code. PR: 183997 Submitted by: Kajetan Staszkiewicz <vegeta tuxpowered.net> Sponsored by: InnoGames GmbH Changes: head/sys/netpfil/pf/pf.c head/sys/netpfil/pf/pf.h
Comment 6 commit-hook 2014-08-25 15:50:26 UTC
A commit references this bug: Author: glebius Date: Mon Aug 25 15:49:41 UTC 2014 New revision: 270576 URL: http://svnweb.freebsd.org/changeset/base/270576 Log: Merge r270022 from head: pf_map_addr() can fail and in this case we should drop the packet, otherwise bad consequences including a routing loop can occur. Move pf_set_rt_ifp() earlier in state creation sequence and inline it, cutting some extra code. PR: 183997 Submitted by: Kajetan Staszkiewicz <vegeta tuxpowered.net> Sponsored by: InnoGames GmbH Changes: _U stable/10/ stable/10/sys/netpfil/pf/pf.c stable/10/sys/netpfil/pf/pf.h