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

T6191: do not append action policy route|route6 when its not specified #3320

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

nicolas-fort
Copy link
Contributor

Change Summary

Do not append action return to policy route|route6 when its not specified, in order to ensure same behavior as in Equuleus

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)

Related PR(s)

Component(s) name

policy route

Proposed changes

Do not append return to policy route|route6 rules when action is not defined, in order to continue parsing next rules, as it's done in Equuleus.
This does not affect firewall rules, since action must be defined for every single rule

How to test

Config:

vyos@latest:~$ show config comm | grep "policy\|protocols"
set policy route FOO interface 'eth1'
set policy route FOO rule 4 protocol 'tcp'
set policy route FOO rule 4 set tcp-mss '1399'
set policy route FOO rule 4 source address '198.51.100.0/24'
set policy route FOO rule 4 tcp flags syn
set policy route FOO rule 10 destination address '!10.0.0.0/8'
set policy route FOO rule 10 set table '100'
set policy route FOO rule 10 source address '198.51.100.0/24'
set policy route6 FOO666 rule 10 protocol 'icmp'
set protocols static route 0.0.0.0/0 next-hop 1.1.1.2
set protocols static route 8.8.8.8/32 next-hop 192.168.0.1
set protocols static table 100 route 0.0.0.0/0 next-hop 192.168.0.1
vyos@latest:~$ 
vyos@latest:~$ show ip route 
Codes: K - kernel route, C - connected, S - static, R - RIP,
       O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
       T - Table, v - VNC, V - VNC-Direct, A - Babel, F - PBR,
       f - OpenFabric,
       > - selected route, * - FIB route, q - queued, r - rejected, b - backup
       t - trapped, o - offload failure

S>* 0.0.0.0/0 [1/0] via 1.1.1.2, eth3, weight 1, 00:00:29
S   0.0.0.0/0 [210/0] via 192.168.0.1, eth0, weight 1, 00:08:59
C>* 1.1.1.0/24 is directly connected, eth3, 00:08:59
S>* 8.8.8.8/32 [1/0] via 192.168.0.1, eth0, weight 1, 00:08:56
C>* 192.168.0.0/24 is directly connected, eth0, 00:08:59
C>* 198.51.100.0/24 is directly connected, eth1, 00:08:58
vyos@latest:~$ 
vyos@latest:~$ show ip route table 100
Codes: K - kernel route, C - connected, S - static, R - RIP,
       O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
       T - Table, v - VNC, V - VNC-Direct, A - Babel, F - PBR,
       f - OpenFabric,
       > - selected route, * - FIB route, q - queued, r - rejected, b - backup
       t - trapped, o - offload failure

VRF default table 100:
S>* 0.0.0.0/0 [1/0] via 192.168.0.1, eth0, weight 1, 00:09:00
vyos@latest:~$ 

And tcpdump while initiating a connection which triggers PBR:

vyos@latest:~$ sudo tcpdump -ni any tcp
tcpdump: data link type LINUX_SLL2
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on any, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 262144 bytes

17:38:42.942595 eth1  In  IP 198.51.100.254.33516 > 142.250.79.132.443: Flags [S], seq 3177853184, win 32120, options [mss 1460,sackOK,TS val 983926083 ecr 0,nop,wscale 6], length 0
17:38:42.942810 eth0  Out IP 192.168.0.192.33516 > 142.250.79.132.443: Flags [S], seq 3177853184, win 32120, options [mss 1399,sackOK,TS val 983926083 ecr 0,nop,wscale 6], length 0
17:38:42.964064 eth0  In  IP 142.250.79.132.443 > 192.168.0.192.33516: Flags [S.], seq 3656182916, ack 3177853185, win 65535, options [mss 1412,sackOK,TS val 2816524360 ecr 983926083,nop,wscale 8], length 0
17:38:42.964191 eth1  Out IP 142.250.79.132.443 > 198.51.100.254.33516: Flags [S.], seq 3656182916, ack 3177853185, win 65535, options [mss 1412,sackOK,TS val 2816524360 ecr 983926083,nop,wscale 8], length 0
17:38:42.964587 eth1  In  IP 198.51.100.254.33516 > 142.250.79.132.443: Flags [.], ack 1, win 502, options [nop,nop,TS val 983926105 ecr 2816524360], length 0

Smoketest result

root@latest:/usr/libexec/vyos/tests/smoke/cli# ./test_firewall.py 
test_bridge_basic_rules (__main__.TestFirewall.test_bridge_basic_rules) ... ok
test_flow_offload (__main__.TestFirewall.test_flow_offload) ... 
Interface "eth0.10" does not support hardware offload

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_dynamic_groups (__main__.TestFirewall.test_ipv4_dynamic_groups) ... ok
test_ipv4_global_state (__main__.TestFirewall.test_ipv4_global_state) ... 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_ipv4_synproxy (__main__.TestFirewall.test_ipv4_synproxy) ... 
"synproxy" option allowed only for action synproxy

ok
test_ipv6_advanced (__main__.TestFirewall.test_ipv6_advanced) ... ok
test_ipv6_basic_rules (__main__.TestFirewall.test_ipv6_basic_rules) ... ok
test_ipv6_dynamic_groups (__main__.TestFirewall.test_ipv6_dynamic_groups) ... 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
test_zone_basic (__main__.TestFirewall.test_zone_basic) ... ok
test_zone_flow_offload (__main__.TestFirewall.test_zone_flow_offload) ... 
Interface "eth0" does not support hardware offload

ok

----------------------------------------------------------------------
Ran 20 tests in 51.322s

OK
root@latest:/usr/libexec/vyos/tests/smoke/cli# 

root@latest:/usr/libexec/vyos/tests/smoke/cli# ./test_policy_route.py 
test_pbr_group (__main__.TestPolicyRoute.test_pbr_group) ... ok
test_pbr_mark (__main__.TestPolicyRoute.test_pbr_mark) ... ok
test_pbr_mark_connection (__main__.TestPolicyRoute.test_pbr_mark_connection) ... ok
test_pbr_matching_criteria (__main__.TestPolicyRoute.test_pbr_matching_criteria) ... ok
test_pbr_table (__main__.TestPolicyRoute.test_pbr_table) ... ok

----------------------------------------------------------------------
Ran 5 tests in 12.932s

OK
root@latest:/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

…its not specified, in order to ensure same behavior as in Equuleus
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team April 16, 2024 17:50
@c-po
Copy link
Member

c-po commented Apr 17, 2024

@Mergifyio backport sagitta

Copy link
Contributor

mergify bot commented Apr 17, 2024

backport sagitta

✅ Backports have been created

@c-po c-po merged commit 24c997d into vyos:current Apr 17, 2024
9 checks passed
dmbaturin added a commit that referenced this pull request Apr 17, 2024
T6191: do not append action  policy route|route6 when its not specified (backport #3320)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants