T#8649: Firewall zones are changed when browsing Device.IP.Interface.

(cherry picked from commit 612ad08a82)

dmtree/tr181/firewall.c

@@ -14,6 +14,37 @@
 /*************************************************************
 * COMMON FUNCTIONS
 **************************************************************/
+void firewall__create_zone_section(char *s_name)
+{
+	struct uci_section *s = NULL;
+	char *input = NULL;
+	char *output = NULL;
+	char *forward = NULL;
+
+	dmuci_get_option_value_string("firewall", "@defaults[0]", "input", &input);
+	dmuci_get_option_value_string("firewall", "@defaults[0]", "output", &output);
+	dmuci_get_option_value_string("firewall", "@defaults[0]", "forward", &forward);
+
+	dmuci_add_section("firewall", "zone", &s);
+	dmuci_rename_section_by_section(s, s_name);
+	dmuci_set_value_by_section(s, "name", s_name);
+	dmuci_set_value_by_section(s, "input", input);
+	dmuci_set_value_by_section(s, "output", output);
+	dmuci_set_value_by_section(s, "forward", forward);
+	dmuci_add_list_value_by_section(s, "network", s_name);
+}
+
+static bool firewall_zone_exists(char *s_name)
+{
+	struct uci_section *s = NULL;
+
+	uci_foreach_option_eq("firewall", "zone", "name", s_name, s) {
+		return true;
+	}
+
+	return false;
+}
+
 static void create_portmapping_section(bool b)
 {
 	struct uci_section *s = NULL;
@@ -1034,19 +1065,12 @@ static int set_rule_interface(struct dmctx *ctx, void *data, char *type, char *v
 			} else {
 				adm_entry_get_linker_value(ctx, value, &iface);
 				if (iface && iface[0] != '\0') {
-					struct uci_section *s = NULL;
-					char *net;
 
-					uci_foreach_sections("firewall", "zone", s) {
-						dmuci_get_value_by_section_string(s, "network", &net);
-						if (dm_strword(net, iface)) {
-							char *zone_name;
+					// check if firewall zone exists
+					if (!firewall_zone_exists(iface))
+						firewall__create_zone_section(iface);
 
-							dmuci_get_value_by_section_string(s, "name", &zone_name);
-							dmuci_set_value_by_section((option && DM_LSTRCMP(option, "*") == 0) ? ((struct dmmap_dup *)data)->dmmap_section : ((struct dmmap_dup *)data)->config_section, type, zone_name);
-							break;
-						}
-					}
+					dmuci_set_value_by_section((option && DM_LSTRCMP(option, "*") == 0) ? ((struct dmmap_dup *)data)->dmmap_section : ((struct dmmap_dup *)data)->config_section, type, iface);
 					dmfree(iface);
 				}
 			}

dmtree/tr181/firewall.h

@@ -20,4 +20,6 @@ extern DMLEAF tFirewallChainParams[];
 extern DMOBJ tFirewallChainObj[];
 extern DMLEAF tFirewallChainRuleParams[];
 
+void firewall__create_zone_section(char *s_name);
+
 #endif

dmtree/tr181/ip.c

@@ -11,6 +11,7 @@
  */
 
 #include "ppp.h"
+#include "firewall.h"
 #include "ip.h"
 #ifdef BBF_TR143
 #include "diagnostics.h"
@@ -66,60 +67,6 @@ static int get_ip_iface_sysfs(const struct uci_section *data, const char *name,
 	return get_net_iface_sysfs(section_name((struct uci_section *)data), name, value);
 }
 
-static bool firewall_zone_exists(char *s_name)
-{
-	struct uci_section *s = NULL;
-
-	uci_foreach_option_eq("firewall", "zone", "name", s_name, s) {
-		return true;
-	}
-
-	return false;
-}
-
-static void create_firewall_zone_section(char *s_name)
-{
-	struct uci_section *s = NULL;
-	char zone_name[32] = {0};
-	char *input = NULL;
-	char *output = NULL;
-	char *forward = NULL;
-
-	snprintf(zone_name, sizeof(zone_name), "zone_%s", s_name);
-
-	dmuci_get_option_value_string("firewall", "@defaults[0]", "input", &input);
-	dmuci_get_option_value_string("firewall", "@defaults[0]", "output", &output);
-	dmuci_get_option_value_string("firewall", "@defaults[0]", "forward", &forward);
-
-	dmuci_add_section("firewall", "zone", &s);
-	dmuci_rename_section_by_section(s, zone_name);
-	dmuci_set_value_by_section(s, "name", s_name);
-	dmuci_set_value_by_section(s, "input", input);
-	dmuci_set_value_by_section(s, "output", output);
-	dmuci_set_value_by_section(s, "forward", forward);
-	dmuci_add_list_value_by_section(s, "network", s_name);
-}
-
-static void remove_unused_firewall_zone_sections(void)
-{
-	struct uci_section *s = NULL, *stmp = NULL;
-
-	uci_foreach_sections_safe("firewall", "zone", stmp, s) {
-		struct uci_section *dmmap_section = NULL;
-		char *zone_added = NULL;
-		char *name = NULL;
-
-		get_dmmap_section_of_config_section("dmmap_firewall", "zone", section_name(s), &dmmap_section);
-		dmuci_get_value_by_section_string(dmmap_section, "added_by_controller", &zone_added);
-		if (zone_added && DM_LSTRCMP(zone_added, "1") == 0)
-			continue;
-
-		dmuci_get_value_by_section_string(s, "name", &name);
-		if (!get_origin_section_from_config("network", "interface", name))
-			dmuci_delete_by_section(s, NULL, NULL);
-	}
-}
-
 static void add_network_to_firewall_zone_network_list(char *zone_name, char *interface_name)
 {
 	struct uci_section *s = NULL;
@@ -615,17 +562,12 @@ static int browseIPInterfaceInst(struct dmctx *dmctx, DMNODE *parent_node, void
 			DM_STRCHR(device, '@'))
 			continue;
 
-		// check if firewall zone exists
-		if (!firewall_zone_exists(section_name(p->config_section)))
-			create_firewall_zone_section(section_name(p->config_section));
-
 		inst = handle_instance(dmctx, parent_node, p->dmmap_section, "ip_int_instance", "ip_int_alias");
 
 		if (DM_LINK_INST_OBJ(dmctx, parent_node, (void *)p->config_section, inst) == DM_STOP)
 			break;
 	}
 	free_dmmap_config_dup_list(&dup_list);
-	remove_unused_firewall_zone_sections();
 	return 0;
 }
 
