Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5590: firewall log rule: fix order which rule are processed #2283

Merged
merged 1 commit into from Sep 18, 2023

Conversation

nicolas-fort
Copy link
Contributor

Change Summary

firewall log rule: fix order which rule are processed. Log options should be added at the end of the rule, after all matchers and befor action. Also change 2 lines in policy_route smoketest, which suddenly wasn't working as expected

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

firewall

Proposed changes

How to test

Testing config and nft ruleset:

vyos@15-clean:~$ show config comm | grep firewall
set firewall ipv4 forward filter rule 116 action 'drop'
set firewall ipv4 forward filter rule 116 destination address '198.51.100.1'
set firewall ipv4 forward filter rule 116 log 'enable'
set firewall ipv4 forward filter rule 116 outbound-interface interface-name 'eth7'
set firewall ipv4 forward filter rule 116 protocol 'tcp'
set firewall ipv4 forward filter rule 116 source port '5544'
vyos@15-clean:~$ sudo nft -s list chain ip vyos_filter VYOS_FORWARD_filter
table ip vyos_filter {
        chain VYOS_FORWARD_filter {
                type filter hook forward priority filter; policy accept;
                ip daddr 198.51.100.1 tcp sport 5544 oifname "eth7" log prefix "[ipv4-FWD-filter-116-D]" counter drop comment "ipv4-FWD-filter-116"
        }
}
vyos@15-clean:~$ 

Smoketest result

root@15-clean:/usr/libexec/vyos/tests/smoke/cli# ./test_firewall.py
test_bridge_basic_rules (__main__.TestFirewall.test_bridge_basic_rules) ... ok
test_flow_offload_software (__main__.TestFirewall.test_flow_offload_software) ... ok
test_geoip (__main__.TestFirewall.test_geoip) ... Updating GeoIP. Please wait...
Updating GeoIP. Please wait...
ok
test_groups (__main__.TestFirewall.test_groups) ... ok
test_ipv4_advanced (__main__.TestFirewall.test_ipv4_advanced) ... ok
test_ipv4_basic_rules (__main__.TestFirewall.test_ipv4_basic_rules) ... ok
test_ipv4_mask (__main__.TestFirewall.test_ipv4_mask) ... ok
test_ipv4_state_and_status_rules (__main__.TestFirewall.test_ipv4_state_and_status_rules) ... ok
test_ipv6_advanced (__main__.TestFirewall.test_ipv6_advanced) ... ok
test_ipv6_basic_rules (__main__.TestFirewall.test_ipv6_basic_rules) ... ok
test_ipv6_mask (__main__.TestFirewall.test_ipv6_mask) ... ok
test_nested_groups (__main__.TestFirewall.test_nested_groups) ... ok
test_source_validation (__main__.TestFirewall.test_source_validation) ... ok
test_sysfs (__main__.TestFirewall.test_sysfs) ... ok

----------------------------------------------------------------------
Ran 14 tests in 33.286s

OK
root@15-clean:/usr/libexec/vyos/tests/smoke/cli# ./test_nat.py
test_dnat (__main__.TestNAT.test_dnat) ... ok
test_dnat_negated_addresses (__main__.TestNAT.test_dnat_negated_addresses) ... ok
test_dnat_redirect (__main__.TestNAT.test_dnat_redirect) ... ok
test_dnat_without_translation_address (__main__.TestNAT.test_dnat_without_translation_address) ... ok
test_nat_balance (__main__.TestNAT.test_nat_balance) ... ok
test_nat_no_rules (__main__.TestNAT.test_nat_no_rules) ... ok
test_snat (__main__.TestNAT.test_snat) ... ok
test_snat_groups (__main__.TestNAT.test_snat_groups) ... ok
test_snat_required_translation_address (__main__.TestNAT.test_snat_required_translation_address) ...
Source NAT configuration error in rule 5: translation requires address
and/or port

ok
test_static_nat (__main__.TestNAT.test_static_nat) ... ok

----------------------------------------------------------------------
Ran 10 tests in 18.021s
OK
root@15-clean:/usr/libexec/vyos/tests/smoke/cli# ./test_nat66.py 
test_destination_nat66 (__main__.TestNAT66.test_destination_nat66) ... ok
test_destination_nat66_prefix (__main__.TestNAT66.test_destination_nat66_prefix) ... ok
test_destination_nat66_protocol (__main__.TestNAT66.test_destination_nat66_protocol) ... ok
test_destination_nat66_without_translation_address (__main__.TestNAT66.test_destination_nat66_without_translation_address) ... ok
test_nat66_no_rules (__main__.TestNAT66.test_nat66_no_rules) ... ok
test_source_nat66 (__main__.TestNAT66.test_source_nat66) ... ok
test_source_nat66_address (__main__.TestNAT66.test_source_nat66_address) ... ok
test_source_nat66_protocol (__main__.TestNAT66.test_source_nat66_protocol) ... ok
test_source_nat66_required_translation_prefix (__main__.TestNAT66.test_source_nat66_required_translation_prefix) ... 
Source NAT66 configuration error in rule 5: outbound-interface not
specified


Source NAT66 configuration error in rule 5: translation address not
specified

ok

----------------------------------------------------------------------
Ran 9 tests in 13.885s

OK
root@15-clean:/usr/libexec/vyos/tests/smoke/cli# 

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

…ions should be added at the end of the rule, after all matchers and befora action. Also change 2 lines in policy_route smoketest, which suddenly wasn't working as expected
@vyosbot vyosbot requested a review from a team September 18, 2023 18:05
@vyosbot vyosbot requested review from dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team September 18, 2023 18:05
@c-po c-po merged commit 5399924 into vyos:current Sep 18, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants