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

NAT: T6371: fix NAT op mode when list of ports/ranges configured #3532

Merged
merged 1 commit into from
May 29, 2024

Conversation

Giggum
Copy link
Contributor

@Giggum Giggum commented May 27, 2024

Change Summary

Before: Issuing the op mode command "show nat source rules" will throw an exception if the user has configured NAT rules using a list of ports as a comma-separated list (e.g. '!22,telnet,http,123,1001-1005'). Also there was no handling for the "!" rule and so '!53' would display as '53'.

With this PR: Introduced iteration to capture all configured ports and append to the appropriate string for display to the user as well as handling of '!' if present in user's configuration.

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/T6371

Related PR(s)

Component(s) name

Proposed changes

See change summary above.

How to 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
  • [N/A] My change requires a change to the documentation
  • [N/A] I have updated the documentation accordingly

@Giggum
Copy link
Contributor Author

Giggum commented May 27, 2024

FYI: same behavior exists in Sagitta, this should be backported to resolve bug there as well.

@c-po
Copy link
Member

c-po commented May 28, 2024

Please add before and after examples to this PR so we can see how the output changed. Backport will be created afterwards.

@Giggum
Copy link
Contributor Author

Giggum commented May 28, 2024

@c-po, here you go:

Before - case 1: -> original bug report, dport should show 5000-8000 but rather the dictionary config including port range is shown instead. This can be reproduced in 1.4x and 1.5x.

set nat source rule 100 outbound-interface name 'eth0'
set nat source rule 100 protocol 'tcp'
set nat source rule 100 source address '10.0.0.0/24'
set nat source rule 100 translation address 'masquerade'

vyos@r4# run show nat source rules 
Rule    Source       Destination                    Proto    Out-Int    Translation
------  -----------  -----------------------------  -------  ---------  -------------
100     10.0.0.0/24  0.0.0.0/0                      IP       eth0       masquerade
        sport any    dport {'range': [5000, 8000]}
[edit]
vyos@r4#

Before - case 2: -> further investigation using a comma-separated list of ports yielded this result. This can be reproduced in 1.4x and 1.5x.

set nat source rule 100 destination port '!22,22,http,5000-8000'
set nat source rule 100 outbound-interface name 'eth0'
set nat source rule 100 protocol 'tcp'
set nat source rule 100 source address '10.0.0.0/24'
set nat source rule 100 translation address 'masquerade'

vyos-A:~/vyos-1x/src/op_mode$ show nat source rules
Traceback (most recent call last):
  File "/usr/libexec/vyos/op_mode/nat.py", line 337, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 263, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/nat.py", line 296, in _wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/nat.py", line 306, in show_rules
    return _get_formatted_output_rules(nat_rules, direction, family)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/op_mode/nat.py", line 139, in _get_formatted_output_rules
    dport = my_dict["set"][0]["range"]
            ~~~~~~~~~~~~~~~~~^^^^^^^^^
TypeError: 'int' object is not sub-scriptable

After:
No NAT rules configured:

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction source --family inet
NAT is not configured

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction destination --family inet
NAT is not configured

Test NAT rules:

set nat destination rule 100 inbound-interface name 'eth1'
set nat destination rule 100 protocol 'tcp'
set nat destination rule 100 source port '!22,22,ssh,http,5000-8000,9000-9500,2222,3999'
set nat destination rule 100 translation address '192.0.2.222'
set nat destination rule 100 destination port '22,22,ssh,http,5000-8000,9000-9500,2222,3999'
set nat destination rule 200 inbound-interface name 'eth1'
set nat destination rule 200 protocol 'udp'
set nat destination rule 200 source port '53'
set nat destination rule 200 translation address '192.0.100.100'
set nat destination rule 200 destination port '53'
set nat source rule 100 outbound-interface name 'eth1'
set nat source rule 100 protocol 'tcp'
set nat source rule 100 source address '10.0.0.0/24'
set nat source rule 100 source port '!ftp,8080,9999-19999'
set nat source rule 100 translation address 'masquerade'

Results after test NAT rules committed:

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction destination --family inet

Rule    Source                                      Destination                                Proto    In-Int    Translation

------  ------------------------------------------  -----------------------------------------  -------  --------  -------------

100     0.0.0.0/0                                   0.0.0.0/0                                  TCP      eth1      192.0.2.222

        sport !22,80,2222,3999,5000-8000,9000-9500  dport 22,80,2222,3999,5000-8000,9000-9500

200     0.0.0.0/0                                   0.0.0.0/0                                  any      eth1      192.0.100.100

        sport 53                                    dport 53

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction source --family inet

Rule    Source                     Destination    Proto    Out-Int    Translation

------  -------------------------  -------------  -------  ---------  -------------

100     10.0.0.0/24                0.0.0.0/0      TCP      eth1       masquerade

        sport !21,8080,9999-19999  dport any

src/op_mode/nat.py Outdated Show resolved Hide resolved
@Giggum Giggum force-pushed the vyos_t6371 branch 2 times, most recently from 780c4f1 to 5429e63 Compare May 29, 2024 15:52
@Giggum
Copy link
Contributor Author

Giggum commented May 29, 2024

@c-po addressed your feedback to add an inner function.

Output:

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction destination --family inet
Rule    Source                      Destination                Proto    In-Int    Translation
------  --------------------------  -------------------------  -------  --------  -------------
100     0.0.0.0/0                   0.0.0.0/0                  TCP      eth1      192.0.2.222
        sport !9000-9500,9000-9500  dport 9000-9500,9000-9500
200     0.0.0.0/0                   0.0.0.0/0                  any      eth1      192.0.100.100
        sport 53                    dport 53

vyos@vyos-A:~/vyos-1x/src/op_mode$ sudo python3 nat.py show_rules --direction source --family inet
Rule    Source                        Destination    Proto    Out-Int    Translation
------  ----------------------------  -------------  -------  ---------  -------------
100     10.0.0.0/24                   0.0.0.0/0      TCP      eth1       masquerade
        sport !9999-19999,9999-19999  dport any

@c-po
Copy link
Member

c-po commented May 29, 2024

Much more readable now!

@c-po c-po merged commit b7595ee into vyos:current May 29, 2024
8 checks passed
@c-po
Copy link
Member

c-po commented May 29, 2024

@Mergifyio backport sagitta

Copy link
Contributor

mergify bot commented May 29, 2024

backport sagitta

✅ Backports have been created

c-po added a commit that referenced this pull request May 30, 2024
NAT: T6371: fix NAT op mode when list of ports/ranges configured (backport #3532)
@Giggum Giggum deleted the vyos_t6371 branch May 31, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants