Make firewall rule proto parameter a list

openwisp · Jul 28, 2020 · 495b22a · 495b22a
1 parent 29957e3
commit 495b22a
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 8 deletions.
diff --git a/netjsonconfig/backends/openwrt/converters/firewall.py b/netjsonconfig/backends/openwrt/converters/firewall.py
@@ -1,3 +1,10 @@
+"""Firewall configuration management for OpenWRT.
+
+See the following resource for a detailed description of the sections and parameters of
+the UCI configuration for the OpenWRT firewall.
+
+    https://openwrt.org/docs/guide-user/firewall/firewall_configuration
+"""
 from collections import OrderedDict
 
 from ..schema import schema
@@ -76,6 +83,15 @@ def __intermediate_rules(self, rules):
             resultdict = OrderedDict(
                 ((".name", self.__get_auto_name_rule(rule)), (".type", "rule"))
             )
+            if "proto" in rule:
+                # If proto is a single value, then force it not to be in a list so that
+                # the UCI uses "option" rather than "list". If proto is only "tcp"
+                # and"udp", we can force it to the single special value of "tcpudp".
+                proto = rule["proto"]
+                if len(proto) == 1:
+                    rule["proto"] = proto[0]
+                elif set(proto) == {"tcp", "udp"}:
+                    rule["proto"] = "tcpudp"
             resultdict.update(rule)
             result.append(resultdict)
         return result
@@ -99,5 +115,12 @@ def to_netjson_loop(self, block, result, index):
     def __netjson_rule(self, rule):
         if "enabled" in rule:
             rule["enabled"] = rule.pop("enabled") == "1"
+        if "proto" in rule:
+            proto = rule.pop("proto")
+            if not isinstance(proto, list):
+                if proto == "tcpudp":
+                    rule["proto"] = ["tcp", "udp"]
+                else:
+                    rule["proto"] = [proto]
 
         return self.type_cast(rule)
diff --git a/netjsonconfig/backends/openwrt/schema.py b/netjsonconfig/backends/openwrt/schema.py
@@ -615,16 +615,20 @@
                                     "propertyOrder": 5,
                                 },
                                 "proto": {
-                                    "type": "string",
+                                    "type": "array",
                                     "title": "proto",
                                     "description": "match incoming traffic using the given protocol. "
                                     "Can be one of tcp, udp, tcpudp, udplite, icmp, esp, "
                                     "ah, sctp, or all or it can be a numeric value, "
                                     "representing one of these protocols or a different one. "
                                     "A protocol name from /etc/protocols is also allowed. "
                                     "The number 0 is equivalent to all",
-                                    "default": "tcpudp",
+                                    "default": ["tcp", "udp"],
                                     "propertyOrder": 6,
+                                    "items": {
+                                        "title": "Protocol type",
+                                        "type": "string",
+                                    },
                                 },
                                 "icmp_type": {
                                     "title": "icmp_type",

diff --git a/tests/openwrt/test_default.py b/tests/openwrt/test_default.py
@@ -26,7 +26,7 @@ def test_render_default(self):
                         {
                             "name": "Allow-MLD",
                             "src": "wan",
-                            "proto": "icmp",
+                            "proto": ["icmp"],
                             "src_ip": "fe80::/10",
                             "family": "ipv6",
                             "target": "ACCEPT",
@@ -35,7 +35,7 @@ def test_render_default(self):
                         {
                             "name": "Rule2",
                             "src": "wan",
-                            "proto": "icmp",
+                            "proto": ["icmp"],
                             "src_ip": "192.168.1.1/24",
                             "family": "ipv4",
                             "target": "ACCEPT",
@@ -152,7 +152,7 @@ def test_parse_default(self):
                         "family": "ipv6",
                         "icmp_type": ["130/0", "131/0", "132/0", "143/0"],
                         "name": "Allow-MLD",
-                        "proto": "icmp",
+                        "proto": ["icmp"],
                         "src": "wan",
                         "src_ip": "fe80::/10",
                         "target": "ACCEPT",

diff --git a/tests/openwrt/test_firewall.py b/tests/openwrt/test_firewall.py
@@ -15,7 +15,7 @@ class TestFirewall(unittest.TestCase, _TabsMixin):
                     "name": "Allow-MLD",
                     "src": "wan",
                     "src_ip": "fe80::/10",
-                    "proto": "icmp",
+                    "proto": ["icmp"],
                     "icmp_type": ["130/0", "131/0", "132/0", "143/0"],
                     "target": "ACCEPT",
                     "family": "ipv6",
@@ -62,7 +62,7 @@ def test_parse_rule_1(self):
                     "src_ip": "fc00::/6",
                     "dest_ip": "fc00::/6",
                     "dest_port": "546",
-                    "proto": "udp",
+                    "proto": ["udp"],
                     "target": "ACCEPT",
                     "family": "ipv6",
                 }
@@ -103,7 +103,7 @@ def test_parse_rule_2(self):
                 {
                     "name": "Allow-Ping",
                     "src": "wan",
-                    "proto": "icmp",
+                    "proto": ["icmp"],
                     "family": "ipv4",
                     "icmp_type": ["echo-request"],
                     "target": "ACCEPT",