[pve-devel] [PATCH firewall 5/7] Only enable multicast rules when needed

[Stefan Reiter]

In corosync 2.x multicast is the default, but if 'udpu' transport is
explicitly configured, we don't need to allow it.

Also, we filter incoming Multicast packets by their source.

Default behaviour prior to this patch series was to always allow
unicast, this has been kept (also needed to run current tests).

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

Unsure if this is correct behaviour, but I didn't want to change it in case
corosync sends unicast packages at some point even if it's configured for MC.

Disallowing Unicast as well would be easy, simply guard the addrule calls
with $unicast_only. It would require some not-quite trivial fixing of the
test cases though, since they currently assume both variants (UC/MC) to be open.

 src/PVE/Firewall.pm | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index ce3d937..a62ee21 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -2406,6 +2406,7 @@ sub enable_host_firewall {
     # corosync preparation
     my $corosync_rule = "-p udp --dport 5404:5405";
     my $corosync_local_addresses = {};
+    my $unicast_only;
     my $local_hostname = PVE::INotify::nodename();
     if (defined($corosync_conf)) {
 	PVE::Corosync::for_all_corosync_addresses($corosync_conf, $ipversion, sub {
@@ -2415,6 +2416,10 @@ sub enable_host_firewall {
 		$corosync_local_addresses->{$key} = $node_ip;
 	    }
 	});
+
+	# multicast is default in corosync 2
+	my $corosync_transport = $corosync_conf->{main}->{totem}->{transport};
+	$unicast_only = defined($corosync_transport) && $corosync_transport eq 'udpu';
     }
 
     # host inbound firewall
@@ -2463,9 +2468,6 @@ sub enable_host_firewall {
 
     # corosync inbound rules
     if (defined($corosync_conf)) {
-	# always allow multicast
-	ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action");
-
 	PVE::Corosync::for_all_corosync_addresses($corosync_conf, $ipversion, sub {
 	    my ($node_name, $node_ip, $node_ipversion, $key) = @_;
 	    my $destination = $corosync_local_addresses->{$key};
@@ -2474,6 +2476,9 @@ sub enable_host_firewall {
 		# accept only traffic on same ring
 		ruleset_addrule($ruleset, $chain, "-d $destination -s $node_ip $corosync_rule", "-j $accept_action");
 	    }
+
+	    ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST -s $node_ip $corosync_rule", "-j $accept_action")
+		if !$unicast_only;
 	});
     }
 
@@ -2529,8 +2534,8 @@ sub enable_host_firewall {
 
     # corosync outbound rules
     if (defined($corosync_conf)) {
-	# always allow multicast
-	ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action");
+	ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action")
+	    if !$unicast_only;
 
 	PVE::Corosync::for_all_corosync_addresses($corosync_conf, $ipversion, sub {
 	    my ($node_name, $node_ip, $node_ipversion, $key) = @_;
-- 
2.20.1