Re: [pve-devel] [PATCH v4 firewall 1/2] implement fail2ban backend and API

[Dominik Csapak <d.csapak@proxmox.com>]

while the code looks ok IMHO, i have some general questions:
* does it really make sense to hard depend on fail2ban?
   could it not also make sense to have it as 'recommends' or 'suggests'?
   setting enabled to 1 could then check if its installed and
   raise an error

* if we do not plan to add more fail2ban options in our config,
   i would rather see a combined fail2ban option (propertystring?)
   that would go into the general host firewall options

   that way we would not have to c&p the whole config parsing/setting api
   and could have a single new option line in the gui instead
   of a whole new panel with only 3 options (i think the majority of our
   users will not use fail2ban)

does that make sense to you?

On 10/11/21 12:57, Oguz Bektas wrote:
> adds a section "[FAIL2BAN]" in the hostfw configuration, which allows
> the properties 'maxretry' and 'bantime' (in minutes) for the GUI ports.
> 
> enable: whether fail2ban jail is enabled or not
> maxretry: amount of login tries allowed
> bantime: amount of minutes to ban suspicious host
> 
> the configuration file is derived from our wiki [0]
> 
> example API usage
> =====
> $ pvesh set /nodes/localhost/firewall/fail2ban --enable 1 --bantime 10 --maxretry 3
> 
> $ pvesh get /nodes/localhost/firewall/fail2ban
> ┌──────────┬───────┐
> │ key      │ value │
> ╞══════════╪═══════╡
> │ bantime  │ 10    │
> ├──────────┼───────┤
> │ enable   │ 1     │
> ├──────────┼───────┤
> │ maxretry │ 3     │
> └──────────┴───────┘
> 
> $ pvesh set /nodes/localhost/firewall/fail2ban --bantime 100
> $ pvesh get /nodes/localhost/firewall/fail2ban
> ┌──────────┬───────┐
> │ key      │ value │
> ╞══════════╪═══════╡
> │ bantime  │ 100   │
> ├──────────┼───────┤
> │ enable   │ 1     │
> ├──────────┼───────┤
> │ maxretry │ 3     │
> └──────────┴───────┘
> 
> $ pvesh set /nodes/localhost/firewall/fail2ban --enable 0
> $ pvesh get /nodes/localhost/firewall/fail2ban
> ┌──────────┬───────┐
> │ key      │ value │
> ╞══════════╪═══════╡
> │ bantime  │ 100   │
> ├──────────┼───────┤
> │ enable   │ 0     │
> ├──────────┼───────┤
> │ maxretry │ 3     │
> └──────────┴───────┘
> =====
> 
> [0]: https://pve.proxmox.com/wiki/Fail2ban
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v3->v4:
> * fix default values when enabling via API
> 
> 
>   debian/control                |   1 +
>   src/PVE/API2/Firewall/Host.pm |  98 +++++++++++++++++++++++++++++++++
>   src/PVE/Firewall.pm           | 101 +++++++++++++++++++++++++++++++++-
>   3 files changed, 199 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/control b/debian/control
> index 4684c5b..377c9ae 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -17,6 +17,7 @@ Package: pve-firewall
>   Architecture: any
>   Conflicts: ulogd,
>   Depends: ebtables,
> +         fail2ban,
>            ipset,
>            iptables,
>            libpve-access-control,
> diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm
> index b66ca55..535f188 100644
> --- a/src/PVE/API2/Firewall/Host.pm
> +++ b/src/PVE/API2/Firewall/Host.pm
> @@ -62,6 +62,17 @@ my $add_option_properties = sub {
>       return $properties;
>   };
>   
> +my $fail2ban_properties = $PVE::Firewall::fail2ban_option_properties;
> +
> +my $add_fail2ban_properties = sub {
> +    my ($properties) = @_;
> +
> +    foreach my $k (keys %$fail2ban_properties) {
> +	$properties->{$k} = $fail2ban_properties->{$k};
> +    }
> +
> +    return $properties;
> +};
>   
>   __PACKAGE__->register_method({
>       name => 'get_options',
> @@ -148,6 +159,93 @@ __PACKAGE__->register_method({
>   	return undef;
>       }});
>   
> +__PACKAGE__->register_method({
> +    name => 'get_fail2ban',
> +    path => 'fail2ban',
> +    method => 'GET',
> +    description => "Get host firewall fail2ban options.",
> +    proxyto => 'node',
> +    permissions => {
> +	check => ['perm', '/nodes/{node}', [ 'Sys.Audit' ]],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	},
> +    },
> +    returns => {
> +	type => "object",
> +	properties => $fail2ban_properties,
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> +	my $hostfw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);
> +
> +	return PVE::Firewall::copy_opject_with_digest($hostfw_conf->{fail2ban});
> +    }});
> +
> +
> +
> +__PACKAGE__->register_method({
> +    name => 'set_fail2ban',
> +    path => 'fail2ban',
> +    method => 'PUT',
> +    description => "Set host firewall fail2ban options.",
> +    protected => 1,
> +    proxyto => 'node',
> +    permissions => {
> +	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => &$add_fail2ban_properties({
> +	    node => get_standard_option('pve-node'),
> +	    delete => {
> +		type => 'string', format => 'pve-configid-list',
> +		description => "A list of settings you want to delete.",
> +		optional => 1,
> +	    },
> +	    digest => get_standard_option('pve-config-digest'),
> +	}),
> +    },
> +    returns => { type => "null" },
> +    code => sub {
> +	my ($param) = @_;
> +	PVE::Firewall::lock_hostfw_conf(10, sub {
> +	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
> +	    my $hostfw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);
> +
> +	    my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($hostfw_conf->{fail2ban});
> +	    PVE::Tools::assert_if_modified($digest, $param->{digest});
> +
> +	    if ($param->{delete}) {
> +		foreach my $opt (PVE::Tools::split_list($param->{delete})) {
> +		    raise_param_exc({ delete => "no such option '$opt'" })
> +			if !$fail2ban_properties->{$opt};
> +		    delete $hostfw_conf->{fail2ban}->{$opt};
> +		}
> +	    }
> +
> +	    if (defined($param->{enable})) {
> +		$param->{enable} = $param->{enable} ? 1 : 0;
> +		$hostfw_conf->{fail2ban}->{maxretry} = $param->{maxretry} ? $param->{maxretry} : $fail2ban_properties->{maxretry}->{default};
> +		$hostfw_conf->{fail2ban}->{bantime} = $param->{bantime} ? $param->{bantime} : $fail2ban_properties->{bantime}->{default};
> +	    }
> +
> +	    foreach my $k (keys %$fail2ban_properties) {
> +		next if !defined($param->{$k});
> +		$hostfw_conf->{fail2ban}->{$k} = $param->{$k};
> +	    }
> +
> +	    PVE::Firewall::save_hostfw_conf($hostfw_conf);
> +	});
> +
> +	return undef;
> +    }});
> +
>   __PACKAGE__->register_method({
>       name => 'log',
>       path => 'log',
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index edc5336..92b77a4 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1347,6 +1347,29 @@ our $host_option_properties = {
>       },
>   };
>   
> +our $fail2ban_option_properties = {
> +	enable => {
> +	    description => "Enable or disable fail2ban on a node.",
> +	    type => 'boolean',
> +	    optional => 1,
> +	    default => 1,
> +	},
> +	maxretry => {
> +	    description => "Amount of failed tries to ban after.",
> +	    type => 'integer',
> +	    optional => 1,
> +	    minimum => 1,
> +	    default => 3,
> +	},
> +	bantime => {
> +	    description => "Minutes to ban suspicious IPs.",
> +	    type => 'integer',
> +	    optional => 1,
> +	    minimum => 1,
> +	    default => 5,
> +	},
> +};
> +
>   our $vm_option_properties = {
>       enable => {
>   	description => "Enable/disable firewall rules.",
> @@ -2407,6 +2430,41 @@ sub ruleset_generate_vm_rules {
>       }
>   }
>   
> +sub generate_fail2ban_config {
> +    my ($fail2ban_opts) = @_;
> +
> +    my $enable = $fail2ban_opts->{enable} ? 'true' : 'false';
> +    my $maxretry = $fail2ban_opts->{maxretry};
> +    my $bantime = $fail2ban_opts->{bantime} * 60; # convert minutes to seconds
> +
> +    my $fail2ban_filter = <<CONFIG;
> +[Definition]
> +failregex = pvedaemon\\[.*authentication failure; rhost=<HOST> user=.* msg=.*
> +ignoreregex =
> +CONFIG
> +    my $filter_path = '/etc/fail2ban/filter.d/proxmox.conf';
> +    PVE::Tools::file_set_contents($filter_path, $fail2ban_filter) if !-f $filter_path;
> +
> +
> +    my $fail2ban_jail = <<CONFIG;
> +[proxmox]
> +enabled = $enable
> +port = https,http,8006
> +filter = proxmox
> +logpath = /var/log/daemon.log
> +maxretry = $maxretry
> +bantime = $bantime
> +CONFIG
> +
> +    my $jail_path = "/etc/fail2ban/jail.d/proxmox.conf";
> +    my $current_fail2ban_jail = PVE::Tools::file_get_contents($jail_path) if -f $jail_path;
> +
> +    if ($current_fail2ban_jail ne $fail2ban_jail) {
> +	PVE::Tools::file_set_contents($jail_path, $fail2ban_jail);
> +	run_command([qw(systemctl try-reload-or-restart fail2ban.service)]);
> +    }
> +}
> +
>   sub generate_nfqueue {
>       my ($options) = @_;
>   
> @@ -2937,6 +2995,16 @@ sub parse_alias {
>       return undef;
>   }
>   
> +sub parse_fail2ban_option {
> +    my ($line) = @_;
> +
> +    if ($line =~ m/^(enable|maxretry|bantime):\s+(\d+)(?:\s*#.*)?$/) {
> +	return ($1, int($2) // $fail2ban_option_properties->{$1}->{default});
> +    } else {
> +	die "error parsing fail2ban options: $line";
> +    }
> +}
> +
>   sub generic_fw_config_parser {
>       my ($filename, $cluster_conf, $empty_conf, $rule_env) = @_;
>   
> @@ -2965,6 +3033,11 @@ sub generic_fw_config_parser {
>   
>   	my $prefix = "$filename (line $linenr)";
>   
> +	if ($empty_conf->{fail2ban} && ($line =~ m/^\[fail2ban\]$/i)) {
> +	    $section = 'fail2ban';
> +	    next;
> +	}
> +
>   	if ($empty_conf->{options} && ($line =~ m/^\[options\]$/i)) {
>   	    $section = 'options';
>   	    next;
> @@ -3046,6 +3119,13 @@ sub generic_fw_config_parser {
>   		$res->{aliases}->{lc($data->{name})} = $data;
>   	    };
>   	    warn "$prefix: $@" if $@;
> +	} elsif ($section eq 'fail2ban') {
> +	    my ($opt, $value) = eval { parse_fail2ban_option($line) };
> +	    if (my $err = $@) {
> +		warn "$err";
> +		next;
> +	    }
> +	    $res->{fail2ban}->{$opt} = $value;
>   	} elsif ($section eq 'rules') {
>   	    my $rule;
>   	    eval { $rule = parse_fw_rule($prefix, $line, $cluster_conf, $res, $rule_env); };
> @@ -3251,6 +3331,21 @@ my $format_options = sub {
>       return $raw;
>   };
>   
> +my $format_fail2ban = sub {
> +    my ($fail2ban_options) = @_;
> +
> +    my $raw = '';
> +
> +    $raw .= "[FAIL2BAN]\n\n";
> +    foreach my $opt (keys %$fail2ban_options) {
> +	$raw .= "$opt: $fail2ban_options->{$opt}\n";
> +    }
> +    $raw .= "\n";
> +
> +    return $raw;
> +
> +};
> +
>   my $format_aliases = sub {
>       my ($aliases) = @_;
>   
> @@ -3620,7 +3715,7 @@ sub load_hostfw_conf {
>   
>       $filename = $hostfw_conf_filename if !defined($filename);
>   
> -    my $empty_conf = { rules => [], options => {}};
> +    my $empty_conf = { rules => [], options => {}, fail2ban => {}};
>       return generic_fw_config_parser($filename, $cluster_conf, $empty_conf, 'host');
>   }
>   
> @@ -3630,7 +3725,9 @@ sub save_hostfw_conf {
>       my $raw = '';
>   
>       my $options = $hostfw_conf->{options};
> +    my $fail2ban_options = $hostfw_conf->{fail2ban};
>       $raw .= &$format_options($options) if $options && scalar(keys %$options);
> +    $raw .= &$format_fail2ban($fail2ban_options) if $fail2ban_options && scalar(keys %$fail2ban_options);
>   
>       my $rules = $hostfw_conf->{rules};
>       if ($rules && scalar(@$rules)) {
> @@ -4590,6 +4687,8 @@ sub update {
>   	}
>   
>   	my $hostfw_conf = load_hostfw_conf($cluster_conf);
> +	my $fail2ban_opts = $hostfw_conf->{fail2ban};
> +	generate_fail2ban_config($fail2ban_opts) if scalar(keys %$fail2ban_opts);
>   
>   	my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = compile($cluster_conf, $hostfw_conf);
>   
>