forwards: properly propagate conntrack flag

In the following topology:

    config zone
      option name A

    config zone
      option name B

    config zone
      option name C
      option conntrack 1

    config forwarding
      option src A
      option dest B

    config forwarding
      option src A
      option dest C

... the conntrack flag needs to be propagated into both zones A and B as well.

Since A is connected with C, A will inherit C's conntrack requirement which
means that B will need to inherit the flag as well since it is connected to A.

The current code fails to apply the conntrack requirement flag recursively to
zones, leading to stray NOTRACK rules which break conntrack based traffic
policing.

Change the implementation to iteratively reapply the conntrack fixup logic
until no more zones had been changed in order to ensure that all directly and
indirectly connected zones receive the conntrack requirement flag.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/forwards.c b/forwards.c
index 6f950520fd37f0f3d004a766e8a9878415ba697f..c610247c8ab6aed99a9d210946c6f807e2607af9 100644 (file)
--- a/forwards.c
+++ b/forwards.c
@@ -38,6 +38,7 @@ fw3_load_forwards(struct fw3_state *state, struct uci_package *p)
        struct uci_section *s;
        struct uci_element *e;
        struct fw3_forward *forward;
+       bool changed;
 
        INIT_LIST_HEAD(&state->forwards);
 
@@ -83,22 +84,34 @@ fw3_load_forwards(struct fw3_state *state, struct uci_package *p)
                        continue;
                }
 
-               /* NB: forward family... */
-               if (forward->_dest)
-               {
-                       fw3_setbit(forward->_dest->flags[0], FW3_FLAG_ACCEPT);
-                       fw3_setbit(forward->_dest->flags[1], FW3_FLAG_ACCEPT);
+               list_add_tail(&forward->list, &state->forwards);
+               continue;
+       }
+
+       /* Propagate conntrack requirement flag into all zones connected through
+          forwarding entries and repeat until all zones are normalized */
+       do {
+               changed = false;
 
-                       if (forward->_src &&
-                           (forward->_src->conntrack || forward->_dest->conntrack))
+               list_for_each_entry(forward, &state->forwards, list)
+               {
+                       /* NB: forward family... */
+                       if (forward->_dest)
                        {
-                               forward->_src->conntrack = forward->_dest->conntrack = true;
+                               fw3_setbit(forward->_dest->flags[0], FW3_FLAG_ACCEPT);
+                               fw3_setbit(forward->_dest->flags[1], FW3_FLAG_ACCEPT);
+
+                               if (forward->_src &&
+                                   (forward->_src->conntrack != forward->_dest->conntrack))
+                               {
+                                       forward->_src->conntrack = true;
+                                       forward->_dest->conntrack = true;
+                                       changed = true;
+                               }
                        }
                }
-
-               list_add_tail(&forward->list, &state->forwards);
-               continue;
        }
+       while (changed);
 }