[pve-devel] [RFC firewall] fix: #2123 Logging of user defined firewall rules
Thomas Lamprecht
t.lamprecht at proxmox.com
Thu Mar 14 13:31:04 CET 2019
On 3/14/19 1:06 PM, Christian Ebner wrote:
> This allows a user to log traffic filtered by a self defined firewall rule.
> Therefore the API is extended to include a 'log' option allow to specify the
> log level for each rule individually.
>
> The 'log' option can also be specified in the fw config. In order to reduce the
> log amount, logging is limited to 1 entry per second.
quick glance over the code looks not bad, but I'm not sure of the hard-coded limit,
maybe not even about the limit itself...
Also the docs state:
> This module matches at a limited rate using a token bucket filter. A rule using
> this extension will match until this limit is reached. It can be used in
> combination with the LOG target to give limited logging, for example.
> --limit rate[/second|/minute|/hour|/day]
> Maximum average matching rate: specified as a number, with an optional `/second',
> `/minute', `/hour', or `/day' suffix; the default is 3/hour.
> --limit-burst
> ...
-- http://ipset.netfilter.org/iptables-extensions.man.html
So how does this logs in reality, does it logs a full second, or does it always
logs for the burst (default 5) and then only if the burst refilled?
Does one sees hints about "not logged packages", e.g., like systemd does, e.g.,
IIRC something like "suppressed XYZ count of log messages from service foo" is
done there.
Just not that we come again in a situation where the user thinks _all_ is logged,
but infact all is then _not_ logged.
>
> For now the rule has to be created or edited via the pvesh API call or via the
> firewall config in order to set the log level.
>
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
>
> This is a tentative patch in order to allow fine grained logging of packets
> dropped by user defined rules.
> Feedback is very much appreciated.
>
> src/PVE/API2/Firewall/Rules.pm | 3 +++
> src/PVE/Firewall.pm | 59 +++++++++++++++++++++++++-----------------
> 2 files changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
> index 1670986..f0bc562 100644
> --- a/src/PVE/API2/Firewall/Rules.pm
> +++ b/src/PVE/API2/Firewall/Rules.pm
> @@ -141,6 +141,9 @@ sub register_get_rule {
> type => 'integer',
> optional => 1,
> },
> + log => PVE::Firewall::get_standard_option('pve-fw-loglevel', {
> + description => 'Log level for firewall rule',
> + }),
> iface => {
> type => 'string',
> optional => 1,
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 6ac3038..8bb7bb9 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1363,6 +1363,9 @@ my $rule_properties = {
> minimum => 0,
> optional => 1,
> },
> + log => get_standard_option('pve-fw-loglevel', {
> + description => "Log level for firewall rule.",
> + }),
> sport => {
> description => "Restrict TCP/UDP source port. $port_descr",
> type => 'string', format => 'pve-fw-sport-spec',
> @@ -2008,8 +2011,10 @@ sub ipt_rule_to_cmds {
> }
>
> my @iptcmds;
> - if (defined $rule->{log} && $rule->{log}) {
> - my $logaction = get_log_rule_base($chain, $vmid, $rule->{logmsg}, $rule->{log});
> + if ($rule->{log} && $rule->{log} ne 'nolog') {
> + my $log = $rule->{log};
> + my $loglevel = $log_level_hash->{$log};
> + my $logaction = get_log_rule_base($chain, $vmid, $rule->{logmsg}, $loglevel);
> push @iptcmds, "-A $chain $matchstr $logaction";
> }
> push @iptcmds, "-A $chain $matchstr $targetstr";
> @@ -2017,7 +2022,7 @@ sub ipt_rule_to_cmds {
> }
>
> sub ruleset_generate_rule {
> - my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf) = @_;
> + my ($ruleset, $chain, $ipversion, $rule, $cluster_conf, $fw_conf, $vmid) = @_;
>
> my $rules;
>
> @@ -2030,7 +2035,7 @@ sub ruleset_generate_rule {
> # update all or nothing
> my @ipt_rule_cmds;
> foreach my $r (@$rules) {
> - push @ipt_rule_cmds, ipt_rule_to_cmds($r, $chain, $ipversion, $cluster_conf, $fw_conf);
> + push @ipt_rule_cmds, ipt_rule_to_cmds($r, $chain, $ipversion, $cluster_conf, $fw_conf, $vmid);
> }
> foreach my $c (@ipt_rule_cmds) {
> ruleset_add_ipt_cmd($ruleset, $chain, $c);
> @@ -2064,17 +2069,18 @@ sub ruleset_add_ipt_cmd {
> }
>
> sub ruleset_addrule {
> - my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
> -
> - die "no such chain '$chain'\n" if !$ruleset->{$chain};
> + my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
>
> - if (defined($log) && $log) {
> - my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $log);
> + die "no such chain '$chain'\n" if !$ruleset->{$chain};
> +
> + if ($log) {
> + my $loglevel = $log_level_hash->{$log};
> + my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $loglevel);
> push @{$ruleset->{$chain}}, "-A $chain $match $logaction";
> - }
> - # for stable ebtables digests avoid double-spaces to match ebtables-save output
> - $match .= ' ' if length($match);
> - push @{$ruleset->{$chain}}, "-A $chain ${match}$action";
> + }
> + # for stable ebtables digests avoid double-spaces to match ebtables-save output
> + $match .= ' ' if length($match);
> + push @{$ruleset->{$chain}}, "-A $chain ${match}$action";
> }
>
> sub ruleset_insertrule {
> @@ -2094,7 +2100,7 @@ sub get_log_rule_base {
> # Note: we use special format for prefix to pass further
> # info to log daemon (VMID, LOGLEVEL and CHAIN)
>
> - return "-j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
> + return "-m limit --limit 1/sec -j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
> }
>
> sub ruleset_add_chain_policy {
> @@ -2234,7 +2240,7 @@ sub ruleset_add_group_rule {
> }
>
> sub ruleset_generate_vm_rules {
> - my ($ruleset, $rules, $cluster_conf, $vmfw_conf, $chain, $netid, $direction, $options, $ipversion) = @_;
> + my ($ruleset, $rules, $cluster_conf, $vmfw_conf, $chain, $netid, $direction, $options, $ipversion, $vmid) = @_;
>
> my $lc_direction = lc($direction);
>
> @@ -2251,12 +2257,13 @@ sub ruleset_generate_vm_rules {
> } else {
> next if $rule->{type} ne $lc_direction;
> eval {
> + $rule->{logmsg} = "$rule->{action}: ";
> if ($direction eq 'OUT') {
> rule_substitude_action($rule, { ACCEPT => "PVEFW-SET-ACCEPT-MARK", REJECT => "PVEFW-reject" });
> - ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $vmfw_conf);
> + ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $vmfw_conf, $vmid);
> } else {
> rule_substitude_action($rule, { ACCEPT => $in_accept , REJECT => "PVEFW-reject" });
> - ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $vmfw_conf);
> + ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $vmfw_conf, $vmid);
> }
> };
> warn $@ if $@;
> @@ -2317,7 +2324,7 @@ sub generate_tap_rules_direction {
> ruleset_create_vm_chain($ruleset, $tapchain, $ipversion, $options, $macaddr, $ipfilter_ipset, $direction);
>
> if ($options->{enable}) {
> - ruleset_generate_vm_rules($ruleset, $rules, $cluster_conf, $vmfw_conf, $tapchain, $netid, $direction, $options, $ipversion);
> + ruleset_generate_vm_rules($ruleset, $rules, $cluster_conf, $vmfw_conf, $tapchain, $netid, $direction, $options, $ipversion, $vmid);
>
> ruleset_generate_vm_ipsrules($ruleset, $options, $direction, $iface);
>
> @@ -2385,7 +2392,7 @@ sub enable_host_firewall {
> ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule, 'IN', $accept_action, $ipversion);
> } elsif ($rule->{type} eq 'in') {
> rule_substitude_action($rule, { ACCEPT => $accept_action, REJECT => "PVEFW-reject" });
> - ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $hostfw_conf);
> + ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $hostfw_conf, 0);
> }
> };
> warn $@ if $@;
> @@ -2437,11 +2444,12 @@ sub enable_host_firewall {
>
> $rule->{iface_out} = $rule->{iface} if $rule->{iface};
> eval {
> + $rule->{logmsg} = "$rule->{action}: ";
> if ($rule->{type} eq 'group') {
> ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule, 'OUT', $accept_action, $ipversion);
> } elsif ($rule->{type} eq 'out') {
> rule_substitude_action($rule, { ACCEPT => $accept_action, REJECT => "PVEFW-reject" });
> - ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $hostfw_conf);
> + ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, $cluster_conf, $hostfw_conf, 0);
> }
> };
> warn $@ if $@;
> @@ -2580,6 +2588,10 @@ sub parse_fw_rule {
> $rule->{dest} = $1;
> next;
> }
> + if ($line =~ s/^-log (emerg|alert|crit|err|warning|notice|info|debug|nolog)\s*//) {
> + $rule->{log} = $1;
> + next;
> + }
>
> last;
> }
> @@ -3032,6 +3044,7 @@ my $format_rules = sub {
> $raw .= " -p $rule->{proto}" if $rule->{proto};
> $raw .= " -dport $rule->{dport}" if $rule->{dport};
> $raw .= " -sport $rule->{sport}" if $rule->{sport};
> + $raw .= " -log $rule->{log}" if $rule->{log};
> }
>
> $raw .= " # " . encode('utf8', $rule->{comment})
> @@ -3190,9 +3203,7 @@ sub get_option_log_level {
>
> return undef if $v eq '' || $v eq 'nolog';
>
> - $v = $log_level_hash->{$v} if defined($log_level_hash->{$v});
> -
> - return $v if ($v >= 0) && ($v <= 7);
> + return $v if defined($log_level_hash->{$v});
>
> warn "unknown log level ($k = '$v')\n";
>
> @@ -3225,7 +3236,7 @@ sub generate_std_chains {
> ruleset_create_chain($ruleset, $chain);
> foreach my $rule (@{$std_chains->{$chain}}) {
> if (ref($rule)) {
> - ruleset_generate_rule($ruleset, $chain, $ipversion, $rule);
> + ruleset_generate_rule($ruleset, $chain, $ipversion, $rule, 0);
> } else {
> die "rule $rule as string - should not happen";
> }
>
More information about the pve-devel
mailing list