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

T6536: nat: add migration script that replaces wildcard charater #3749

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

nicolas-fort
Copy link
Contributor

@nicolas-fort nicolas-fort commented Jul 2, 2024

Change Summary

Change wildcard character from + to *

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

nat

Proposed changes

How to test

Config valid an running on 1.3:

vyos@vyos:~$ show version | grep Version
Version:          VyOS 1.3.7
vyos@upgrade-test:~$ show config comm | grep "nat\|zone"
set nat destination rule 10 destination port '9101'
set nat destination rule 10 inbound-interface 'eth2'
set nat destination rule 10 protocol 'tcp'
set nat destination rule 10 translation address '198.51.100.2'
set nat destination rule 200 inbound-interface 'pppoe2.+'
set nat destination rule 200 protocol 'gre'
set nat destination rule 200 translation address '203.0.113.66'
set nat source rule 10 outbound-interface 'eth0'
set nat source rule 10 translation address 'masquerade'
set nat source rule 200 outbound-interface 'l2tp+'
set nat source rule 200 protocol 'udp'
set nat source rule 200 translation port '8899'
set zone-policy zone LAN interface 'eth2'
set zone-policy zone LAN interface 'bond+'
set zone-policy zone LOCAL local-zone
set zone-policy zone WAN interface 'eth1'
vyos@upgrade-test:~$ 

### And from iptables:
-A VZONE_LAN -i bond+ -j RETURN
-A VZONE_LAN -i eth2 -j RETURN
-A VZONE_LAN -j DROP
-A VZONE_WAN -i eth1 -j RETURN
-A VZONE_WAN -j DROP
# and nat:
-A PREROUTING -j VYATTA_PRE_DNAT_HOOK
-A PREROUTING -i eth2 -p tcp -m tcp --dport 9101 -m comment --comment DST-NAT-10 -j DNAT --to-destination 198.51.100.2
-A PREROUTING -i pppoe2.+ -p gre -m comment --comment DST-NAT-200 -j DNAT --to-destination 203.0.113.66
-A POSTROUTING -j VYATTA_PRE_SNAT_HOOK
-A POSTROUTING -o eth0 -m comment --comment SRC-NAT-10 -j MASQUERADE
-A POSTROUTING -o l2tp+ -p udp -m comment --comment SRC-NAT-200 -j SNAT --to-source :8899

And after upgrade:

[   45.733753] vyos-router[1058]: Starting VyOS router: migrate system configure.
[   46.221513] vyos-config[1064]: Configuration success

Welcome to VyOS - upgrade-test ttyS0

upgrade-test login: vyos                                                          
Password: 
Welcome to VyOS!

   ┌── ┐
   . VyOS 1.5-rolling-202407031340
   └ ──┘  current

 * Documentation:  https://docs.vyos.io/en/latest
 * Project news:   https://blog.vyos.io
 * Bug reports:    https://vyos.dev

You can change this banner using "set system login banner post-login" command.

VyOS is a free software distribution that includes multiple components,
you can check individual component licenses under /usr/share/doc/*/copyright
vyos@upgrade-test:~$ show config comm | grep "nat\|zone"
set firewall zone LAN default-action 'drop'
set firewall zone LAN interface 'eth2'
set firewall zone LAN interface 'bond*'
set firewall zone LOCAL default-action 'drop'
set firewall zone LOCAL local-zone
set firewall zone WAN default-action 'drop'
set firewall zone WAN interface 'eth1'
set nat destination rule 10 destination port '9101'
set nat destination rule 10 inbound-interface name 'eth2'
set nat destination rule 10 protocol 'tcp'
set nat destination rule 10 translation address '198.51.100.2'
set nat destination rule 200 inbound-interface name 'pppoe2.*'
set nat destination rule 200 protocol 'gre'
set nat destination rule 200 translation address '203.0.113.66'
set nat source rule 10 outbound-interface name 'eth0'
set nat source rule 10 translation address 'masquerade'
set nat source rule 200 outbound-interface name 'l2tp*'
set nat source rule 200 protocol 'udp'
set nat source rule 200 translation port '8899'
vyos@upgrade-test:~$ 
vyos@upgrade-test:~$ sudo nft list table ip vyos_nat | grep "ROUTING\|ifname"
	chain PREROUTING {
		iifname "eth2" tcp dport 9101 counter packets 0 bytes 0 dnat to 198.51.100.2 comment "DST-NAT-10"
		iifname "pppoe2.*" meta l4proto gre counter packets 0 bytes 0 dnat to 203.0.113.66 comment "DST-NAT-200"
	chain POSTROUTING {
		oifname "eth0" counter packets 0 bytes 0 masquerade comment "SRC-NAT-10"
		oifname "l2tp*" meta l4proto udp counter packets 0 bytes 0 snat to :8899 comment "SRC-NAT-200"
vyos@upgrade-test:~$ 
vyos@upgrade-test:~$ sudo nft list table ip vyos_filter | grep "ifname\|ZONE"
	chain VYOS_ZONE_FORWARD {
		oifname { "bond*", "eth2" } counter packets 0 bytes 0 jump VZONE_LAN
		oifname "eth1" counter packets 0 bytes 0 jump VZONE_WAN
	chain VYOS_ZONE_LOCAL {
		counter packets 897 bytes 168268 jump VZONE_LOCAL_IN
	chain VYOS_ZONE_OUTPUT {
		counter packets 144 bytes 10656 jump VZONE_LOCAL_OUT
	chain VZONE_LAN {
		iifname { "bond*", "eth2" } counter packets 0 bytes 0 return
	chain VZONE_LOCAL_IN {
		iifname "lo" counter packets 144 bytes 10656 return
	chain VZONE_LOCAL_OUT {
		oifname "lo" counter packets 144 bytes 10656 return
	chain VZONE_WAN {
		iifname "eth1" counter packets 0 bytes 0 return
vyos@upgrade-test:~$ 

Smoketest result

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 Jul 2, 2024

👍
No issues in PR Title / Commit Title

Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

I wonder if this should be done in a new migration script with a new version number.

TL;DR;
please move the migration code to nat/6-to-7

As with the original bug report it seems that the NAT config is "lost in migration" I see no reason for a bump of the version number as the actual migrator nat/6-to-7 seems to have the issue and should get the fix.

One can not upgrade from 1.4.0 -> 1.4.1 to restore the behavior which would be the only reason for a new migration version, but the user should rather re-upgrade form 1.3.x -> 1.4.1 or re-load the configuration from his 1.3.x installation on the upgraded 1.4.1 image.

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

There is another place in the CLI where the plus-notation for wildcard interfaces was supported: set zone-policy zone Foo interface 'vti+'.

From my look in the firewall migration scripts, we don't have a similar migration there. Am I missing anything? If not, I think we need a firewall counterpart to the NAT script as well.

@c-po c-po merged commit d6a332a into vyos:current Jul 3, 2024
11 of 12 checks passed
@c-po
Copy link
Member

c-po commented Jul 3, 2024

@Mergifyio backport circinus sagitta

Copy link

mergify bot commented Jul 3, 2024

backport circinus sagitta

✅ Backports have been created

Copy link

github-actions bot commented Jul 3, 2024

CI integration ❌ failed!

Details

CI logs

  • ❌ failed CLI Smoketests returned: 1
  • 👍 passed Config tests returned: 0
  • 👍 passed RAID1 tests returned: 0

c-po added a commit that referenced this pull request Jul 3, 2024
T6536: nat: add migration script that replaces wildcard charater (backport #3749)
c-po added a commit that referenced this pull request Jul 3, 2024
T6536: nat: add migration script that replaces wildcard charater (backport #3749)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants