From d1d71f39eaf7e7fadb6726f9fd5b96f262d10773 Mon Sep 17 00:00:00 2001 From: Lindsay Harvey Date: Sat, 11 Jul 2026 20:48:17 +1000 Subject: [PATCH 1/2] Enhance Shorewall-nft to preserve dynamic sets and improve routing/accounting display - Preserve externally populated nft sets across `shorewall stop` by snapshotting and restoring their elements. - Implement `shorewall show routing` and `shorewall6 show routing` for live policy rules and route tables. - Add `shorewall show accounting` for live named counters and accounting chains. - Improve handling of large rulesets in `shorewall check` and `shorewall migrate` with chunked validation. - Update accounting parsing to support new actions: COUNT and DONE. - Enhance tests to verify preservation of dynamic sets across stop/start cycles. --- debian/changelog | 18 ++ src/shorewall_nft/cli.py | 287 +++++++++++++++++++++- src/shorewall_nft/emit.py | 128 ++++++++-- src/shorewall_nft/model.py | 2 + src/shorewall_nft/parsers.py | 30 ++- src/shorewall_nft/script.py | 7 + tests/corpus/0041-ipset-persist/case.toml | 11 +- tests/harness/hardening-unit.py | 21 +- 8 files changed, 457 insertions(+), 47 deletions(-) diff --git a/debian/changelog b/debian/changelog index 887802b..fed5f2c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,21 @@ +shorewall-nft (0.0.6+local1) UNRELEASED; urgency=medium + + * Preserve Shorewall-referenced externally populated nft sets across + `shorewall stop`: snapshot them, declare them in the stopped-state table, + and restore their elements immediately after the table replacement. + Stop now atomically refreshes its generated wrapper first, preventing an + artifact left by an older package from retaining destructive stop logic. + * Implement `shorewall show routing` and `shorewall6 show routing` with + live policy rules and standard, provider, balance, and additional route + tables in Shorewall-style output. + * Implement `shorewall show accounting` for live named counters, + per-address counter sets, and accounting chains. + * Fall back to chunked kernel validation when `shorewall check` or + `shorewall migrate` hits the netlink transaction-size limit on a large + generated ruleset. + + -- Lindsay Harvey Sat, 11 Jul 2026 00:00:01 +0000 + shorewall-nft (0.0.6) unstable; urgency=medium * A source or destination column may again mix an address list with an diff --git a/src/shorewall_nft/cli.py b/src/shorewall_nft/cli.py index 8351cab..c4921cd 100644 --- a/src/shorewall_nft/cli.py +++ b/src/shorewall_nft/cli.py @@ -7,6 +7,7 @@ """ import ipaddress import os +import re import select import shutil import subprocess @@ -14,7 +15,7 @@ import tempfile import time -from . import __version__, capabilities +from . import __version__, capabilities, chunk from .compile import compile_config from .emit import table_for from .errors import ConfigError @@ -152,6 +153,43 @@ def cmd_version(args, family): return 0 +def _check_ruleset(path, family): + """Ask the kernel to validate a ruleset in a disposable netns. + + Prefer one check-only transaction. Large rulesets can exceed the + netlink send buffer, so on E2BIG validate the same fail-closed skeleton + and chunks the runtime loader uses. The chunked pass applies rules in + the private namespace because separate ``nft -c`` calls would not retain + the skeleton needed by later chunks. + """ + nft_path = _nft() + result = subprocess.run( + ["unshare", "-r", "-n", nft_path, "-c", "-f", path], + capture_output=True, text=True) + if result.returncode == 0 or "Message too long" not in result.stderr: + return result + + with open(path) as ruleset_file: + ruleset = ruleset_file.read() + skeleton, chunks = chunk.split(ruleset, table_for(family)) + with tempfile.TemporaryDirectory(prefix="shorewall-nft-check-") as tmpdir: + paths = [] + for number, text in enumerate([skeleton] + chunks): + chunk_path = os.path.join(tmpdir, f"{number:05d}.nft") + with open(chunk_path, "w") as chunk_file: + chunk_file.write(text) + paths.append(chunk_path) + # One unshare invocation keeps all chunk loads in the same disposable + # namespace. Arguments carry paths without interpolating them into sh. + command = ( + 'nft=$1; shift; for file do "$nft" -f "$file" || exit; done' + ) + return subprocess.run( + ["unshare", "-r", "-n", "sh", "-c", command, + "shorewall-nft-check", nft_path] + paths, + capture_output=True, text=True) + + def cmd_check(args, family): directory, _, fam_flag, _ = _parse_compile_args(args) confdir = directory or _confdir(family) @@ -160,8 +198,7 @@ def cmd_check(args, family): path = tmp.name try: compile_config(confdir, path, family) - nft = subprocess.run(["unshare", "-r", "-n", _nft(), "-c", "-f", - path], capture_output=True, text=True) + nft = _check_ruleset(path, family) if nft.returncode != 0: print(nft.stderr, file=sys.stderr) print(" ERROR: nft rejected the generated ruleset", @@ -204,8 +241,31 @@ def cmd_reload(args, family): def cmd_stop(args, family): vardir = _vardir(family) script = _script_path(vardir) - if not os.path.exists(script): - _compile_to(_confdir(family), family, script) + # A package upgrade can change stopped-state lifecycle semantics while an + # older generated wrapper remains in /var/lib. Compile a fresh wrapper + # before stopping so `stop` uses the installed implementation just as + # start/reload/restart do. Build beside the live artifact and replace it + # only after a complete successful compile; an invalid current config can + # therefore still be stopped with its last valid wrapper. + fd, candidate = tempfile.mkstemp(prefix="firewall.stop-", dir=vardir) + os.close(fd) + try: + try: + _compile_to(_confdir(family), family, candidate) + except (ConfigError, OSError) as error: + if not os.path.exists(script): + raise + print(f"shorewall-nft: could not refresh the stop artifact; " + f"using the last compiled one: {error}", file=sys.stderr) + else: + os.replace(candidate + ".nft", script + ".nft") + os.replace(candidate, script) + finally: + for pathname in (candidate, candidate + ".nft"): + try: + os.unlink(pathname) + except OSError: + pass rc = _run_script(script, "stop") if rc == 0: _state(vardir, "Stopped") @@ -353,6 +413,216 @@ def cmd_status(args, family): return 0 +def _accounting_listing(text): + """Extract live accounting objects from an ``nft list table`` result.""" + objects = [] + current = [] + depth = 0 + start = re.compile( + r"^\s*(?:counter\s+acct_|set\s+acct_|chain\s+(?:accounting|acct_chain_))" + r"[^\n{]*\{") + for line in text.splitlines(): + if not current: + if not start.match(line): + continue + current = [line] + depth = line.count("{") - line.count("}") + else: + current.append(line) + depth += line.count("{") - line.count("}") + if current and depth == 0: + objects.append("\n".join(current)) + current = [] + return objects + + +def _show_accounting(family): + table = table_for(family).split() + result = subprocess.run( + [_nft(), "list", "table", *table], capture_output=True, text=True) + if result.returncode: + if result.stderr: + print(result.stderr, end="", file=sys.stderr) + return result.returncode + objects = _accounting_listing(result.stdout) + chains = [obj for obj in objects + if obj.lstrip().startswith("chain ")] + if not objects: + print(f"No accounting objects in table {' '.join(table)}") + return 0 + import socket + product = "Shorewall6" if family == 6 else "Shorewall" + now = time.strftime("%a %d %b %Y %H:%M:%S %Z") + print(f"{product}-nft {__version__} Accounting at {socket.gethostname()} " + f"- {now}\n") + + named = {} + for obj in objects: + match = re.search( + r"^\s*counter\s+(acct_[\w.-]+)\s*\{.*?" + r"packets\s+(\d+)\s+bytes\s+(\d+)", obj, re.S) + if match: + named[match.group(1)] = (int(match.group(2)), int(match.group(3))) + + references = {} + chains.sort(key=lambda obj: 0 if re.search( + r"^\s*chain\s+accounting\b", obj) else 1) + for obj in chains: + for target in re.findall(r"\bjump\s+acct_chain_([\w.-]+)", obj): + references[target] = references.get(target, 0) + 1 + default_addr = "::/0" if family == 6 else "0.0.0.0/0" + printed = False + for obj in chains: + first = obj.splitlines()[0] + match = re.search(r"\bchain\s+(\S+)", first) + if not match: + continue + nft_chain = match.group(1) + chain = (nft_chain[len("acct_chain_"):] if + nft_chain.startswith("acct_chain_") else nft_chain) + rows = [] + for line in obj.splitlines()[1:-1]: + if "counter" not in line or line.strip().startswith("type "): + continue + packets = bytes_ = 0 + count = re.search(r"\bcounter\s+packets\s+(\d+)\s+bytes\s+(\d+)", + line) + named_count = re.search(r'\bcounter\s+name\s+"([^"]+)"', line) + if count: + packets, bytes_ = int(count.group(1)), int(count.group(2)) + elif named_count: + packets, bytes_ = named.get(named_count.group(1), (0, 0)) + + jump = re.search(r"\bjump\s+acct_chain_([\w.-]+)", line) + update = re.search(r"\bupdate\s+@(acct_[\w.-]+)", line) + if jump: + target = jump.group(1) + detail = "" + elif update: + target = "ACCOUNT" + set_name = update.group(1) + table_name = re.sub(r"_\d+$", "", set_name[len("acct_"):]) + address = re.search(r"\bip6?\s+[sd]addr\s+(\S+)", line) + detail = (f" ACCOUNT addr {address.group(1)} " + f"tname {table_name}" if address else + f" ACCOUNT tname {table_name}") + elif " return" in line: + target, detail = "DONE", "" + elif named_count: + target = named_count.group(1)[len("acct_"):] + detail = "" + else: + target, detail = "COUNT", "" + + incoming = re.search(r'\biifname\s+"([^"]+)"', line) + outgoing = re.search(r'\boifname\s+"([^"]+)"', line) + source = re.search(r"\bip6?\s+saddr\s+(\S+)", line) + dest = re.search(r"\bip6?\s+daddr\s+(\S+)", line) + comment = re.search(r'\bcomment\s+"([^"]+)"', line) + rows.append((packets, bytes_, target, + incoming.group(1) if incoming else "*", + outgoing.group(1) if outgoing else "*", + source.group(1) if source else default_addr, + dest.group(1) if dest else default_addr, + comment.group(1) if comment else "", detail)) + if not rows: + continue + printed = True + refs = references.get(chain, 0) + suffix = f" ({refs} reference{'s' if refs != 1 else ''})" \ + if nft_chain != "accounting" else "" + print(f"Chain {chain}{suffix}") + print(" pkts bytes target prot opt in out " + "source destination") + for packets, bytes_, target, incoming, outgoing, source, dest, \ + comment, detail in rows: + note = f" /* {comment} */" if comment else "" + print(f"{_metric(packets):>5} {_metric(bytes_):>5} " + f"{target:<12} all -- {incoming:<10} {outgoing:<10} " + f"{source:<20} {dest:<20}{note}{detail}") + print() + if not printed: + print("Accounting objects exist, but no live accounting rules were found.") + return 0 + + +def _metric(value): + """iptables-style compact packet/byte counter.""" + units = ("", "K", "M", "G", "T", "P") + number = float(value) + unit = units[0] + for unit in units: + if number < 1000 or unit == units[-1]: + break + number /= 1000.0 + if not unit: + return str(value) + return f"{number:.0f}{unit}" if number >= 10 else f"{number:.1f}{unit}" + + +def _show_routing(family): + """Show live policy rules and every relevant route table.""" + import socket + ip = _ip() + flag = "-6" if family == 6 else "-4" + product = "Shorewall6" if family == 6 else "Shorewall" + now = time.strftime("%a %d %b %Y %H:%M:%S %Z") + print(f"{product}-nft {__version__} Routing at {socket.gethostname()} " + f"- {now}\n") + print("Routing Rules\n") + rules = subprocess.run([ip, flag, "rule", "show"], + capture_output=True, text=True) + if rules.stdout: + print(rules.stdout.rstrip()) + if rules.returncode: + if rules.stderr: + print(rules.stderr, end="", file=sys.stderr) + return rules.returncode + + # label -> kernel table selector. Provider names are the useful labels, + # while their numbers work even when /etc/iproute2/rt_tables lacks them. + tables = {"default": "253", "local": "255", "main": "254"} + number_labels = {"253": "default", "254": "main", "255": "local"} + try: + from .compile import load + cfg = load(_confdir(family), family) + except (ConfigError, OSError): + cfg = None + if cfg is not None: + for provider in cfg.providers: + tables[provider.name] = str(provider.number) + number_labels[str(provider.number)] = provider.name + if any(provider.balance for provider in cfg.providers): + tables["balance"] = "250" + number_labels["250"] = "balance" + for route in cfg.routes: + selector = str(route["table"]) + label = number_labels.get(selector, selector) + tables.setdefault(label, selector) + + # Include tables introduced outside Shorewall or by a stale/live rule. + for selector in re.findall(r"\b(?:lookup|table)\s+(\S+)", rules.stdout): + selector = selector.rstrip() + standard = {"local": "255", "main": "254", "default": "253", + "balance": "250"} + query = standard.get(selector, selector) + label = number_labels.get(query, selector) + tables.setdefault(label, query) + + for label in sorted(tables): + print(f"\nTable {label}:\n") + routes = subprocess.run( + [ip, flag, "route", "show", "table", tables[label]], + capture_output=True, text=True) + if routes.stdout: + print(routes.stdout.rstrip()) + if routes.stderr: + print(routes.stderr.rstrip()) + if not routes.stdout and not routes.stderr: + print("(empty)") + return 0 + + def cmd_show(args, family): what = args[0] if args else "filter" if what in ("filter", "nat", "mangle", "raw", "rules"): @@ -380,6 +650,10 @@ def cmd_show(args, family): return 0 if what in ("providers", "provider"): return _show_providers(family) + if what == "accounting": + return _show_accounting(family) + if what == "routing": + return _show_routing(family) print(f"shorewall-nft: 'show {what}' is not implemented yet", file=sys.stderr) return 1 @@ -656,8 +930,7 @@ def cmd_migrate(args, family): except ConfigError as e: _fatal(f"configuration does not compile: {e}\n" "Nothing changed. Resolve the above and run migrate again.") - nft = subprocess.run(["unshare", "-r", "-n", _nft(), "-c", "-f", path], - capture_output=True, text=True) + nft = _check_ruleset(path, family) if nft.returncode != 0: print(nft.stderr, file=sys.stderr) _fatal("the generated ruleset was rejected by nft. Nothing " diff --git a/src/shorewall_nft/emit.py b/src/shorewall_nft/emit.py index 4c98d19..e0c1eaa 100644 --- a/src/shorewall_nft/emit.py +++ b/src/shorewall_nft/emit.py @@ -353,6 +353,19 @@ def _mark_match(mark): return f"meta mark {neg}{int(value, 0):#x}" +def _acct_ident(name): + return name.replace("-", "_").replace(".", "_") + + +def _acct_chain(name): + return "accounting" if name == "accounting" else \ + f"acct_chain_{_acct_ident(name)}" + + +def _acct_counter(name): + return f"acct_{_acct_ident(name)}" + + def _time_match(spec): """TIME column: &-separated timestart/timestop/weekdays/etc.""" out = [] @@ -1007,42 +1020,71 @@ def _accounting(self): return ipkw = "ip6" if self.cfg.family == 6 else "ip" addr_type = "ipv6_addr" if self.cfg.family == 6 else "ipv4_addr" + + def match(a): + m = [] + if a.in_iface: + m.append(f'iifname "{a.in_iface}"') + if a.out_iface: + m.append(f'oifname "{a.out_iface}"') + if a.saddr: + m.append(f"{ipkw} saddr {_addr_set(a.saddr)}") + if a.daddr: + m.append(f"{ipkw} daddr {_addr_set(a.daddr)}") + return m + + def emit_stmt(chain, stmt, a, indent=2): + comment = f' comment "{a.origin}"' if a.origin else "" + self.out(" ".join(match(a) + [stmt]).strip() + comment, indent) + + def emit_chain_body(chain): + for idx, a in enumerate(self.cfg.accounting): + if a.action == "count-chain": + if a.chain == chain and a.table != chain: + emit_stmt(chain, + f"counter jump {_acct_chain(a.table)}", a) + if a.table == chain: + emit_stmt(chain, f'counter name "{_acct_counter(a.table)}"', a) + continue + if a.chain != chain: + continue + if a.net: + name = f"acct_{_acct_ident(a.table)}_{idx}" + for sel in (f"{ipkw} saddr", f"{ipkw} daddr"): + emit_stmt(chain, f"{sel} {a.net} counter update " + f"@{name} {{ {sel} }}", a) + elif a.action == "done": + emit_stmt(chain, "counter return", a) + elif a.action == "count": + emit_stmt(chain, "counter", a) + self.out("") counters = [] for idx, a in enumerate(self.cfg.accounting): if a.net: - self.out(f"set acct_{a.table.replace('-', '_')}_{idx} {{", 1) + self.out(f"set acct_{_acct_ident(a.table)}_{idx} {{", 1) self.out(f"type {addr_type}; flags dynamic; counter; " "size 65535;", 2) self.out("}", 1) - else: - name = f"acct_{a.table.replace('-', '_').replace('.', '_')}" + elif a.action == "count-chain": + name = _acct_counter(a.table) if name not in counters: counters.append(name) self.out(f"counter {name} {{", 1) self.out("}", 1) + chains = set() + for a in self.cfg.accounting: + if a.chain != "accounting": + chains.add(a.chain) + if a.action == "count-chain": + chains.add(a.table) + for chain in sorted(chains): + self.out(f"chain {_acct_chain(chain)} {{", 1) + emit_chain_body(chain) + self.out("}", 1) self.out("chain accounting {", 1) self.out("type filter hook forward priority filter - 5;", 2) - for idx, a in enumerate(self.cfg.accounting): - m = [] - if a.in_iface: - m.append(f'iifname "{a.in_iface}"') - if a.out_iface: - m.append(f'oifname "{a.out_iface}"') - if a.saddr: - m.append(f"{ipkw} saddr {_addr_set(a.saddr)}") - if a.daddr: - m.append(f"{ipkw} daddr {_addr_set(a.daddr)}") - comment = f' comment "{a.origin}"' if a.origin else "" - if a.net: - name = f"acct_{a.table.replace('-', '_')}_{idx}" - for sel in (f"{ipkw} saddr", f"{ipkw} daddr"): - self.out(" ".join(m) + f" {sel} {a.net} update " - f"@{name} {{ {sel} }}{comment}", 2) - else: - name = f"acct_{a.table.replace('-', '_').replace('.', '_')}" - self.out(" ".join(m) + - f' counter name "{name}"{comment}', 2) + emit_chain_body("accounting") self.out("}", 1) def _mangle_statement(self, r): @@ -1580,6 +1622,46 @@ def render_stop(cfg): f"table {table} {{", ] + # Keep every referenced set in the stopped table so stop does not remove + # objects live traffic policy depends on. Externally-filled sets are still + # snapshotted/restored by the lifecycle script; static sets carry their + # compiled elements as they do in the started table. + addr_type = "ipv6_addr" if cfg.family == 6 else "ipv4_addr" + sets = set() + _collect_sets(cfg, sets) + for name in sorted(sets): + if name.startswith("geoip:"): + lines.append(f" set geoip_{name.split(':', 1)[1]} {{") + lines.append(f" type {addr_type}; flags interval;") + lines.append(" }") + lines.append("") + continue + defn = cfg.ipsets.get(name) + static = bool(defn and defn.elements) + interval = defn is None or defn.settype == "hash:net" + timeout = defn.timeout if defn else 0 + flags = ["interval"] if interval else [] + if timeout: + flags.append("timeout") + declaration = f"type {addr_type};" + if flags: + declaration += " flags " + ", ".join(flags) + ";" + if interval: + declaration += " auto-merge;" + if timeout: + declaration += f" timeout {timeout}s;" + lines.append(f" set {name} {{") + lines.append(f" {declaration}") + if static: + lines.append(" elements = {") + elems = defn.elements + for i in range(0, len(elems), 8): + tail = "," if i + 8 < len(elems) else "" + lines.append(" " + ", ".join(elems[i:i + 8]) + tail) + lines.append(" }") + lines.append(" }") + lines.append("") + def chain(name, policy, body): lines.append(f" chain {name} {{") lines.append(f" type filter hook {name} priority filter; " diff --git a/src/shorewall_nft/model.py b/src/shorewall_nft/model.py index 1d5492a..ef730ae 100644 --- a/src/shorewall_nft/model.py +++ b/src/shorewall_nft/model.py @@ -205,6 +205,8 @@ class AcctRule: saddr: str = "" daddr: str = "" origin: str = "" + action: str = "account" # account, count-chain, count or done + chain: str = "accounting" @dataclass diff --git a/src/shorewall_nft/parsers.py b/src/shorewall_nft/parsers.py index a39cebf..41b8ea4 100644 --- a/src/shorewall_nft/parsers.py +++ b/src/shorewall_nft/parsers.py @@ -932,6 +932,8 @@ def parse_conntrack(path, variables, interfaces): ACCT_RE = re.compile(r"^ACCOUNT\((?P[\w.-]+),(?P[^)]+)\)$") COUNT_RE = re.compile(r"^(?P[\w.-]+):COUNT$") +DONE_RE = re.compile(r"^DONE$") +PLAIN_COUNT_RE = re.compile(r"^COUNT$") def parse_accounting(path, variables, interfaces): @@ -941,13 +943,14 @@ def parse_accounting(path, variables, interfaces): cols = split_columns(line.text, line.path, line.lineno) m = ACCT_RE.match(cols[0]) cm = COUNT_RE.match(cols[0]) - if not m and not cm: + done = DONE_RE.match(cols[0]) + count = PLAIN_COUNT_RE.match(cols[0]) + if not m and not cm and not done and not count: raise line.error(f"unsupported accounting action {cols[0]}; " - "ACCOUNT(table,net) and name:COUNT are " - "supported") + "ACCOUNT(table,net), name:COUNT, COUNT and " + "DONE are supported") chain = cols[1] if len(cols) > 1 else "-" - if chain != "-": - raise line.error("accounting CHAIN column not supported yet") + chain = "accounting" if chain == "-" else chain source = cols[2] if len(cols) > 2 and cols[2] != "-" else "" dest = cols[3] if len(cols) > 3 and cols[3] != "-" else "" origin = f"{os.path.basename(line.path)}:{line.lineno}" @@ -964,11 +967,22 @@ def side(spec): out.append(AcctRule(table=m.group("table"), net=m.group("net"), in_iface=s_iface or s_addr, out_iface=d_iface or d_addr, - origin=origin)) - else: + origin=origin, chain=chain)) + elif cm: out.append(AcctRule(table=cm.group("name"), net="", in_iface=s_iface, out_iface=d_iface, - origin=origin, saddr=s_addr, daddr=d_addr)) + origin=origin, saddr=s_addr, daddr=d_addr, + action="count-chain", chain=chain)) + elif count: + out.append(AcctRule(table="", net="", in_iface=s_iface, + out_iface=d_iface, origin=origin, + saddr=s_addr, daddr=d_addr, action="count", + chain=chain)) + else: + out.append(AcctRule(table="", net="", in_iface=s_iface, + out_iface=d_iface, origin=origin, + saddr=s_addr, daddr=d_addr, action="done", + chain=chain)) return out diff --git a/src/shorewall_nft/script.py b/src/shorewall_nft/script.py index 7c73200..e636230 100644 --- a/src/shorewall_nft/script.py +++ b/src/shorewall_nft/script.py @@ -606,7 +606,14 @@ def _simple_tc(cfg): ;; stop) run_stop + # Keep externally-filled sets live even in the stopped-state table. + # The stopped ruleset declares them empty; snapshot before replacing + # the table and restore their elements immediately afterwards. + save_dynamic_sets load_stop_ruleset || {{ echo "$0: stop ruleset load failed" >&2; exit 1; }} + for sf in "$STATE"/sets/*.nft; do + [ -e "$sf" ] && nft -f "$sf" 2>/dev/null || : + done clear_routing clear_tc clear_proxyarp diff --git a/tests/corpus/0041-ipset-persist/case.toml b/tests/corpus/0041-ipset-persist/case.toml index 45cfa55..7be0b7b 100644 --- a/tests/corpus/0041-ipset-persist/case.toml +++ b/tests/corpus/0041-ipset-persist/case.toml @@ -1,4 +1,4 @@ -description = "An externally-filled ipset survives a cold stop and start, not just a reload. A knock daemon fills +knoc_ssh, the admin runs savesets, then stops and starts the firewall. The knocked-in entry must come back. Regression: on start the set snapshot was captured before the ruleset loaded, and a cold start (no live set) deleted the snapshot instead of keeping it, so the entry was lost across a stop or reboot. Ours-verified; nft sets are ours-specific runtime state." +description = "An externally-filled ipset remains populated through stop and start, not just reload. A knock daemon fills +knoc_ssh, then stop snapshots and restores it into the stopped-state table automatically. The subsequent start must retain the entry without an explicit savesets command. Ours-verified; nft sets are ours-specific runtime state." family = 4 mode = "script" no_upstream = true @@ -34,13 +34,8 @@ proto = "tcp" port = 22 expect = "allow" -# Snapshot the live sets, as the admin does before a planned stop. -[[events]] -description = "savesets" -run = "savesets" - -# Stop the firewall. The stop ruleset drops, so the set is gone from the -# live table. +# Stop the firewall. Its drop policy still blocks SSH, but the dynamic set +# remains populated in the stopped-state table. [[events]] description = "stop" run = "stop" diff --git a/tests/harness/hardening-unit.py b/tests/harness/hardening-unit.py index 53c65ea..0ebdb19 100755 --- a/tests/harness/hardening-unit.py +++ b/tests/harness/hardening-unit.py @@ -5,6 +5,7 @@ # down, lsm --once state round-trips, and an externally filled set is an # interval set. Pure Python, no packets. import os +import shutil import sys import tempfile @@ -12,7 +13,7 @@ "..", "..", "src")) from shorewall_nft import ipsets, lsm # noqa: E402 from shorewall_nft.compile import load # noqa: E402 -from shorewall_nft.emit import render, _match_addr_alts # noqa: E402 +from shorewall_nft.emit import render, render_stop, _match_addr_alts # noqa: E402 from shorewall_nft.errors import ConfigError # noqa: E402 from shorewall_nft.lsm import Monitor, MonitorCfg, parse_lsm # noqa: E402 from shorewall_nft.parsers import parse_providers # noqa: E402 @@ -117,6 +118,24 @@ def parse_ipset_str(text): decl = text[i:i + 200] if i >= 0 else "" (ok if "flags interval" in decl and "auto-merge" in decl else bad)("emit: external set is an interval set with auto-merge") +stop_text = render_stop(cfg) +(ok if "set knoc_ssh {" in stop_text + else bad)("emit: stop ruleset keeps external set declarations") + +# --- stop ruleset keeps referenced defined ipsets too --- +tmp_conf = tempfile.mkdtemp(prefix="shorewall-nft-static-ipset-") +try: + shutil.copytree(os.path.join(REPO, "tests/corpus/0039-ipset-dynamic/config"), + tmp_conf, dirs_exist_ok=True) + with open(os.path.join(tmp_conf, "ipsets"), "w") as f: + f.write("create knoc_ssh hash:net\n") + f.write("add knoc_ssh 198.51.100.0/24\n") + cfg = load(tmp_conf, 4) + stop_text = render_stop(cfg) + (ok if "set knoc_ssh {" in stop_text and "198.51.100.0/24" in stop_text + else bad)("emit: stop ruleset keeps defined ipset declarations") +finally: + shutil.rmtree(tmp_conf) # --- a mixed address column fans out into one match per group --- alts = _match_addr_alts("1.2.3.4,192.168.1.0/24,+knoc", "saddr", "ip", set()) From c77a58e6800ab0d0e72fd9acbd9acb7839395458 Mon Sep 17 00:00:00 2001 From: Lindsay Harvey Date: Sat, 11 Jul 2026 22:53:36 +1000 Subject: [PATCH 2/2] fix named accounting chains to show per-direction counters instead of a shared aggregate --- src/shorewall_nft/cli.py | 1 + src/shorewall_nft/emit.py | 9 +-------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/shorewall_nft/cli.py b/src/shorewall_nft/cli.py index c4921cd..8fa03bb 100644 --- a/src/shorewall_nft/cli.py +++ b/src/shorewall_nft/cli.py @@ -510,6 +510,7 @@ def _show_accounting(family): target, detail = "DONE", "" elif named_count: target = named_count.group(1)[len("acct_"):] + target = re.sub(r"_\d+$", "", target) detail = "" else: target, detail = "COUNT", "" diff --git a/src/shorewall_nft/emit.py b/src/shorewall_nft/emit.py index e0c1eaa..0e3e3a2 100644 --- a/src/shorewall_nft/emit.py +++ b/src/shorewall_nft/emit.py @@ -1044,7 +1044,7 @@ def emit_chain_body(chain): emit_stmt(chain, f"counter jump {_acct_chain(a.table)}", a) if a.table == chain: - emit_stmt(chain, f'counter name "{_acct_counter(a.table)}"', a) + emit_stmt(chain, "counter", a) continue if a.chain != chain: continue @@ -1059,19 +1059,12 @@ def emit_chain_body(chain): emit_stmt(chain, "counter", a) self.out("") - counters = [] for idx, a in enumerate(self.cfg.accounting): if a.net: self.out(f"set acct_{_acct_ident(a.table)}_{idx} {{", 1) self.out(f"type {addr_type}; flags dynamic; counter; " "size 65535;", 2) self.out("}", 1) - elif a.action == "count-chain": - name = _acct_counter(a.table) - if name not in counters: - counters.append(name) - self.out(f"counter {name} {{", 1) - self.out("}", 1) chains = set() for a in self.cfg.accounting: if a.chain != "accounting":