rpms/firewalld
6
0
Fork 0
Code Issues Pull Requests Releases Activity

feat(rich): support "burst" attribute to limit in rich rules

Browse Source 
Resolves: RHEL-9316
This commit is contained in:
Eric Garver 2024-06-13 17:05:53 -04:00
parent 9c7750524f
commit b8c444eec4
5 changed files with 525 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
0023-v2.2.0-fix-rich-fix-range-check-for-large-rule-limit.patch Normal file
View File

@ -0,0 +1,27 @@
From 39e8946ba75fc3ce36c3ff72e3af1fb2ae0d95ec Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 5 Feb 2024 13:24:25 +0100
Subject: [PATCH 23/26] v2.2.0: fix(rich): fix range check for large rule limit
Fixes: 555ae1307a3e ('New rich language usable in zones')
(cherry picked from commit e790c64ebb5760e8d8f8afd1b978baab891d5933)
---
src/firewall/core/rich.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/firewall/core/rich.py b/src/firewall/core/rich.py
index 6a03eeca5d8a..b150a0dca402 100644
--- a/src/firewall/core/rich.py
+++ b/src/firewall/core/rich.py
@@ -264,7 +264,7 @@ class Rich_Limit(object):
elif duration == "d":
mult = 24*60*60
- if 10000 * mult / rate == 0:
+ if 10000 * mult // rate == 0:
raise FirewallError(errors.INVALID_LIMIT,
"%s too fast" % self.value)
--
2.43.0

63
0024-v2.2.0-improvement-policy-extract-helper-function-fo.patch Normal file
View File

@ -0,0 +1,63 @@
From 028529e33ed45507bcb1f3eb2722de3344eea091 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 5 Feb 2024 13:09:02 +0100
Subject: [PATCH 24/26] v2.2.0: improvement(policy): extract helper function
for writing limit rule element
Soon the Rich_Limit will also get a burst attribute. Then _handler_add_rich_limit()
will become more complicated. We wouldn't want to duplicated that code.
(cherry picked from commit f662606891569f09553c73023a2f70086d137512)
---
src/firewall/core/io/policy.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/firewall/core/io/policy.py b/src/firewall/core/io/policy.py
index 514a20251ef4..66535e0d0368 100644
--- a/src/firewall/core/io/policy.py
+++ b/src/firewall/core/io/policy.py
@@ -372,6 +372,11 @@ def common_check_config(obj, config, item, all_config):
),
)
+
+def _handler_add_rich_limit(handler, limit):
+ handler.simpleElement("limit", {"value": limit.value})
+
+
def common_writer(obj, handler):
# short
if obj.short and obj.short != "":
@@ -533,8 +538,7 @@ def common_writer(obj, handler):
handler.ignorableWhitespace(" ")
handler.startElement("log", attrs)
handler.ignorableWhitespace("\n ")
- handler.simpleElement("limit",
- { "value": rule.log.limit.value })
+ _handler_add_rich_limit(handler, rule.log.limit)
handler.ignorableWhitespace("\n ")
handler.endElement("log")
else:
@@ -549,8 +553,7 @@ def common_writer(obj, handler):
handler.ignorableWhitespace(" ")
handler.startElement("audit", { })
handler.ignorableWhitespace("\n ")
- handler.simpleElement("limit",
- { "value": rule.audit.limit.value })
+ _handler_add_rich_limit(handler, rule.audit.limit)
handler.ignorableWhitespace("\n ")
handler.endElement("audit")
else:
@@ -579,8 +582,7 @@ def common_writer(obj, handler):
handler.ignorableWhitespace(" ")
handler.startElement(action, attrs)
handler.ignorableWhitespace("\n ")
- handler.simpleElement("limit",
- { "value": rule.action.limit.value })
+ _handler_add_rich_limit(handler, rule.action.limit)
handler.ignorableWhitespace("\n ")
handler.endElement(action)
else:
--
2.43.0

189
0025-v2.2.0-improvement-rich-add-Rich_Limit.value_parse-a.patch Normal file
View File

