Hi, After upgrading from source from r343710 to r344737 the behavior of limit-source address changed. I have rules like this (which skip after my ipv4 NAT rule where a accept all rule is): skipto 10000 ip4 from any to me 443 in recv em0 proto tcp limit src-addr 10 Altough after my upgrade, these packets don't get allowed out. When I change the rule to the below one it works just fine. skipto 10000 ip4 from any to me 443 in recv em0 proto tcp keep-state I see the dynamic rule getting installed with LIMIT: [/usr/src]$ sudo ipfw show -d |grep LIMIT 00000 2 120 (19s) LIMIT tcp 109.140.18.212 10087 <-> 141.135.72.71 443 :default 00000 3 180 (299s) LIMIT tcp 109.140.18.212 10087 <-> 141.135.72.71 443 :default 00000 3 180 (296s) LIMIT tcp 109.140.18.212 10087 <-> 141.135.72.71 443 :default Although I see the returning packets getting denied: Mar 5 20:23:13 vados kernel: ipfw: 9999 Deny TCP 141.135.72.71:443 109.140.18.212:10087 out via em0 Mar 5 20:23:16 vados kernel: ipfw: 9999 Deny TCP 141.135.72.71:443 109.140.18.212:10087 out via em0 Mar 5 20:23:19 vados kernel: ipfw: 9999 Deny TCP 141.135.72.71:443 109.140.18.212:10087 out via em0 Mar 5 20:23:22 vados kernel: ipfw: 9999 Deny TCP 141.135.72.71:443 109.140.18.212:10087 out via em0 Can somebody help me out with this? Did the behavior of limit source address change?
Also, I just noticed that this is only a problem for rules matching on TCP.
(In reply to Dries Michiels from comment #0) > After upgrading from source from r343710 to r344737 the behavior of > limit-source address changed. Hi, can you try revert r343931 and test your scenario? You can do this with following command: # cd /usr/src # svn merge -c -343931 ^/stable/12
Created attachment 202644 [details] Proposed patch (for stable/12) Can you try this patch?
Do I first revert to r343931 or can I apply the patch on r344737? Thanks!
(In reply to Dries Michiels from comment #4) > Do I first revert to r343931 or can I apply the patch on r344737? > > Thanks! You can just apply the patch, It reverts the part of r344737, where the bug is, I think.
Yep, that fixes it! Thanks.
A commit references this bug: Author: ae Date: Thu Mar 7 04:40:45 UTC 2019 New revision: 344870 URL: https://svnweb.freebsd.org/changeset/base/344870 Log: Fix the problem with O_LIMIT states introduced in r344018. dyn_install_state() uses `rule` pointer when it creates state. For O_LIMIT states this pointer actually is not struct ip_fw, it is pointer to O_LIMIT_PARENT state, that keeps actual pointer to ip_fw parent rule. Thus we need to cache rule id and number before calling dyn_get_parent_state(), so we can use them later when the `rule` pointer is overrided. PR: 236292 MFC after: 3 days Changes: head/sys/netpfil/ipfw/ip_fw_dynamic.c
A commit references this bug: Author: ae Date: Sun Mar 10 04:41:31 UTC 2019 New revision: 344976 URL: https://svnweb.freebsd.org/changeset/base/344976 Log: MFC r344870: Fix the problem with O_LIMIT states introduced in r344018. dyn_install_state() uses `rule` pointer when it creates state. For O_LIMIT states this pointer actually is not struct ip_fw, it is pointer to O_LIMIT_PARENT state, that keeps actual pointer to ip_fw parent rule. Thus we need to cache rule id and number before calling dyn_get_parent_state(), so we can use them later when the `rule` pointer is overrided. PR: 236292 Changes: _U stable/12/ stable/12/sys/netpfil/ipfw/ip_fw_dynamic.c
Fixed in head/ and stable/12. Thanks!
A commit references this bug: Author: ae Date: Sun Apr 14 12:05:09 UTC 2019 New revision: 346205 URL: https://svnweb.freebsd.org/changeset/base/346205 Log: MFC r341471: Reimplement how net.inet.ip.fw.dyn_keep_states works. Turning on of this feature allows to keep dynamic states when parent rule is deleted. But it works only when the default rule is "allow from any to any". Now when rule with dynamic opcode is going to be deleted, and net.inet.ip.fw.dyn_keep_states is enabled, existing states will reference named objects corresponding to this rule, and also reference the rule. And when ipfw_dyn_lookup_state() will find state for deleted parent rule, it will return the pointer to the deleted rule, that is still valid. This implementation doesn't support O_LIMIT_PARENT rules. The refcnt field was added to struct ip_fw to keep reference, also next pointer added to be able iterate rules and not damage the content when deleted rules are chained. Named objects are referenced only when states are going to be deleted to be able reuse kidx of named objects when new parent rules will be installed. ipfw_dyn_get_count() function was modified and now it also looks into dynamic states and constructs maps of existing named objects. This is needed to correctly export orphaned states into userland. ipfw_free_rule() was changed to be global, since now dynamic state can free rule, when it is expired and references counters becomes 1. External actions subsystem also modified, since external actions can be deregisterd and instances can be destroyed. In these cases deleted rules, that are referenced by orphaned states, must be modified to prevent access to freed memory. ipfw_dyn_reset_eaction(), ipfw_reset_eaction_instance() functions added for these purposes. Obtained from: Yandex LLC Sponsored by: Yandex LLC Differential Revision: https://reviews.freebsd.org/D17532 MFC r341472: Add ability to request listing and deleting only for dynamic states. This can be useful, when net.inet.ip.fw.dyn_keep_states is enabled, but after rules reloading some state must be deleted. Added new flag '-D' for such purpose. Retire '-e' flag, since there can not be expired states in the meaning that this flag historically had. Also add "verbose" mode for listing of dynamic states, it can be enabled with '-v' flag and adds additional information to states list. This can be useful for debugging. Obtained from: Yandex LLC Sponsored by: Yandex LLC MFC r344018: Remove `set' field from state structure and use set from parent rule. Initially it was introduced because parent rule pointer could be freed, and rule's information could become inaccessible. In r341471 this was changed. And now we don't need this information, and also it can become stale. E.g. rule can be moved from one set to another. This can lead to parent's set and state's set will not match. In this case it is possible that static rule will be freed, but dynamic state will not. This can happen when `ipfw delete set N` command is used to delete rules, that were moved to another set. To fix the problem we will use the set number from parent rule. Obtained from: Yandex LLC Sponsored by: Yandex LLC MFC r344870: Fix the problem with O_LIMIT states introduced in r344018. dyn_install_state() uses `rule` pointer when it creates state. For O_LIMIT states this pointer actually is not struct ip_fw, it is pointer to O_LIMIT_PARENT state, that keeps actual pointer to ip_fw parent rule. Thus we need to cache rule id and number before calling dyn_get_parent_state(), so we can use them later when the `rule` pointer is overrided. PR: 236292 Changes: _U stable/11/ stable/11/sbin/ipfw/ipfw.8 stable/11/sbin/ipfw/ipfw2.c stable/11/sbin/ipfw/ipfw2.h stable/11/sbin/ipfw/main.c stable/11/sys/netinet/ip_fw.h stable/11/sys/netpfil/ipfw/ip_fw_dynamic.c stable/11/sys/netpfil/ipfw/ip_fw_eaction.c stable/11/sys/netpfil/ipfw/ip_fw_private.h stable/11/sys/netpfil/ipfw/ip_fw_sockopt.c stable/11/sys/netpfil/ipfw/nat64/nat64lsn_control.c stable/11/sys/netpfil/ipfw/nat64/nat64stl_control.c stable/11/sys/netpfil/ipfw/nptv6/nptv6.c