From 459c7079bebe7059d90441a5014d948a92d2ee19 Mon Sep 17 00:00:00 2001 From: sarthurdev <965089+sarthurdev@users.noreply.github.com> Date: Tue, 4 Jan 2022 20:11:31 +0100 Subject: firewall: zone-policy: T2199: T4130: Fixes for firewall, state-policy and zone-policy --- data/templates/firewall/nftables.tmpl | 10 ++-------- data/templates/zone_policy/nftables.tmpl | 14 ++++++++++++-- python/vyos/template.py | 6 +----- smoketest/scripts/cli/test_firewall.py | 4 ++-- src/conf_mode/firewall-interface.py | 31 +++++++++++++++++-------------- src/conf_mode/firewall.py | 14 ++++++-------- src/conf_mode/zone_policy.py | 6 ++++-- 7 files changed, 44 insertions(+), 41 deletions(-) diff --git a/data/templates/firewall/nftables.tmpl b/data/templates/firewall/nftables.tmpl index bbb111b1f..68e83de64 100644 --- a/data/templates/firewall/nftables.tmpl +++ b/data/templates/firewall/nftables.tmpl @@ -46,11 +46,8 @@ define P_{{ group_name }} = { table ip filter { {% if first_install is defined %} - chain VYOS_FW_IN { + chain VYOS_FW_FORWARD { type filter hook forward priority 0; policy accept; - } - chain VYOS_FW_OUT { - type filter hook forward priority 1; policy accept; jump VYOS_POST_FW } chain VYOS_FW_LOCAL { @@ -104,11 +101,8 @@ table ip filter { table ip6 filter { {% if first_install is defined %} - chain VYOS_FW6_IN { + chain VYOS_FW6_FORWARD { type filter hook forward priority 0; policy accept; - } - chain VYOS_FW6_OUT { - type filter hook forward priority 1; policy accept; jump VYOS_POST_FW6 } chain VYOS_FW6_LOCAL { diff --git a/data/templates/zone_policy/nftables.tmpl b/data/templates/zone_policy/nftables.tmpl index 21230c688..fae6e8c4f 100644 --- a/data/templates/zone_policy/nftables.tmpl +++ b/data/templates/zone_policy/nftables.tmpl @@ -81,7 +81,7 @@ table ip6 filter { insert rule ip filter VYOS_FW_LOCAL counter jump VZONE_{{ zone_name }}_IN insert rule ip filter VYOS_FW_OUTPUT counter jump VZONE_{{ zone_name }}_OUT {% else %} -insert rule ip filter VYOS_FW_OUT oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE_{{ zone_name }} +insert rule ip filter VYOS_FW_FORWARD oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE_{{ zone_name }} {% endif %} {% endif %} {% if zone_conf.ipv6 %} @@ -89,9 +89,19 @@ insert rule ip filter VYOS_FW_OUT oifname { {{ zone_conf.interface | join(',') } insert rule ip6 filter VYOS_FW6_LOCAL counter jump VZONE6_{{ zone_name }}_IN insert rule ip6 filter VYOS_FW6_OUTPUT counter jump VZONE6_{{ zone_name }}_OUT {% else %} -insert rule ip6 filter VYOS_FW6_OUT oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE6_{{ zone_name }} +insert rule ip6 filter VYOS_FW6_FORWARD oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE6_{{ zone_name }} {% endif %} {% endif %} {% endfor %} +{# Ensure that state-policy rule is first in the chain #} +{% if firewall.state_policy is defined %} +{% for chain in ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL'] %} +insert rule ip filter {{ chain }} jump VYOS_STATE_POLICY +{% endfor %} +{% for chain in ['VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL'] %} +insert rule ip6 filter {{ chain }} jump VYOS_STATE_POLICY6 +{% endfor %} +{% endif %} + {% endif %} diff --git a/python/vyos/template.py b/python/vyos/template.py index 7671bf377..6f65c6c98 100644 --- a/python/vyos/template.py +++ b/python/vyos/template.py @@ -526,11 +526,7 @@ def nft_state_policy(conf, state, ipv6=False): out.append('counter') if 'action' in conf: - if conf['action'] == 'accept': - jump_target = 'VYOS_POST_FW6' if ipv6 else 'VYOS_POST_FW' - out.append(f'jump {jump_target}') - else: - out.append(conf['action']) + out.append(conf['action']) return " ".join(out) diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py index 5f728f0cd..2b3b354ba 100755 --- a/smoketest/scripts/cli/test_firewall.py +++ b/smoketest/scripts/cli/test_firewall.py @@ -142,8 +142,8 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase): self.cli_commit() chains = { - 'ip filter': ['VYOS_FW_IN', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL'], - 'ip6 filter': ['VYOS_FW6_IN', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL'] + 'ip filter': ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL'], + 'ip6 filter': ['VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL'] } for table in ['ip filter', 'ip6 filter']: diff --git a/src/conf_mode/firewall-interface.py b/src/conf_mode/firewall-interface.py index 516fa6c48..b0df9dff4 100755 --- a/src/conf_mode/firewall-interface.py +++ b/src/conf_mode/firewall-interface.py @@ -32,13 +32,13 @@ from vyos import airbag airbag.enable() NFT_CHAINS = { - 'in': 'VYOS_FW_IN', - 'out': 'VYOS_FW_OUT', + 'in': 'VYOS_FW_FORWARD', + 'out': 'VYOS_FW_FORWARD', 'local': 'VYOS_FW_LOCAL' } NFT6_CHAINS = { - 'in': 'VYOS_FW6_IN', - 'out': 'VYOS_FW6_OUT', + 'in': 'VYOS_FW6_FORWARD', + 'out': 'VYOS_FW6_FORWARD', 'local': 'VYOS_FW6_LOCAL' } @@ -91,11 +91,11 @@ def verify(if_firewall): def generate(if_firewall): return None -def cleanup_rule(table, chain, ifname, new_name=None): +def cleanup_rule(table, chain, prefix, ifname, new_name=None): results = cmd(f'nft -a list chain {table} {chain}').split("\n") retval = None for line in results: - if f'ifname "{ifname}"' in line: + if f'{prefix}ifname "{ifname}"' in line: if new_name and f'jump {new_name}' in line: # new_name is used to clear rules for any previously referenced chains # returns true when rule exists and doesn't need to be created @@ -108,6 +108,7 @@ def cleanup_rule(table, chain, ifname, new_name=None): return retval def state_policy_handle(table, chain): + # Find any state-policy rule to ensure interface rules are only inserted afterwards results = cmd(f'nft -a list chain {table} {chain}').split("\n") for line in results: if 'jump VYOS_STATE_POLICY' in line: @@ -126,11 +127,12 @@ def apply(if_firewall): name = dict_search_args(if_firewall, direction, 'name') if name: - rule_exists = cleanup_rule('ip filter', chain, ifname, name) - rule_action = 'insert' - rule_prefix = '' + rule_exists = cleanup_rule('ip filter', chain, if_prefix, ifname, name) if not rule_exists: + rule_action = 'insert' + rule_prefix = '' + handle = state_policy_handle('ip filter', chain) if handle: rule_action = 'add' @@ -138,15 +140,16 @@ def apply(if_firewall): run(f'nft {rule_action} rule ip filter {chain} {rule_prefix} {if_prefix}ifname {ifname} counter jump {name}') else: - cleanup_rule('ip filter', chain, ifname) + cleanup_rule('ip filter', chain, if_prefix, ifname) ipv6_name = dict_search_args(if_firewall, direction, 'ipv6_name') if ipv6_name: - rule_exists = cleanup_rule('ip6 filter', ipv6_chain, ifname, ipv6_name) - rule_action = 'insert' - rule_prefix = '' + rule_exists = cleanup_rule('ip6 filter', ipv6_chain, if_prefix, ifname, ipv6_name) if not rule_exists: + rule_action = 'insert' + rule_prefix = '' + handle = state_policy_handle('ip filter', chain) if handle: rule_action = 'add' @@ -154,7 +157,7 @@ def apply(if_firewall): run(f'nft {rule_action} rule ip6 filter {ipv6_chain} {rule_prefix} {if_prefix}ifname {ifname} counter jump {ipv6_name}') else: - cleanup_rule('ip6 filter', ipv6_chain, ifname) + cleanup_rule('ip6 filter', ipv6_chain, if_prefix, ifname) return None diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py index 8e037c679..6016d94fa 100755 --- a/src/conf_mode/firewall.py +++ b/src/conf_mode/firewall.py @@ -53,14 +53,12 @@ preserve_chains = [ 'INPUT', 'FORWARD', 'OUTPUT', - 'VYOS_FW_IN', - 'VYOS_FW_OUT', + 'VYOS_FW_FORWARD', 'VYOS_FW_LOCAL', 'VYOS_FW_OUTPUT', 'VYOS_POST_FW', 'VYOS_FRAG_MARK', - 'VYOS_FW6_IN', - 'VYOS_FW6_OUT', + 'VYOS_FW6_FORWARD', 'VYOS_FW6_LOCAL', 'VYOS_FW6_OUTPUT', 'VYOS_POST_FW6', @@ -228,7 +226,7 @@ def cleanup_commands(firewall): commands.append(f'delete chain {table} {chain}') elif 'rule' in item: rule = item['rule'] - if rule['chain'] in ['VYOS_FW_IN', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL', 'VYOS_FW6_IN', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']: + if rule['chain'] in ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL', 'VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']: if 'expr' in rule and any([True for expr in rule['expr'] if dict_search_args(expr, 'jump', 'target') == state_chain]): if 'state_policy' not in firewall: chain = rule['chain'] @@ -303,7 +301,7 @@ def post_apply_trap(firewall): def state_policy_rule_exists(): # Determine if state policy rules already exist in nft - search_str = cmd(f'nft list chain ip filter VYOS_FW_IN') + search_str = cmd(f'nft list chain ip filter VYOS_FW_FORWARD') return 'VYOS_STATE_POLICY' in search_str def apply(firewall): @@ -317,10 +315,10 @@ def apply(firewall): raise ConfigError('Failed to apply firewall') if 'state_policy' in firewall and not state_policy_rule_exists(): - for chain in ['VYOS_FW_IN', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL']: + for chain in ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL']: cmd(f'nft insert rule ip filter {chain} jump VYOS_STATE_POLICY') - for chain in ['VYOS_FW6_IN', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']: + for chain in ['VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']: cmd(f'nft insert rule ip6 filter {chain} jump VYOS_STATE_POLICY6') apply_sysfs(firewall) diff --git a/src/conf_mode/zone_policy.py b/src/conf_mode/zone_policy.py index 2535ea33b..d605e9639 100755 --- a/src/conf_mode/zone_policy.py +++ b/src/conf_mode/zone_policy.py @@ -152,7 +152,9 @@ def cleanup_commands(): continue for expr in item['rule']['expr']: target = dict_search_args(expr, 'jump', 'target') - if target and target.startswith("VZONE"): + if not target: + continue + if target.startswith("VZONE") or target.startswith("VYOS_STATE_POLICY"): commands.append(f'delete rule {table} {chain} handle {handle}') for item in obj['nftables']: if 'chain' in item: @@ -180,7 +182,7 @@ def generate(zone_policy): def apply(zone_policy): install_result = run(f'nft -f {nftables_conf}') - if install_result == 1: + if install_result != 0: raise ConfigError('Failed to apply zone-policy') return None -- cgit v1.2.3