[pve-devel] [RFC firewall] fix: #2123 Logging of user defined firewall rules
Christian Ebner
c.ebner at proxmox.com
Thu Mar 14 14:07:44 CET 2019
As I understand it this matches the 5 packets from the burst and then only if the burst refilled.
See https://thelowedown.wordpress.com/2008/07/03/iptables-how-to-use-the-limits-module/
We could let the user decide the rate and burst limit, but that would probably be a bit overkill and add unwanted complexity.
Without rate limit there is the danger of spaming the logs...
I doubt that you will get any hints about unlogged packages as they simply won't match the rule for logging, although I still need to test this.
> On March 14, 2019 at 1:31 PM Thomas Lamprecht <t.lamprecht at proxmox.com> wrote:
>
>
> 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