Merge pull request #5573 from c-po/firewall-fixes

firewall: T9474: multiple issues in rule processing (time based rules, fragment matching and failing commits)

8 files changed, 236 insertions, 14 deletions

diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index d820292c6..2f0529965 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -26,6 +26,7 @@ from vyos.configdict import is_node_changed
 from vyos.configdiff import Diff, get_config_diff
 from vyos.configdep import set_dependents, call_dependents
 from vyos.configverify import verify_interface_exists
+from vyos.defaults import nft_time_refresh
 from vyos.ethtool import Ethtool
 from vyos.firewall import fqdn_config_parse
 from vyos.geoip import geoip_refresh, geoip_update
@@ -150,9 +151,6 @@ def get_config(config=None):
 
     fqdn_config_parse(firewall, 'firewall')
 
-    if not os.path.exists(nftables_conf):
-        firewall['first_install'] = True
-
     if 'zone' in firewall:
         for local_zone, local_zone_conf in firewall['zone'].items():
             if 'local_zone' not in local_zone_conf:
@@ -786,6 +784,9 @@ def apply(firewall):
     # Apply firewall global-options sysctl settings
     cmdl(['sysctl', '-f', sysctl_file])
 
+    # hour matches were converted to UTC with the current offset
+    call(f'{nft_time_refresh} --arm')
+
     call_dependents()
 
     ## DOMAIN RESOLVER
diff --git a/src/conf_mode/policy_local-route.py b/src/conf_mode/policy_local-route.py
index 23aadfade..e58cce041 100755
--- a/src/conf_mode/policy_local-route.py
+++ b/src/conf_mode/policy_local-route.py
@@ -22,6 +22,7 @@ from vyos.configdict import dict_merge
 from vyos.configdict import node_changed
 from vyos.configdict import leaf_node_changed
 from vyos.configverify import verify_interface_exists
+from vyos.defaults import user_mark_mask
 from vyos.utils.process import call
 from vyos import ConfigError
 from vyos import airbag
@@ -282,7 +283,7 @@ def apply(pbr):
                     f_src_port = '' if src_port == '' else f' sport {src_port} '
                     f_dst = '' if dst == '' else f' to {dst} '
                     f_dst_port = '' if dst_port == '' else f' dport {dst_port} '
-                    f_fwmk = '' if fwmk == '' else f' fwmark {fwmk} '
+                    f_fwmk = '' if fwmk == '' else f' fwmark {fwmk}/{user_mark_mask} '
                     f_iif = '' if iif == '' else f' iif {iif} '
                     f_proto = '' if proto == '' else f' ipproto {proto} '
                     f_table = '' if table_or_vrf == '' else f' lookup {table_or_vrf} '
@@ -320,7 +321,8 @@ def apply(pbr):
                         f_dst = f' to {dst} ' if dst else ''
                         f_src_port = f' sport {source_port} ' if source_port else ''
                         f_dst_port = f' dport {destination_port} ' if destination_port else ''
-                        f_fwmk = f' fwmark {fwmark} ' if fwmark else ''
+                        # fragments carry bit 31 of the mark while being routed
+                        f_fwmk = f' fwmark {fwmark}/{user_mark_mask} ' if fwmark else ''
                         f_iif = f' iif {inbound_interface} ' if inbound_interface else ''
                         f_proto = f' ipproto {protocol} ' if protocol else ''
 
diff --git a/src/conf_mode/policy_route.py b/src/conf_mode/policy_route.py
index 64bb7b323..4ca9a5643 100755
--- a/src/conf_mode/policy_route.py
+++ b/src/conf_mode/policy_route.py
@@ -32,8 +32,10 @@ from vyos.utils.process import cmdl
 from vyos.utils.process import run
 from vyos.utils.network import get_vrf_tableid
 from vyos.utils.network import interface_exists
+from vyos.defaults import nft_time_refresh
 from vyos.defaults import rt_global_table
 from vyos.defaults import rt_global_vrf
+from vyos.defaults import user_mark_mask
 from vyos.geoip import  geoip_refresh, geoip_update
 from vyos import ConfigError
 from vyos import airbag
@@ -265,8 +267,9 @@ def apply_table_marks(policy):
                                 continue
                             tables.append(vrf_table_id)
                             table_mark = mark_offset - vrf_table_id
+                            # fragments carry bit 31 of the mark while being routed
                             cmdl(cmd_list + ['rule', 'add', 'pref', str(vrf_table_id),
-                                              'fwmark', str(table_mark), 'table', str(vrf_table_id)])
+                                              'fwmark', f'{table_mark}/{user_mark_mask}', 'table', str(vrf_table_id)])
 
 def cleanup_table_marks():
     for cmd_list in [['ip'], ['ip', '-6']]:
@@ -280,6 +283,9 @@ def cleanup_table_marks():
             if fwmark[:2] == '0x':
                 fwmark = int(fwmark, 16)
             if (int(fwmark) == (mark_offset - table)):
+                # a rule with a mask is only found again with the same mask
+                if 'fwmask' in rule:
+                    fwmark = f'{fwmark}/{rule["fwmask"]}'
                 cmdl(cmd_list + ['rule', 'del', 'fwmark', str(fwmark), 'table', str(table)])
 
 def apply(policy):
@@ -292,6 +298,9 @@ def apply(policy):
 
     apply_table_marks(policy)
 
+    # hour matches were converted to UTC with the current offset
+    call(f'{nft_time_refresh} --arm')
+
     update_domain_resolver(policy)
 
     if policy['geoip_sets']['name'] or policy['geoip_sets']['ipv6_name']:
diff --git a/src/conf_mode/system_conntrack.py b/src/conf_mode/system_conntrack.py
index 559aebcc8..7e659d1f6 100755
--- a/src/conf_mode/system_conntrack.py
+++ b/src/conf_mode/system_conntrack.py
@@ -180,9 +180,6 @@ def verify(conntrack):
     return None
 
 def generate(conntrack):
-    if not os.path.exists(nftables_ct_file):
-        conntrack['first_install'] = True
-
     if 'log' not in conntrack:
         # Remove old conntrack-logger config and return
         if os.path.exists(vyos_conntrack_logger_config):
diff --git a/src/conf_mode/system_timezone.py b/src/conf_mode/system_timezone.py
index 54ffb88ee..2f1830900 100755
--- a/src/conf_mode/system_timezone.py
+++ b/src/conf_mode/system_timezone.py
@@ -18,6 +18,8 @@ import sys
 
 from copy import deepcopy
 from vyos.config import Config
+from vyos.configdep import set_dependents
+from vyos.configdep import call_dependents
 from vyos import ConfigError
 from vyos.utils.process import call
 
@@ -37,6 +39,12 @@ def get_config(config=None):
     if conf.exists('system time-zone'):
         tz['name'] = conf.return_value('system time-zone')
 
+    # nft converts hour matches with the offset of the time-zone in use
+    if conf.exists('firewall'):
+        set_dependents('firewall', conf)
+    if conf.exists('policy route') or conf.exists('policy route6'):
+        set_dependents('policy_route', conf)
+
     return tz
 
 def verify(tz):
@@ -49,6 +57,8 @@ def apply(tz):
     call('/usr/bin/timedatectl set-timezone {}'.format(tz['name']))
     call('systemctl restart rsyslog')
 
+    call_dependents()
+
 if __name__ == '__main__':
     try:
         c = get_config()
diff --git a/src/helpers/vyos-load-balancer.py b/src/helpers/vyos-load-balancer.py
index 677938174..d525621e0 100755
--- a/src/helpers/vyos-load-balancer.py
+++ b/src/helpers/vyos-load-balancer.py
@@ -192,11 +192,6 @@ def restore_default_route(lb: dict, ifname: str) -> None:
 
 def nftables_update(lb):
     # Atomically reload nftables table from template