@@ -864,7 +806,7 @@ static int addObjIPInterface(char *refparam, struct dmctx *ctx, void *data, char
 	dmuci_set_value("network", ip_name, "disabled", "1");
 
 	// Firewall zone section
-	create_firewall_zone_section(ip_name);
+	firewall__create_zone_section(ip_name);
 
 	dmuci_add_section_bbfdm("dmmap_network", "interface", &dmmap_ip_interface);
 	dmuci_set_value_by_section(dmmap_ip_interface, "section_name", ip_name);

dmtree/tr181/nat.c

@@ -77,7 +77,6 @@ static int add_NAT_InterfaceSetting(char *refparam, struct dmctx *ctx, void *dat
 
 	dmuci_add_section_bbfdm("dmmap_firewall", "zone", &dmmap_firewall);
 	dmuci_set_value_by_section(dmmap_firewall, "section_name", zone_name);
-	dmuci_set_value_by_section(dmmap_firewall, "added_by_controller", "1");
 	dmuci_set_value_by_section(dmmap_firewall, "interface_setting_instance", *instance);
 	return 0;
 }

docs/firewall.md

@@ -1,9 +1,10 @@
 # Firewall
 Aim of this document to explain the TR181 firewall datamodel parameter mappings with firewall and network uci.
 
-In TR-181 firewall definition, we have Device.Firewall.Level., Deivce.Firewall.Chain. and Firewall.Chain.{i}.Rules., which does not have one to one mapping with firewall uci sections. Also due to lack of mapping between Device.IP.Interface and Firewall uci zones, its not possible to define rules for newly created interfaces.
+In TR-181 firewall definition, we have Device.Firewall.Level., Deivce.Firewall.Chain. and Firewall.Chain.{i}.Rules., which does not have one to one mapping with firewall uci sections.
 
-To simplify the mappings, libbbf during bootstrap, does
+So for each new network interface created by libbbf, a new firewall uci zone will be created as follow:
+- Create a Network interface section
 - Create a Firewall zone section corresponding to the Interface section in the network uci file
 - Give it the same name as the interface section in the network uci file.
 - Set the default firewall zone value of input/output/forward to ACCEPT/ACCEPT/ACCEPT for all bridge interface and REJECT/ACCEPT/REJECT for all non bridge interfaces
@@ -32,6 +33,8 @@ config rule ‘x’
     option target ‘ACCEPT’
 ```
 
+> Note: when trying to define a rule as Chain.1.Rule.x.SourceInterface = Device.IP.Interface.x and the zone for this interface (Device.IP.Interface.x) doesn't exist in the firewall uci file so, a new firewall zone section corresponding to this interface section will be created.
+
 Similarly, to configure firewall rules for each interfaces, add rule objects in Device.Firewall.Chain.{i}.Rule.{i}. table to the existing Device.Firewall.Chain.{i}. in the order in which they should be applied.
 
 # Limitations