Re: [pve-devel] [PATCH V2 pve-firewall] add cluster ebtables_dst_macfilter option.

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

Sorry for the late reply.

On Fri, Sep 10, 2021 at 05:34:29PM +0200, Alexandre Derumier wrote:
> This new option allow filtering of destination macs for ingress traffic.
> 
> This is a protection from bad/hosting networks (like hetzner) flooding
> traffic with non-hosted mac.
> 
> To be fast, one rule, this use the "--among-dst mac,mac,mac,mac," syntax.
> broadcast mac ff:ff:ff:ff:ff:ff is always allowed
> 
> currently, ebtables-restore segfault if too many are defined
> https://www.spinics.net/lists/netfilter/msg55995.html
> 
> So, I'm using "--among-dst-file", loading macs from an external file.

It's a little awkward but works, I guess, however, it does mess with the
digests we use to verify the ruleset and therefore keeps logging errors
in syslog. This will need fixing.

How many entries can you have before this starts happening anyway?
And how many entries do you expect there to be?

> Note that "ebtables-save" still show the syntax with "--among-dst mac,mac,mac,"
> (with a comma at the end), so I compile the full mac list with --among-dst to
> compare, and if update is needed, I'm writing the dst file in
> /var/lib/pve-firewall/chain-macfilter, and replace among-dst syntax by among-dst-file
> 
> Changelog v2:
>  - as we use one rule for performance, add all vms/ct macaddress when vm firewall is enabled.
>    (even if vm macfilter option is disabled).
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/Firewall.pm | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index edc5336..8277ee0 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1221,6 +1221,11 @@ our $cluster_option_properties = {
>  	default => 1,
>  	optional => 1,
>      },
> +    ebtables_dst_macfilter => {
> +	description => "Filtering VM/CT destination mac for ingress traffic.",
> +	type => 'boolean',
> +	optional => 1,
> +    },
>      policy_in => {
>  	description => "Input policy.",
>  	type => 'string',
> @@ -2867,7 +2872,7 @@ sub parse_clusterfw_option {
>  	if (($value > 1) && ((time() - $value) > 60)) {
>  	    $value = 0
>  	}
> -    } elsif ($line =~ m/^(ebtables):\s*(0|1)\s*$/i) {
> +    } elsif ($line =~ m/^(ebtables|ebtables_dst_macfilter):\s*(0|1)\s*$/i) {
>  	$opt = lc($1);
>  	$value = int($2);
>      } elsif ($line =~ m/^(policy_(in|out)):\s*(ACCEPT|DROP|REJECT)\s*$/i) {
> @@ -3948,11 +3953,19 @@ sub compile_ebtables_filter {
>      ruleset_create_chain($ruleset, "PVEFW-FORWARD");
>  
>      ruleset_create_chain($ruleset, "PVEFW-FWBR-OUT");
> +
> +    if ($cluster_conf->{options}->{ebtables_dst_macfilter}) {
> +	#filtering destination mac for ipv4/ipv6
> +	ruleset_create_chain($ruleset, "PVEFW-FWBR-IN");
> +	ruleset_addrule($ruleset, 'PVEFW-FORWARD', '-i fwln+', '-j PVEFW-FWBR-IN');
> +    }
> +
>      #for ipv4 and ipv6, check macaddress in iptables, so we use conntrack 'ESTABLISHED', to speedup rules
>      ruleset_addrule($ruleset, 'PVEFW-FORWARD', '-p IPv4', '-j ACCEPT');
>      ruleset_addrule($ruleset, 'PVEFW-FORWARD', '-p IPv6', '-j ACCEPT');
>      ruleset_addrule($ruleset, 'PVEFW-FORWARD', '-o fwln+', '-j PVEFW-FWBR-OUT');
>  
> +    my $maclist = [];
>      # generate firewall rules for QEMU VMs
>      foreach my $vmid (sort keys %{$vmdata->{qemu}}) {
>  	eval {
> @@ -3975,7 +3988,7 @@ sub compile_ebtables_filter {
>  			push(@$arpfilter, $ip);
>  		    }
>  		}
> -		generate_tap_layer2filter($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter);
> +		generate_tap_layer2filter($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter, $maclist);
>  	    }
>  	};
>  	warn $@ if $@; # just to be sure - should not happen
> @@ -4012,17 +4025,23 @@ sub compile_ebtables_filter {
>  			push @$arpfilter, $ip;
>  		    }
>  		}
> -		generate_tap_layer2filter($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter);
> +		generate_tap_layer2filter($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter, $maclist);
>  	    }
>  	};
>  	warn $@ if $@; # just to be sure - should not happen
>      }
>  
> +    if ($cluster_conf->{options}->{'ebtables_dst_macfilter'} && @$maclist > 0) {
> +	push @$maclist, 'ff:ff:ff:ff:ff:ff';  #allow broadcast mac
> +	my $maclist_str = join ',',sort(@$maclist);
> +	ruleset_addrule($ruleset, 'PVEFW-FWBR-IN', "--among-dst ! $maclist_str,", '-j DROP');
> +    }
> +
>      return $ruleset;
>  }
>  
>  sub generate_tap_layer2filter {
> -    my ($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter) = @_;
> +    my ($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter, $maclist) = @_;
>      my $options = $vmfw_conf->{options};
>  
>      my $tapchain = $iface."-OUT";
> @@ -4037,6 +4056,10 @@ sub generate_tap_layer2filter {
>  	    ruleset_addrule($ruleset, $tapchain, "-s ! $macaddr", '-j DROP');
>      }
>  
> +    if (defined($macaddr)) {
> +	push @$maclist, $macaddr;
> +    }
> +
>      if (@$arpfilter){
>  	my $arpchain = $tapchain."-ARP";
>  	ruleset_addrule($ruleset, $tapchain, "-p ARP", "-j $arpchain");
> @@ -4225,6 +4248,15 @@ sub get_ebtables_cmdlist {
>  		next if ! $pve_include;
>  		$pve_include = 0;
>  	    }
> +
> +	    if ($cmd =~ m/^-A (\S+) --among-dst ! (\S+) -j DROP/) {
> +		my $chain = $1;
> +		my $maclist_raw = $2."\n";
> +		my $filename = "$pve_fw_status_dir/ebtables_macfilter-$chain";
> +		PVE::Tools::file_set_contents($filename, $maclist_raw);
> +		$cmd = "-A $1 --among-dst-file ! $filename -j DROP";
> +	    }
> +
>  	    $cmdlist .= "$cmd\n";
>  	}
>      }
> -- 
> 2.30.2