Page MenuHomeVyOS Platform
Create Task
Maniphest T5732

generate firewall rule-resequence drops geoip country-code from output
Closed, ResolvedPublicBUG
Actions

Description

Version: VyOS 1.5-rolling-202311081451
Steps to reproduce:

set firewall ipv4 name SOMEZONE-FORWARD-IPV4 default-action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip country-code 'us'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip country-code 'ca'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip inverse-match
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 log 'enable'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 20 action 'accept'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 20 source address 192.168.0.5

$ show config commands | grep SOMEZONE
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 default-action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip country-code 'us'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip country-code 'ca'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 destination geoip inverse-match
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 10 log 'enable'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 20 action 'accept'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 20 source address '192.168.0.5'

$ generate firewall rule-resequence | grep SOMEZONE
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 default-action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 100 action 'drop'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 100 destination geoip inverse-match
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 100 log 'enable'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 110 action 'accept'
set firewall ipv4 name SOMEZONE-FORWARD-IPV4 rule 110 source address '192.168.0.5'

Details

Difficulty level
Unknown (require assessment)
Version
1.5-rolling-202311081451
Why the issue appeared?
Other
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Bug (incorrect behavior)

Event Timeline

JeffWDH created this task.Nov 11 2023, 12:28 AM
pasik added a subscriber: pasik.Nov 11 2023, 5:02 AM
JeffWDH added a comment.EditedNov 14 2023, 6:54 PM

The country-code item in the config_dict is type list and gets skipped by the convert_to_set_commands function.

'destination': {'geoip': {'country-code': ['us', 'ca'], 'inverse-match': {}}},

I think below is a potential fix, although I have only tested it with my own config, not sure if it breaks anything else.

diff -rupP /usr/libexec/vyos/op_mode/generate_firewall_rule-resequence.py generate_firewall_rule-resequence.py
--- /usr/libexec/vyos/op_mode/generate_firewall_rule-resequence.py      2023-09-10 13:42:53.000000000 +0000
+++ generate_firewall_rule-resequence.py        2023-11-14 18:45:53.663591434 +0000
@@ -41,6 +41,10 @@ def convert_to_set_commands(config_dict,
                 commands.extend(
                     convert_to_set_commands(value, f"{current_key} "))

+        elif isinstance(value, list):
+            for item in value:
+                commands.append(f"set {current_key} '{item}'")
+
         elif isinstance(value, str):
             commands.append(f"set {current_key} '{value}'")

My config:

$ show config commands | grep "set firewall ipv4\|set firewall ipv6" | wc -l
349

Pre-patch:

$ generate firewall rule-resequence | grep "set firewall ipv4\|set firewall ipv6" | wc -l
347

Post-patch:

$ generate firewall rule-resequence | grep "set firewall ipv4\|set firewall ipv6" | wc -l
349

@Viacheslav - you're the most familiar with this code, what are your thoughts?

JeffWDH added a subscriber: Viacheslav.Nov 14 2023, 7:07 PM
Viacheslav added a comment.Nov 15 2023, 6:45 AM

@JeffWDH looks good to me. It definitely should fix this.

JeffWDH added a comment.Nov 15 2023, 12:34 PM

https://github.com/vyos/vyos-1x/pull/2486

Viacheslav changed the task status from Open to In progress.Nov 15 2023, 1:26 PM
Viacheslav assigned this task to JeffWDH.
JeffWDH added a comment.Nov 15 2023, 5:52 PM

Fix was merged into 1.4 and 1.5.

JeffWDH closed this task as Resolved.Nov 15 2023, 5:53 PM
JeffWDH triaged this task as Low priority.
JeffWDH changed Why the issue appeared? from Will be filled on close to Other.
Viacheslav added a project: VyOS 1.4 Sagitta.Nov 15 2023, 5:59 PM
Viacheslav moved this task from Need Triage to Finished on the VyOS 1.5 Circinus board.
Viacheslav moved this task from Need Triage to Finished on the VyOS 1.4 Sagitta board.