vyos-1x icon indicating copy to clipboard operation
vyos-1x copied to clipboard

firewall: T4694: Adding rt ipsec exists/missing match to firewall configs

Open talmakion opened this issue 8 months ago • 6 comments

Change Summary

Current firewall syntax only permits inbound matching of IPsec, using "meta ipsec", which indicates a packet was decrypted from IPsec encap.

This extends firewall capability to also be able to match outbound traffic using "rt ipsec", which indicates a packet will be encrypted into an IPsec encap.

"meta ipsec" is not valid (nft will refuse to load config) inside output hooks so additional care has been taken to ensure it doesn't end up in output or any inappropriate jump targets from an output chain.

In doing these checks, I've also added an infinite jump-target loop check to the validation pass.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] 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
  • [x] Other (please describe): Changes to existing firewall ipsec match syntax

Related Task(s)

  • https://vyos.dev/T4694
  • https://vyos.dev/T4667

Related PR(s)

  • https://github.com/vyos/vyos-1x/pull/3637

Component(s) name

  • firewall

Proposed changes

I've covered some concerns I have about my implementation in the forum: https://forum.vyos.io/t/outbound-ipsec-filtering-by-firewall-would-like-some-dev-opinions/14710.

  • Change ipsec match-ipsec/none to match-ipsec-in and match-none-in for fw rules
  • Add ipsec match-ipsec-out and match-none-out
    • *-in means "decrypted" and *-out means "encrypting", the names may benefit from tweaking to make intent clearer
  • Change all the points where the match-ipsec.xml.i include was used before, making sure the new includes (match-ipsec-in/out.xml.i) are used appropriately. There were a handful of spots where match-ipsec.xml.i had snuck back in for output hooked chains already (the common-rule-* includes)
  • Add the -out generators to rendered templates
  • Heavy modification to firewall config validators:
    • I needed to check for ipsec-in matches no matter how deeply nested under an output-hook chain(via jump-target) - this always generates an error.
    • Ended up retrofitting the jump-targets validator from root chains and for named custom chains. It checks for recursive loops and bad IPsec. However, it could be improved - it will not detect a cycle between 2 otherwise un-referenced named chains.
    • Alternatively, perhaps custom chains shouldn't be able to jump or have depth restrictions. The code is already capable of the latter (currently issues a depth warning at 7 jumps deep).

How to test

  • Configured VyOS testing pair in a "triangle" (LEFT -> FAKEWWW <- RIGHT)
  • 2 additional VMs behind LEFT and RIGHT for forward/prerouting chain testing
  • Applied numerous combinations of GRE & IPsec matches in logging/nflogging rules across all filter chains
  • Created several common IPsec & clear use cases for both ptp and forwarded traffic:
    • DMVPN clear
    • DMVPN encrypted
    • IPsec transport, PtP GRE encrypted
    • IPsec tunnel mode
  • Checking dmesg output and tcpdump on FAKEWWW for expected operation in hitting all the right rules
  • Combined with GRE-match PR for complete testing in a handful of logical and completely stupid scenarios
  • Created specific scenarios for blocking DMVPN traffic in the clear when SAs go down, without interfering with other GRE/IPsec traffic, checking coverage on the original ticket issues
  • Exercised validators and ensured all possible combinations of new matches to chains result in working nftables rules.

Smoketest result

Note, the groups failure below is also encountered in a clean rolling image:

# python3 /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) ... ok
test_geoip (__main__.TestFirewall.test_geoip) ... ok
test_groups (__main__.TestFirewall.test_groups) ... FAIL
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) ... 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_timeout_sysctl (__main__.TestFirewall.test_timeout_sysctl) ... ok
test_zone_basic (__main__.TestFirewall.test_zone_basic) ... ok
test_zone_flow_offload (__main__.TestFirewall.test_zone_flow_offload) ... ok

======================================================================
FAIL: test_groups (__main__.TestFirewall.test_groups)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/libexec/vyos/tests/smoke/cli/test_firewall.py", line 152, in test_groups
    self.verify_nftables(nftables_search, 'ip vyos_filter')
  File "/usr/libexec/vyos/tests/smoke/cli/base_vyostest_shim.py", line 122, in verify_nftables
    self.assertTrue(not matched if inverse else matched, msg=search)
AssertionError: False is not true : ['elements = { 192.0.2.5, 192.0.2.8,']

----------------------------------------------------------------------
Ran 21 tests in 184.628s

FAILED (failures=1)

Checklist:

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

talmakion avatar Jun 10 '24 08:06 talmakion