summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Topp <andrewt@telekinetica.net>2024-08-04 17:52:57 +1000
committerAndrew Topp <andrewt@telekinetica.net>2024-08-04 17:52:57 +1000
commit60b0614296874c144665417130d4881461114db0 (patch)
tree404eb8bf72582b60cad69d9c23535b41a49094f6
parent15c77978f30bebe7c6d4f4e9a87c56e12e1382cd (diff)
downloadvyos-1x-60b0614296874c144665417130d4881461114db0.tar.gz
vyos-1x-60b0614296874c144665417130d4881461114db0.zip
firewall: T4694: Adding GRE flags & fields matches to firewall rules
* Only matching flags and fields used by modern RFC2890 "extended GRE" - this is backwards-compatible, but does not match all possible flags. * There are no nftables helpers for the GRE key field, which is critical to match individual tunnel sessions (more detail in the forum post) * nft expression syntax is not flexible enough for multiple field matches in a single rule and the key offset changes depending on flags. * Thus, clumsy compromise in requiring an explicit match on the "checksum" flag if a key is present, so we know where key will be. In most cases, nobody uses the checksum, but assuming it to be off or automatically adding a "not checksum" match unless told otherwise would be confusing * The automatic "flags key" check when specifying a key doesn't have similar validation, I added it first and it makes sense. I would still like to find a workaround to the "checksum" offset problem. * If we could add 2 rules from 1 config definition, we could match both cases with appropriate offsets, but this would break existing FW generation logic, logging, etc. * Added a "test_gre_match" smoketest
-rw-r--r--interface-definitions/include/firewall/common-rule-inet.xml.i1
-rw-r--r--interface-definitions/include/firewall/gre.xml.i116
-rw-r--r--python/vyos/firewall.py61
-rwxr-xr-xsmoketest/scripts/cli/test_firewall.py55
-rwxr-xr-xsrc/conf_mode/firewall.py30
5 files changed, 263 insertions, 0 deletions
diff --git a/interface-definitions/include/firewall/common-rule-inet.xml.i b/interface-definitions/include/firewall/common-rule-inet.xml.i
index 0acb08ec9..e44938b14 100644
--- a/interface-definitions/include/firewall/common-rule-inet.xml.i
+++ b/interface-definitions/include/firewall/common-rule-inet.xml.i
@@ -19,5 +19,6 @@
#include <include/firewall/synproxy.xml.i>
#include <include/firewall/tcp-flags.xml.i>
#include <include/firewall/tcp-mss.xml.i>
+#include <include/firewall/gre.xml.i>
#include <include/firewall/time.xml.i>
<!-- include end -->
diff --git a/interface-definitions/include/firewall/gre.xml.i b/interface-definitions/include/firewall/gre.xml.i
new file mode 100644
index 000000000..285233434
--- /dev/null
+++ b/interface-definitions/include/firewall/gre.xml.i
@@ -0,0 +1,116 @@
+<!-- include start from firewall/gre.xml.i -->
+<node name="gre">
+ <properties>
+ <help>GRE fields to match</help>
+ </properties>
+ <children>
+ <node name="flags">
+ <properties>
+ <help>GRE flag bits to match</help>
+ </properties>
+ <children>
+ <node name="key">
+ <properties>
+ <help>Header includes optional key field</help>
+ </properties>
+ <children>
+ <leafNode name="unset">
+ <properties>
+ <help>Header does not include optional key field</help>
+ <valueless/>
+ </properties>
+ </leafNode>
+ </children>
+ </node>
+ <node name="checksum">
+ <properties>
+ <help>Header includes optional checksum</help>
+ </properties>
+ <children>
+ <leafNode name="unset">
+ <properties>
+ <help>Header does not include optional checksum</help>
+ <valueless/>
+ </properties>
+ </leafNode>
+ </children>
+ </node>
+ <node name="sequence">
+ <properties>
+ <help>Header includes a sequence number field</help>
+ </properties>
+ <children>
+ <leafNode name="unset">
+ <properties>
+ <help>Header does not include a sequence number field</help>
+ <valueless/>
+ </properties>
+ </leafNode>
+ </children>
+ </node>
+ </children>
+ </node>
+ <leafNode name="inner-proto">
+ <properties>
+ <help>EtherType of encapsulated packet</help>
+ <completionHelp>
+ <list>ip ip6 arp 802.1q 802.1ad</list>
+ </completionHelp>
+ <valueHelp>
+ <format>u32:0-65535</format>
+ <description>Ethernet protocol number</description>
+ </valueHelp>
+ <valueHelp>
+ <format>u32:0x0-0xffff</format>
+ <description>Ethernet protocol number (hex)</description>
+ </valueHelp>
+ <valueHelp>
+ <format>ip</format>
+ <description>IPv4</description>
+ </valueHelp>
+ <valueHelp>
+ <format>ip6</format>
+ <description>IPv6</description>
+ </valueHelp>
+ <valueHelp>
+ <format>arp</format>
+ <description>Address Resolution Protocol</description>
+ </valueHelp>
+ <valueHelp>
+ <format>802.1q</format>
+ <description>VLAN-tagged frames (IEEE 802.1q)</description>
+ </valueHelp>
+ <valueHelp>
+ <format>802.1ad</format>
+ <description>Provider Bridging (IEEE 802.1ad, Q-in-Q)</description>
+ </valueHelp>
+ <valueHelp>
+ <format>gretap</format>
+ <description>Transparent Ethernet Bridging (L2 Ethernet over GRE, gretap)</description>
+ </valueHelp>
+ <constraint>
+ <regex>(ip|ip6|arp|802.1q|802.1ad|gretap|0x[0-9a-fA-F]{1,4})</regex>
+ <validator name="numeric" argument="--range 0-65535"/>
+ </constraint>
+ </properties>
+ </leafNode>
+ #include <interface/parameters-key.xml.i>
+ <leafNode name="version">
+ <properties>
+ <help>GRE Version</help>
+ <valueHelp>
+ <format>gre</format>
+ <description>Standard GRE</description>
+ </valueHelp>
+ <valueHelp>
+ <format>pptp</format>
+ <description>Point to Point Tunnelling Protocol</description>
+ </valueHelp>
+ <constraint>
+ <regex>(gre|pptp)</regex>
+ </constraint>
+ </properties>
+ </leafNode>
+ </children>
+</node>
+<!-- include end -->
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index cac6d2953..3976a5580 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -409,6 +409,41 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
time = rule_conf['recent']['time']
output.append(f'add @RECENT{def_suffix}_{hook}_{fw_name}_{rule_id} {{ {ip_name} saddr limit rate over {count}/{time} burst {count} packets }}')
+ if 'gre' in rule_conf:
+ gre_key = dict_search_args(rule_conf, 'gre', 'key')
+
+ gre_flags = dict_search_args(rule_conf, 'gre', 'flags')
+ output.append(parse_gre_flags(gre_flags or {}, force_keyed=gre_key is not None))
+
+ gre_proto_alias_map = {
+ '802.1q': '8021q',
+ '802.1ad': '8021ad',
+ 'gretap': '0x6558',
+ }
+
+ gre_proto = dict_search_args(rule_conf, 'gre', 'inner_proto')
+ if gre_proto is not None:
+ gre_proto = gre_proto_alias_map.get(gre_proto, gre_proto)
+ output.append(f'gre protocol {gre_proto}')
+
+ gre_ver = dict_search_args(rule_conf, 'gre', 'version')
+ if gre_ver == 'gre':
+ output.append('gre version 0')
+ elif gre_ver == 'pptp':
+ output.append('gre version 1')
+
+ if gre_key:
+ # The offset of the key within the packet shifts depending on the C-flag.
+ # nftables cannot handle complex enough expressions to match multiple
+ # offsets based on bitfields elsewhere.
+ # We enforce a specific match for the checksum flag in validation, so the
+ # gre_flags dict will always have a 'checksum' key when gre_key is populated.
+ if not gre_flags['checksum']:
+ # No "unset" child node means C is set, we offset key lookup +32 bits
+ output.append(f'@th,64,32 == {gre_key}')
+ else:
+ output.append(f'@th,32,32 == {gre_key}')
+
if 'time' in rule_conf:
output.append(parse_time(rule_conf['time']))
@@ -544,6 +579,32 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
output.append(f'comment "{family}-{hook}-{fw_name}-{rule_id}"')
return " ".join(output)
+def parse_gre_flags(flags, force_keyed=False):
+ flag_map = { # nft does not have symbolic names for these.
+ 'checksum': 1<<0,
+ 'routing': 1<<1,
+ 'key': 1<<2,
+ 'sequence': 1<<3,
+ 'strict_routing': 1<<4,
+ }
+
+ include = 0
+ exclude = 0
+ for fl_name, fl_state in flags.items():
+ if not fl_state:
+ include |= flag_map[fl_name]
+ else: # 'unset' child tag
+ exclude |= flag_map[fl_name]
+
+ if force_keyed:
+ # Implied by a key-match.
+ include |= flag_map['key']
+
+ if include == 0 and exclude == 0:
+ return '' # Don't bother extracting and matching no bits
+
+ return f'gre flags & {include + exclude} == {include}'
+
def parse_tcp_flags(flags):
include = [flag for flag in flags if flag != 'not']
exclude = list(flags['not']) if 'not' in flags else []
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index 551f8ce65..dfc816a42 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -1100,5 +1100,60 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
with self.assertRaises(ConfigSessionError):
self.cli_commit()
+ def test_gre_match(self):
+ name = 'smoketest-gre'
+
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'default-action', 'return'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'action', 'accept'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'protocol', 'gre'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'gre', 'flags', 'key'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'gre', 'flags', 'checksum', 'unset'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'gre', 'key', '1234'])
+ self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'log'])
+
+ self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '2', 'action', 'continue'])
+ self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '2', 'protocol', 'gre'])
+ self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '2', 'gre', 'inner-proto', '0x6558'])
+ self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '2', 'log'])
+
+ self.cli_set(['firewall', 'ipv6', 'input', 'filter', 'rule', '3', 'action', 'drop'])
+ self.cli_set(['firewall', 'ipv6', 'input', 'filter', 'rule', '3', 'protocol', 'gre'])
+ self.cli_set(['firewall', 'ipv6', 'input', 'filter', 'rule', '3', 'gre', 'flags', 'checksum'])
+ self.cli_set(['firewall', 'ipv6', 'input', 'filter', 'rule', '3', 'gre', 'key', '4321'])
+
+ self.cli_set(['firewall', 'ipv6', 'output', 'filter', 'rule', '4', 'action', 'reject'])
+ self.cli_set(['firewall', 'ipv6', 'output', 'filter', 'rule', '4', 'protocol', 'gre'])
+ self.cli_set(['firewall', 'ipv6', 'output', 'filter', 'rule', '4', 'gre', 'version', 'pptp'])
+
+ self.cli_commit()
+
+ nftables_search_v4 = [
+ ['gre protocol 0x6558', 'continue comment'],
+ ['gre flags & 5 == 4 @th,32,32 0x4d2', 'accept comment'],
+ ]
+
+ nftables_search_v6 = [
+ ['gre flags & 5 == 5 @th,64,32 0x10e1', 'drop comment'],
+ ['gre version 1', 'reject comment'],
+ ]
+
+ self.verify_nftables(nftables_search_v4, 'ip vyos_filter')
+ self.verify_nftables(nftables_search_v6, 'ip6 vyos_filter')
+
+ # GRE match will only work with protocol GRE
+ self.cli_delete(['firewall', 'ipv4', 'name', name, 'rule', '1', 'protocol', 'gre'])
+
+ with self.assertRaises(ConfigSessionError):
+ self.cli_commit()
+
+ self.cli_discard()
+
+ # GREv1 (PPTP) does not include a key field, match not available
+ self.cli_set(['firewall', 'ipv6', 'output', 'filter', 'rule', '4', 'gre', 'flags', 'checksum', 'unset'])
+ self.cli_set(['firewall', 'ipv6', 'output', 'filter', 'rule', '4', 'gre', 'key', '1234'])
+
+ with self.assertRaises(ConfigSessionError):
+ self.cli_commit()
+
if __name__ == '__main__':
unittest.main(verbosity=2)
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 02bf00bcc..b71ce7124 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -231,6 +231,36 @@ def verify_rule(firewall, family, hook, priority, rule_id, rule_conf):
if not {'count', 'time'} <= set(rule_conf['recent']):
raise ConfigError('Recent "count" and "time" values must be defined')
+ if 'gre' in rule_conf:
+ if dict_search_args(rule_conf, 'protocol') != 'gre':
+ raise ConfigError('Protocol must be gre when matching GRE flags and fields')
+
+ if dict_search_args(rule_conf, 'gre', 'key'):
+ if dict_search_args(rule_conf, 'gre', 'version') == 'pptp':
+ raise ConfigError('GRE tunnel keys are not present in PPTP')
+
+ if dict_search_args(rule_conf, 'gre', 'flags', 'checksum') is None:
+ # There is no builtin match in nftables for the GRE key, so we need to do a raw lookup.
+ # The offset of the key within the packet shifts depending on the C-flag.
+ # 99% of the time, nobody will have checksums enabled - it's usually a manual config option.
+ # We can either assume it is unset unless otherwise directed
+ # (confusing, requires doco to explain why it doesn't work sometimes)
+ # or, demand an explicit selection to be made for this specific match rule.
+ # This check enforces the latter. The user is free to create rules for both cases.
+ raise ConfigError('Matching GRE tunnel key requires an explicit checksum flag match. For most cases, use "gre flags checksum unset"')
+
+ if dict_search_args(rule_conf, 'gre', 'flags', 'key', 'unset') is not None:
+ raise ConfigError('Matching GRE tunnel key implies "flags key", cannot specify "flags key unset"')
+
+ gre_inner_proto = dict_search_args(rule_conf, 'gre', 'inner_proto')
+ if gre_inner_proto is not None:
+ try:
+ gre_inner_value = int(gre_inner_proto, 0)
+ if gre_inner_value < 0 or gre_inner_value > 65535:
+ raise ConfigError('inner-proto outside valid ethertype range 0-65535')
+ except ValueError:
+ pass # Symbolic constant, pre-validated before reaching here.
+
tcp_flags = dict_search_args(rule_conf, 'tcp', 'flags')
if tcp_flags:
if dict_search_args(rule_conf, 'protocol') != 'tcp':