@ -0,0 +1,189 @@
From 2844fedea7b0c65d864f9960b513150c4468adb2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 13 Dec 2023 19:42:37 +0100
Subject: [PATCH 25/26] v2.2.0: improvement(rich): add Rich_Limit.value_parse()
and normalize value
Instead of duplicating the parsing, add a Rich_Limit.value_parse()
function that can be used to "understand" the value string.
Note that already previously, Rich_Limit.__init__() would normalize the
value (e.g. modify "/minute" to "/m"). Go one step further with this.
Now parse and stringify the value, so that it is normalized. Invalid
values are left unnormalized, and Rich_Limit.__init__() still does not
fail the operation (like before). For that we have check().
This normalization matters. For example, the parser is (rightfully)
graceful and will accept 'limit value="1 /m"'. If we add two rules
that are identical, except for the white space, we want that the
normalize string is identical. That's useful, because the normalized
string of a rule is used as identity for the rule.
(cherry picked from commit 8d2f9502db98b349cabf76bb9c0a303fe6e3512a)
---
src/firewall-config.in | 6 +--
src/firewall/core/nftables.py | 9 ++---
src/firewall/core/rich.py | 76 ++++++++++++++++++++++-------------
3 files changed, 53 insertions(+), 38 deletions(-)
diff --git a/src/firewall-config.in b/src/firewall-config.in
index f91e945ca7de..e4fbb029ac6d 100755
--- a/src/firewall-config.in
+++ b/src/firewall-config.in
@@ -3245,7 +3245,7 @@ class FirewallConfig(object):
if old_obj.action.limit:
self.richRuleDialogActionLimitCheck.set_active(True)
- (rate, duration) = old_obj.action.limit.value.split("/")
+ (rate, duration) = old_obj.action.limit.value_parse()
self.richRuleDialogActionLimitRateEntry.set_text(rate)
combobox_select_text( \
self.richRuleDialogActionLimitDurationCombobox,
@@ -3288,7 +3288,7 @@ class FirewallConfig(object):
loglevel[log_level])
if old_obj.log.limit:
self.richRuleDialogLogLimitCheck.set_active(True)
- (rate, duration) = old_obj.log.limit.value.split("/")
+ (rate, duration) = old_obj.log.limit.value_parse()
self.richRuleDialogLogLimitRateEntry.set_text(rate)
combobox_select_text( \
self.richRuleDialogLogLimitDurationCombobox,
@@ -3299,7 +3299,7 @@ class FirewallConfig(object):
self.richRuleDialogAuditCheck.set_active(True)
if old_obj.audit.limit:
self.richRuleDialogAuditLimitCheck.set_active(True)
- (rate, duration) = old_obj.audit.limit.value.split("/")
+ (rate, duration) = old_obj.audit.limit.value_parse()
self.richRuleDialogAuditLimitRateEntry.set_text(rate)
combobox_select_text( \
self.richRuleDialogAuditLimitDurationCombobox,
diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py
index 67fb6457e86c..f24095ce729c 100644
--- a/src/firewall/core/nftables.py
+++ b/src/firewall/core/nftables.py
@@ -1071,13 +1071,10 @@ class nftables(object):
"d" : "day",
}
- try:
- i = limit.value.index("/")
- except ValueError:
- raise FirewallError(INVALID_RULE, "Expected '/' in limit")
+ rate, duration = limit.value_parse()
- return {"limit": {"rate": int(limit.value[0:i]),
- "per": rich_to_nft[limit.value[i+1]]}}
+ return {"limit": {"rate": rate,
+ "per": rich_to_nft[duration]}}
def _rich_rule_chain_suffix(self, rich_rule):
if type(rich_rule.element) in [Rich_Masquerade, Rich_ForwardPort, Rich_IcmpBlock]:
diff --git a/src/firewall/core/rich.py b/src/firewall/core/rich.py
index b150a0dca402..a77f2b4aa495 100644
--- a/src/firewall/core/rich.py
+++ b/src/firewall/core/rich.py
@@ -230,54 +230,72 @@ class Rich_Mark(object):
# value is uint32
raise FirewallError(errors.INVALID_MARK, x)
+DURATION_TO_MULT = {
+ "s": 1,
+ "m": 60,
+ "h": 60 * 60,
+ "d": 24 * 60 * 60,
+}
+
class Rich_Limit(object):
def __init__(self, value):
self.value = value
- if "/" in self.value:
- splits = self.value.split("/")
- if len(splits) == 2 and \
- splits[1] in [ "second", "minute", "hour", "day" ]:
- self.value = "%s/%s" % (splits[0], splits[1][:1])
def check(self):
+ self.value_parse()
+
+ @property
+ def value(self):
+ return self._value
+
+ @value.setter
+ def value(self, value):
+ if value is None:
+ self._value = None
+ return
+ try:
+ rate, duration = self._value_parse(value)
+ except FirewallError:
+ # The value is invalid. We cannot normalize it.
+ v = value
+ else:
+ v = f"{rate}/{duration}"
+ if getattr(self, "_value", None) != v:
+ self._value = v
+
+ @staticmethod
+ def _value_parse(value):
splits = None
- if "/" in self.value:
- splits = self.value.split("/")
+ if "/" in value:
+ splits = value.split("/")
if not splits or len(splits) != 2:
- raise FirewallError(errors.INVALID_LIMIT, self.value)
+ raise FirewallError(errors.INVALID_LIMIT, value)
(rate, duration) = splits
try:
rate = int(rate)
except:
- raise FirewallError(errors.INVALID_LIMIT, self.value)
+ raise FirewallError(errors.INVALID_LIMIT, value)
- if rate < 1 or duration not in [ "s", "m", "h", "d" ]:
- raise FirewallError(errors.INVALID_LIMIT, self.value)
+ if duration in ["second", "minute", "hour", "day"]:
+ duration = duration[:1]
- mult = 1
- if duration == "s":
- mult = 1
- elif duration == "m":
- mult = 60
- elif duration == "h":
- mult = 60*60
- elif duration == "d":
- mult = 24*60*60
+ if rate < 1 or duration not in ["s", "m", "h", "d"]:
+ raise FirewallError(errors.INVALID_LIMIT, value)
- if 10000 * mult // rate == 0:
- raise FirewallError(errors.INVALID_LIMIT,
- "%s too fast" % self.value)
+ if 10000 * DURATION_TO_MULT[duration] // rate == 0:
+ raise FirewallError(errors.INVALID_LIMIT, "%s too fast" % (value,))
if rate == 1 and duration == "d":
# iptables (v1.4.21) doesn't accept 1/d
- raise FirewallError(errors.INVALID_LIMIT,
- "%s too slow" % self.value)
+ raise FirewallError(errors.INVALID_LIMIT, "%s too slow" % (value,))
- def __str__(self):
- return 'limit value="%s"' % (self.value)
+ return rate, duration
- def command(self):
- return ''
+ def value_parse(self):
+ return self._value_parse(self._value)
+
+ def __str__(self):
+ return f'limit value="{self._value}"'
class Rich_Rule(object):
priority_min = -32768
--
2.43.0

238
0026-v2.2.0-improvement-rich-support-burst-attribute-to-l.patch Normal file
View File

@ -0,0 +1,238 @@
From 45ebffc5521db62064f365f4a9100b4ab40dce90 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 13 Dec 2023 20:35:51 +0100
Subject: [PATCH 26/26] v2.2.0: improvement(rich): support "burst" attribute to
limit in rich rules
For iptables, this is `-m limit --limit rate/suffix --limit-burst burst`.
For nftables, this is `limit rate [over] packet_number / TIME_UNIT [burst packet_number packets]`
Not implemented in `src/firewall-config.in`.
https://issues.redhat.com/browse/RHEL-9316
(cherry picked from commit 58dfdcafabaaad639bfcf389ebbd6b2c242965a4)
---
src/firewall/core/io/policy.py | 9 +++--
src/firewall/core/io/zone.py | 1 +
src/firewall/core/ipXtables.py | 9 +++--
src/firewall/core/nftables.py | 12 +++++--
src/firewall/core/rich.py | 63 ++++++++++++++++++++++++++++++----
src/tests/cli/firewall-cmd.at | 4 +--
6 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/src/firewall/core/io/policy.py b/src/firewall/core/io/policy.py
index 66535e0d0368..c732324c441b 100644
--- a/src/firewall/core/io/policy.py
+++ b/src/firewall/core/io/policy.py
@@ -278,7 +278,7 @@ def common_startElement(obj, name, attrs):
obj._rule_error = True
return True
value = attrs["value"]
- obj._limit_ok.limit = rich.Rich_Limit(value)
+ obj._limit_ok.limit = rich.Rich_Limit(value, attrs.get("burst"))
else:
return False
@@ -374,7 +374,11 @@ def common_check_config(obj, config, item, all_config):
def _handler_add_rich_limit(handler, limit):
- handler.simpleElement("limit", {"value": limit.value})
+ d = {"value": limit.value}
+ burst = limit.burst
+ if burst is not None:
+ d["burst"] = burst
+ handler.simpleElement("limit", d)
def common_writer(obj, handler):
@@ -652,6 +656,7 @@ class Policy(IO_Object):
"destination": [ "address", "invert", "ipset" ],
"log": [ "prefix", "level" ],
"reject": [ "type" ],
+ "limit": ["burst"],
}
def __init__(self):
diff --git a/src/firewall/core/io/zone.py b/src/firewall/core/io/zone.py
index 0c419ee0f2bd..753036e4fb55 100644
--- a/src/firewall/core/io/zone.py
+++ b/src/firewall/core/io/zone.py
@@ -94,6 +94,7 @@ class Zone(IO_Object):
"destination": [ "address", "invert", "ipset" ],
"log": [ "prefix", "level" ],
"reject": [ "type" ],
+ "limit": ["burst"],
}
@staticmethod
diff --git a/src/firewall/core/ipXtables.py b/src/firewall/core/ipXtables.py
index 401377104ce1..0f9a1518380e 100644
--- a/src/firewall/core/ipXtables.py
+++ b/src/firewall/core/ipXtables.py
@@ -967,9 +967,12 @@ class ip4tables(object):
return rules
def _rule_limit(self, limit):
- if limit:
- return [ "-m", "limit", "--limit", limit.value ]
- return []
+ if not limit:
+ return []
+ s = ["-m", "limit", "--limit", limit.value]
+ if limit.burst is not None:
+ s += ["--limit-burst", limit.burst]
+ return s
def _rich_rule_chain_suffix(self, rich_rule):
if type(rich_rule.element) in [Rich_Masquerade, Rich_ForwardPort, Rich_IcmpBlock]:
diff --git a/src/firewall/core/nftables.py b/src/firewall/core/nftables.py
index f24095ce729c..834176c09cbc 100644
--- a/src/firewall/core/nftables.py
+++ b/src/firewall/core/nftables.py
@@ -1073,8 +1073,16 @@ class nftables(object):
rate, duration = limit.value_parse()
- return {"limit": {"rate": rate,
- "per": rich_to_nft[duration]}}
+ d = {
+ "rate": rate,
+ "per": rich_to_nft[duration],
+ }
+
+ burst = limit.burst_parse()
+ if burst is not None:
+ d["burst"] = burst
+
+ return {"limit": d}
def _rich_rule_chain_suffix(self, rich_rule):
if type(rich_rule.element) in [Rich_Masquerade, Rich_ForwardPort, Rich_IcmpBlock]:
diff --git a/src/firewall/core/rich.py b/src/firewall/core/rich.py
index a77f2b4aa495..c561709af0e2 100644
--- a/src/firewall/core/rich.py
+++ b/src/firewall/core/rich.py
@@ -238,11 +238,13 @@ DURATION_TO_MULT = {
}
class Rich_Limit(object):
- def __init__(self, value):
+ def __init__(self, value, burst=None):
self.value = value
+ self.burst = burst
def check(self):
self.value_parse()
+ self.burst_parse()
@property
def value(self):
@@ -263,6 +265,24 @@ class Rich_Limit(object):
if getattr(self, "_value", None) != v:
self._value = v
+ @property
+ def burst(self):
+ return self._burst
+
+ @burst.setter
+ def burst(self, burst):
+ if burst is None:
+ self._burst = None
+ return
+ try:
+ b = self._burst_parse(burst)
+ except FirewallError:
+ b = burst
+ else:
+ b = str(burst)
+ if getattr(self, "_burst", None) != b:
+ self._burst = b
+
@staticmethod
def _value_parse(value):
splits = None
@@ -294,8 +314,28 @@ class Rich_Limit(object):
def value_parse(self):
return self._value_parse(self._value)
+ @staticmethod
+ def _burst_parse(burst):
+ if burst is None:
+ return None
+ try:
+ b = int(burst)
+ except:
+ raise FirewallError(errors.INVALID_LIMIT, burst)
+
+ if b < 1 or b > 10_000_000:
+ raise FirewallError(errors.INVALID_LIMIT, burst)
+
+ return b
+
+ def burst_parse(self):
+ return self._burst_parse(self._burst)
+
def __str__(self):
- return f'limit value="{self._value}"'
+ s = f'limit value="{self._value}"'
+ if self._burst is not None:
+ s += f" burst={self._burst}"
+ return s
class Rich_Rule(object):
priority_min = -32768
@@ -368,7 +408,7 @@ class Rich_Rule(object):
'invert', 'value',
'port', 'protocol', 'to-port', 'to-addr',
'name', 'prefix', 'level', 'type',
- 'set']:
+ 'set', 'burst']:
raise FirewallError(errors.INVALID_RULE, "bad attribute '%s'" % attr_name)
else: # element
if element in ['rule', 'source', 'destination', 'protocol',
@@ -554,11 +594,20 @@ class Rich_Rule(object):
attrs.clear()
index = index -1 # return token to input
elif in_element == 'limit':
- if attr_name == 'value':
- attrs['limit'] = Rich_Limit(attr_value)
- in_elements.pop() # limit
+ if attr_name in ["value", "burst"]:
+ attrs[f"limit.{attr_name}"] = attr_value
else:
- raise FirewallError(errors.INVALID_RULE, "invalid 'limit' element")
+ if "limit.value" not in attrs:
+ raise FirewallError(
+ errors.INVALID_RULE, "invalid 'limit' element"
+ )
+ attrs["limit"] = Rich_Limit(
+ attrs["limit.value"], attrs.get("limit.burst")
+ )
+ attrs.pop("limit.value", None)
+ attrs.pop("limit.burst", None)
+ in_elements.pop() # limit
+ index = index - 1 # return token to input
index = index + 1
diff --git a/src/tests/cli/firewall-cmd.at b/src/tests/cli/firewall-cmd.at
index c4ab3108d37c..6c69f0ccebd4 100644
--- a/src/tests/cli/firewall-cmd.at
+++ b/src/tests/cli/firewall-cmd.at
@@ -1356,8 +1356,8 @@ FWD_START_TEST([rich rules good])
rich_rule_test([rule protocol value="ah" reject])
rich_rule_test([rule protocol value="esp" accept])
rich_rule_test([rule protocol value="sctp" log])
- rich_rule_test([rule family="ipv4" source address="192.168.0.0/24" service name="tftp" log prefix="tftp: " level="info" limit value="1/m" accept])
- rich_rule_test([rule family="ipv4" source not address="192.168.0.0/24" service name="dns" log prefix="dns: " level="info" limit value="2/m" drop])
+ rich_rule_test([rule family="ipv4" source address="192.168.0.0/24" service name="tftp" log prefix="tftp: " level="info" limit value="1/m" burst=5 accept])
+ rich_rule_test([rule family="ipv4" source not address="192.168.0.0/24" service name="dns" log prefix="dns: " level="info" limit value="2/m" burst=5 drop])
IF_HOST_SUPPORTS_IPV6_RULES([
rich_rule_test([rule family="ipv6" source address="1:2:3:4:6::" service name="radius" log prefix="dns -- " level="info" limit value="3/m" reject type="icmp6-addr-unreachable" limit value="20/m"])
rich_rule_test([rule family="ipv6" source address="1:2:3:4:6::" port port="4011" protocol="tcp" log prefix="port 4011: " level="info" limit value="4/m" drop])
--
2.43.0

9
firewalld.spec
View File

@ -1,7 +1,7 @@
Summary: A firewall daemon with D-Bus interface providing a dynamic firewall
Name: firewalld
Version: 0.9.11
Release: 7%{?dist}
Release: 8%{?dist}
URL: http://www.firewalld.org
License: GPLv2+
Source0: https://github.com/firewalld/firewalld/releases/download/v%{version}/firewalld-%{version}.tar.gz
@ -27,6 +27,10 @@ Patch19: 0019-v1.0.0-test-rich-destination-ipset.patch
Patch20: 0020-v1.0.0-test-rich-destination-ipset-verify-policy-sup.patch
Patch21: 0021-v2.1.0-feat-icmp-add-ICMPv6-Multicast-Listener-Disco.patch
Patch22: 0022-v2.1.0-fix-rich-validate-service-name-of-rich-rule.patch
Patch23: 0023-v2.2.0-fix-rich-fix-range-check-for-large-rule-limit.patch
Patch24: 0024-v2.2.0-improvement-policy-extract-helper-function-fo.patch
Patch25: 0025-v2.2.0-improvement-rich-add-Rich_Limit.value_parse-a.patch
Patch26: 0026-v2.2.0-improvement-rich-support-burst-attribute-to-l.patch
BuildArch: noarch
BuildRequires: autoconf
@ -228,6 +232,9 @@ desktop-file-install --delete-original \
%{_mandir}/man1/firewall-config*.1*
%changelog
* Thu Jun 13 2024 Eric Garver <egarver@redhat.com> - 0.9.11-8
- feat(rich): support "burst" attribute to limit in rich rules
* Thu Jun 13 2024 Eric Garver <egarver@redhat.com> - 0.9.11-7
- fix(rich): validate service name of rich rule