-    if not os.path.exists(nftables_wlb_conf):
-        lb['first_install'] = True
-    elif 'first_install' in lb:
-        del lb['first_install']
-
     render(nftables_wlb_conf, 'load-balancing/nftables-wlb.j2', lb)
 
     rc, out = rc_cmd(f'nft -f {nftables_wlb_conf}')
diff --git a/src/helpers/vyos-nft-time-refresh.py b/src/helpers/vyos-nft-time-refresh.py
new file mode 100755
index 000000000..c52567840
--- /dev/null
+++ b/src/helpers/vyos-nft-time-refresh.py
@@ -0,0 +1,165 @@
+#!/usr/bin/env python3
+#
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# nft converts "meta hour" to UTC when a ruleset is loaded, using the UTC
+# offset of that moment. After a daylight saving change every hour window is
+# off by the difference until the rules are loaded again. This helper loads
+# only the affected rules again, in place, so sets and counters are kept, and
+# arms itself for the next change of the UTC offset.
+
+import argparse
+import json
+import re
+import sys
+
+from datetime import datetime
+from datetime import timedelta
+from datetime import timezone
+from zoneinfo import ZoneInfo
+
+from vyos.defaults import nft_time_refresh
+from vyos.utils.commit import acquire_commit_lock_file
+from vyos.utils.commit import release_commit_lock_file
+from vyos.utils.process import cmdl
+from vyos.utils.process import rc_cmd
+
+# rendered by conf_mode/firewall.py and conf_mode/policy_route.py
+rule_files = ['/run/nftables.conf', '/run/nftables_policy.conf']
+unit_prefix = 'vyos-nft-time-refresh'
+
+table_re = re.compile(r'^\s*table\s+(\S+)\s+(\S+)\s*\{')
+chain_re = re.compile(r'^\s*chain\s+(\S+)\s*\{')
+comment_re = re.compile(r'comment "([^"]+)"')
+
+def hour_rules(path: str) -> dict:
+    """Return {(family, table, chain, comment): rule} for rules with an hour match"""
+    rules = {}
+    family = table = chain = None
+    try:
+        with open(path) as f:
+            lines = f.read().splitlines()
+    except FileNotFoundError:
+        return rules
+
+    for line in lines:
+        if match := table_re.match(line):
+            family, table = match.groups()
+            chain = None
+        elif match := chain_re.match(line):
+            chain = match.group(1)
+        elif chain and 'hour "' in line and (match := comment_re.search(line)):
+            rules[(family, table, chain, match.group(1))] = line.strip()
+    return rules
+
+def all_hour_rules() -> dict:
+    rules = {}
+    for path in rule_files:
+        rules.update(hour_rules(path))
+    return rules
+
+def rule_handles(family: str, table: str) -> dict:
+    """Return {(family, table, chain, comment): handle} of a loaded table"""
+    handles = {}
+    rc, out = rc_cmd(f'nft -j -a list table {family} {table}')
+    if rc != 0:
+        return handles
+    for item in json.loads(out)['nftables']:
+        rule = item.get('rule')
+        if rule and 'comment' in rule:
+            key = (rule['family'], rule['table'], rule['chain'], rule['comment'])
+            handles[key] = rule['handle']
+    return handles
+
+def replace_batch(rules: dict) -> list:
+    """nft commands replacing every rendered hour rule found in the ruleset"""
+    batch = []
+    handles = {}
+    for family, table in {(k[0], k[1]) for k in rules}:
+        handles.update(rule_handles(family, table))
+    for key, rule in sorted(rules.items()):
+        if key in handles:
+            family, table, chain, _ = key
+            batch.append(f'replace rule {family} {table} {chain} handle {handles[key]} {rule}')
+    return batch
+
+def next_offset_change(now: datetime, tz: ZoneInfo, horizon_days=400):
+    """First moment after now with a different UTC offset, or None"""
+    offset = now.astimezone(tz).utcoffset()
+    lo = now
+    for _ in range(horizon_days):
+        hi = lo + timedelta(days=1)
+        if hi.astimezone(tz).utcoffset() != offset:
+            # narrow down to the second
+            while hi - lo > timedelta(seconds=1):
+                mid = lo + (hi - lo) / 2
+                if mid.astimezone(tz).utcoffset() == offset:
+                    lo = mid
+                else:
+                    hi = mid
+            return hi.replace(microsecond=0)
+        lo = hi
+    return None
+
+def arm():
+    """Schedule a refresh at the next UTC offset change if any rule needs it"""
+    # stopping only the timers keeps a refresh which is running right now
+    rc_cmd(f"systemctl stop '{unit_prefix}-*.timer'")
+    if not all_hour_rules():
+        return
+
+    with open('/etc/localtime', 'rb') as f:
+        tz = ZoneInfo.from_file(f)
+    change = next_offset_change(datetime.now(timezone.utc), tz)
+    if not change:
+        return
+
+    when = change + timedelta(seconds=5)
+    # unique unit name - the unit of a running refresh still exists while it
+    # arms the next one
+    unit = f'{unit_prefix}-{int(when.timestamp())}'
+    # timers default to AccuracySec=1min, which leaves a window up to a minute long
+    cmdl(['systemd-run', f'--unit={unit}', '--timer-property=RemainAfterElapse=no',
+          '--timer-property=AccuracySec=1s',
+          f'--on-calendar={when:%Y-%m-%d %H:%M:%S} UTC', nft_time_refresh, '--refresh'])
+
+def refresh():
+    batch = replace_batch(all_hour_rules())
+    if not batch:
+        return
+    rc, out = rc_cmd('nft -f -', input='\n'.join(batch) + '\n')
+    if rc != 0:
+        print(f'Failed to refresh time based rules: {out}', file=sys.stderr)
+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    group = parser.add_mutually_exclusive_group(required=True)
+    group.add_argument('--arm', action='store_true', help='Schedule a refresh for the next UTC offset change')
+    group.add_argument('--refresh', action='store_true', help='Load rules with an hour match again')
+    args = parser.parse_args()
+
+    if args.arm:
+        arm()
+        sys.exit(0)
+
+    # a commit loads the rules itself and arms the next refresh
+    lock, _ = acquire_commit_lock_file()
+    if lock is None:
+        sys.exit(0)
+    try:
+        refresh()
+        arm()
+    finally:
+        release_commit_lock_file(lock)
diff --git a/src/tests/test_firewall_time.py b/src/tests/test_firewall_time.py
new file mode 100644
index 000000000..92da6a06d
--- /dev/null
+++ b/src/tests/test_firewall_time.py
@@ -0,0 +1,43 @@
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from unittest import TestCase
+
+from vyos.firewall import parse_time
+
+class TestFirewallParseTime(TestCase):
+    def test_hour_window(self):
+        self.assertEqual(parse_time({'starttime': '08:00', 'stoptime': '17:00'}),
+                         'hour "08:00"-"17:00"')
+
+    def test_hour_window_overnight(self):
+        self.assertEqual(parse_time({'starttime': '22:00', 'stoptime': '06:00'}),
+                         'hour "22:00"-"06:00"')
+
+    def test_hour_start_only(self):
+        self.assertEqual(parse_time({'starttime': '01:00'}),
+                         'hour "01:00"-"23:59:59"')
+
+    def test_hour_stop_only(self):
+        self.assertEqual(parse_time({'stoptime': '17:00'}),
+                         'hour "00:00"-"17:00"')
+
+    def test_date_and_time(self):
+        self.assertEqual(parse_time({'startdate': '2026-01-01', 'starttime': '08:00:00',
+                                     'stopdate': '2026-12-31', 'stoptime': '18:00:00'}),
+                         'time >= "2026-01-01 08:00:00" time < "2026-12-31 18:00:00"')
+
+    def test_startdate_with_daily_stoptime(self):
+        self.assertEqual(parse_time({'startdate': '2026-01-01', 'stoptime': '18:00'}),
+                         'time >= "2026-01-01" hour "00:00"-"18:00"')