Skip to content

firewall: T7452: update rule generation for Zone-based firewall #4506

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

Open
wants to merge 1 commit into
base: current
Choose a base branch
from

Conversation

davi2367
Copy link
Contributor

@davi2367 davi2367 commented May 14, 2025

Change summary

When utilizing the ZBF with VRF, issues occur with outgoing traffic - specifically with ping packets destined for interface on vyos itself.

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)

https://vyos.dev/T7452

Related PR(s)

How to test / Smoketest result

I added the below entries in netfilter above the default drop, and it solved the issue

oifname "MGMT" counter packets 0 bytes 0 jump NAME_IPV4_LOCAL_TO_MGMT
oifname "MGMT" counter packets 0 bytes 0 return

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

Copy link

github-actions bot commented May 14, 2025

👍
No issues in PR Title / Commit Title

@davi2367 davi2367 changed the title ZBF/VRF: T7452: genrate netfilter rules with outgoing rules containin… T7452: update zone rule for ZBF/VRF May 14, 2025
@davi2367 davi2367 changed the title T7452: update zone rule for ZBF/VRF firewall: vrf: T7452: update rule generation for Zone-based firewal May 14, 2025
@davi2367 davi2367 changed the title firewall: vrf: T7452: update rule generation for Zone-based firewal firewall: vrf: T7452: update rule generation for Zone-based firewall May 14, 2025
@davi2367 davi2367 changed the title firewall: vrf: T7452: update rule generation for Zone-based firewall firewall: T7452: update rule generation for Zone-based firewall May 14, 2025
@c-po c-po requested review from sarthurdev and c-po May 15, 2025 04:36
@c-po c-po added current bp/sagitta Create automatic backport for sagitta LTS version bp/circinus Create automatic backport for circinus labels May 15, 2025
@davi2367 davi2367 force-pushed the zbf-vrf branch 3 times, most recently from c597268 to c3a69f9 Compare May 15, 2025 07:37
@davi2367
Copy link
Contributor Author

On a local VyOS instance ive replaced "/usr/share/vyos/templates/firewall/nftables-zone.j2" with the file in the commit, and corrected some errors. It seems to be working so-far without any issues.

Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC outbound packets to VRF used the physical interface name.

Ping @nicolas-fort as I think this was something you worked on previously. If you have any feedback here?

Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) ❌ failed
  • Config tests ❌ failed
  • RAID1 tests ❌ failed
  • TPM tests ❌ failed

@davi2367
Copy link
Contributor Author

IIRC outbound packets to VRF used the physical interface name.

Ping @nicolas-fort as I think this was something you worked on previously. If you have any feedback here?

If need be, one can specify the interfaces to listen use in rules by using the "member interface" instead of "member vrf", i changed the behaviour as both are possible with this approach, and it follows the same approach as earlier commits where both vrf and interface was specified under "interface" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bp/circinus Create automatic backport for circinus bp/sagitta Create automatic backport for sagitta LTS version current
Development

Successfully merging this pull request may close these issues.

3 participants