Re: [pve-devel] [PATCH pve-firewall v2 37/39] add configuration option for new nftables firewall

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 17/04/2024 um 15:54 schrieb Stefan Hanreich:
> Introduces new nftables configuration option that en/disables the new
> nftables firewall.
> 
> pve-firewall reads this option and only generates iptables rules when
> nftables is set to `0`. Conversely proxmox-firewall only generates
> nftables rules when the option is set to `1`.

this is missing from your v3, but I assume it staid the same.

> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/Firewall.pm | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 81a8798..b39843d 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1408,6 +1408,12 @@ our $host_option_properties = {
>  	default => 0,
>  	optional => 1
>      },
> +    nftables => {
> +	description => "Enable nftables based firewall",
> +	type => 'boolean',
> +	default => 0,
> +	optional => 1,
> +    },
>  };
>  
>  our $vm_option_properties = {
> @@ -2929,7 +2935,7 @@ sub parse_hostfw_option {
>  
>      my $loglevels = "emerg|alert|crit|err|warning|notice|info|debug|nolog";
>  
> -    if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood):\s*(0|1)\s*$/i) {
> +    if ($line =~ m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i) {
>  	$opt = lc($1);
>  	$value = int($2);
>      } elsif ($line =~ m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i) {
> @@ -4676,7 +4682,11 @@ sub remove_pvefw_chains_ebtables {
>  sub init {
>      my $cluster_conf = load_clusterfw_conf();
>      my $cluster_options = $cluster_conf->{options};
> -    my $enable = $cluster_options->{enable};
> +
> +    my $host_conf = load_hostfw_conf($cluster_conf);
> +    my $host_options = $host_conf->{options};
> +
> +    my $enable = $cluster_options->{enable} && !$host_options->{nftables};

should we also check for -x '/usr/libexec/proxmox/proxmox-firewall' to consider
the nftables option enabled, as otherwise one might tick the check box while not
having the proxmox-firewall package installed (we probably make that only a
Recommends for now), which would then mean no firewall at all..

>  
>      return if !$enable;
>  
> @@ -4689,12 +4699,14 @@ sub update {
>  	my $cluster_conf = load_clusterfw_conf();
>  	my $cluster_options = $cluster_conf->{options};
>  
> -	if (!$cluster_options->{enable}) {
> +	my $hostfw_conf = load_hostfw_conf($cluster_conf);
> +	my $host_options = $hostfw_conf->{options};
> +
> +	if (!$cluster_options->{enable} || $host_options->{nftables}) {

same here, maybe add this in a helper

>  	    PVE::Firewall::remove_pvefw_chains();
>  	    return;
>  	}
>  
> -	my $hostfw_conf = load_hostfw_conf($cluster_conf);
>  
>  	my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = compile($cluster_conf, $hostfw_conf);
>  



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel