optimize: expand expression list when merging into concatenation

commit 0d17d28bb06bf2a04862d5cd879a14bcb9a2d2dc upstream.

The following rules:

    udp dport 137 ct state new,untracked accept
    udp dport 138 ct state new,untracked accept

results in:

  nft: src/optimize.c:670: __merge_concat: Assertion `0' failed.

The logic to expand to the new,untracked list in the concatenation is
missing.

Fixes: 187c6d01d357 ("optimize: expand implicit set element when merging into concatenation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/optimize.c b/src/optimize.c
index 48c46cecba122a4fc0cc1695b5d86aed64a38883..e2156b5d208ba8f0193f8b21155ef17a9c4d8989 100644 (file)
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -664,6 +664,16 @@ static void __merge_concat(const struct optimize_ctx *ctx, uint32_t i,
                                clone = expr_clone(stmt_a->expr->right);
                                compound_expr_add(concat, clone);
                                break;
+                       case EXPR_LIST:
+                               list_for_each_entry(expr, &stmt_a->expr->right->expressions, list) {
+                                       concat_clone = expr_clone(concat);
+                                       clone = expr_clone(expr);
+                                       compound_expr_add(concat_clone, clone);
+                                       list_add_tail(&concat_clone->list, &pending_list);
+                               }
+                               list_del(&concat->list);
+                               expr_free(concat);
+                               break;
                        default:
                                assert(0);
                                break;

diff --git a/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.json-nft b/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.json-nft
new file mode 100644 (file)
index 0000000..46e740a
--- /dev/null
+++ b/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.json-nft
@@ -0,0 +1,435 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "ip",
+        "name": "x",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "x",
+        "name": "y",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "x",
+        "name": "c1",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "x",
+        "name": "c2",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "x",
+        "name": "c3",
+        "handle": 0
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "y",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "meta": {
+                      "key": "iifname"
+                    }
+                  },
+                  {
+                    "payload": {
+                      "protocol": "ip",
+                      "field": "saddr"
+                    }
+                  },
+                  {
+                    "payload": {
+                      "protocol": "ip",
+                      "field": "daddr"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      "eth1",
+                      "1.1.1.1",
+                      "2.2.2.3"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "eth1",
+                      "1.1.1.2",
+                      "2.2.2.4"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "eth1",
+                      "1.1.1.2",
+                      {
+                        "prefix": {
+                          "addr": "2.2.3.0",
+                          "len": 24
+                        }
+                      }
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "eth1",
+                      "1.1.1.2",
+                      {
+                        "range": [
+                          "2.2.4.0",
+                          "2.2.4.10"
+                        ]
+                      }
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "eth2",
+                      "1.1.1.3",
+                      "2.2.2.5"
+                    ]
+                  }
+                ]
+              }
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "y",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "ip",
+                      "field": "protocol"
+                    }
+                  },
+                  {
+                    "payload": {
+                      "protocol": "th",
+                      "field": "dport"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      "tcp",
+                      22
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "udp",
+                      67
+                    ]
+                  }
+                ]
+              }
+            }
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "y",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "udp",
+                      "field": "dport"
+                    }
+                  },
+                  {
+                    "ct": {
+                      "key": "state"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      137,
+                      "new"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      138,
+                      "new"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      137,
+                      "untracked"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      138,
+                      "untracked"
+                    ]
+                  }
+                ]
+              }
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "c1",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "udp",
+                      "field": "dport"
+                    }
+                  },
+                  {
+                    "meta": {
+                      "key": "iifname"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      51820,
+                      "foo"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      514,
+                      "bar"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      67,
+                      "bar"
+                    ]
+                  }
+                ]
+              }
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "c2",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "udp",
+                      "field": "dport"
+                    }
+                  },
+                  {
+                    "meta": {
+                      "key": "iifname"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      100,
+                      "foo"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      51820,
+                      "foo"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      514,
+                      "bar"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      67,
+                      "bar"
+                    ]
+                  }
+                ]
+              }
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "c3",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "udp",
+                      "field": "dport"
+                    }
+                  },
+                  {
+                    "meta": {
+                      "key": "iifname"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      100,
+                      "foo"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      51820,
+                      "foo"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      514,
+                      "bar"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      67,
+                      "bar"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      100,
+                      "test"
+                    ]
+                  },
+                  {
+                    "concat": [
+                      51820,
+                      "test"
+                    ]
+                  }
+                ]
+              }
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    }
+  ]
+}

diff --git a/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.nft b/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.nft
index f56cea1c4fd7b24af659e3a9ae45b58cde8a38b5..d00ac417ca759b1bd94715bc46bbf9c7db522104 100644 (file)
--- a/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.nft
+++ b/tests/shell/testcases/optimizations/dumps/merge_stmts_concat.nft
@@ -2,6 +2,7 @@ table ip x {
        chain y {
                iifname . ip saddr . ip daddr { "eth1" . 1.1.1.1 . 2.2.2.3, "eth1" . 1.1.1.2 . 2.2.2.4, "eth1" . 1.1.1.2 . 2.2.3.0/24, "eth1" . 1.1.1.2 . 2.2.4.0-2.2.4.10, "eth2" . 1.1.1.3 . 2.2.2.5 } accept
                ip protocol . th dport { tcp . 22, udp . 67 }
+               udp dport . ct state { 137 . new, 138 . new, 137 . untracked, 138 . untracked } accept
        }
 
        chain c1 {

diff --git a/tests/shell/testcases/optimizations/merge_stmts_concat b/tests/shell/testcases/optimizations/merge_stmts_concat
index 9679d86223fd1ce918fff2f61a6ae786093a494e..1fd1a3031fb50e07fecc74e6bc3ba1a4c7f4562c 100755 (executable)
--- a/tests/shell/testcases/optimizations/merge_stmts_concat
+++ b/tests/shell/testcases/optimizations/merge_stmts_concat
@@ -10,6 +10,8 @@ RULESET="table ip x {
                meta iifname eth1 ip saddr 1.1.1.2 ip daddr 2.2.4.0-2.2.4.10 accept
                meta iifname eth2 ip saddr 1.1.1.3 ip daddr 2.2.2.5 accept
                ip protocol . th dport { tcp . 22, udp . 67 }
+               udp dport 137 ct state new,untracked accept
+               udp dport 138 ct state new,untracked accept
        }
 }"