[pve-devel] [PATCH v2 firewall 3/4] integrate logging into ruleset_addrule

[Wolfgang Bumiller]

On Wed, Sep 27, 2017 at 12:02:32AM +0200, Tom Weber wrote:
> ---
>  src/PVE/Firewall.pm | 33 ++++++++++-----------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index f1aecef..f8a9300 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -2002,10 +2002,14 @@ sub ruleset_addrule_old {
>  }
>  
>  sub ruleset_addrule {
> -   my ($ruleset, $chain, $match, $action, $log) = @_;
> +   my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_;
>  
>     die "no such chain '$chain'\n" if !$ruleset->{$chain};
>  
> +   if (defined($log) && $log) {

Did you mean to also check $logmsg? (As a sanity check this would probably
be useful). Because the 'defined($log)' part is redundant since `undef`
is false anyway.

> +	my $logaction = get_log_rule_base($chain, $vmid, $logmsg, $log);
> +	push @{$ruleset->{$chain}}, "-A $chain $match $logaction";
> +   }
>     push @{$ruleset->{$chain}}, "-A $chain $match $action";
>  }
>  
> @@ -2020,27 +2024,15 @@ sub ruleset_insertrule {
>  sub get_log_rule_base {
>      my ($chain, $vmid, $msg, $loglevel) = @_;
>  
> -    die "internal error - no log level" if !defined($loglevel);
> -
>      $vmid = 0 if !defined($vmid);
> +    $msg = "" if !defined($msg);
>  
>      # Note: we use special format for prefix to pass further
> -    # info to log daemon (VMID, LOGVELEL and CHAIN)
> +    # info to log daemon (VMID, LOGLEVEL and CHAIN)
>  
>      return "-j NFLOG --nflog-prefix \":$vmid:$loglevel:$chain: $msg\"";
>  }
>  
> -sub ruleset_addlog {
> -    my ($ruleset, $chain, $vmid, $msg, $loglevel, $match) = @_;
> -
> -    return if !defined($loglevel);
> -
> -    my $logaction = get_log_rule_base($chain, $vmid, $msg, $loglevel);
> -
> -    $match = "" if !defined $match;
> -    ruleset_addrule($ruleset, $chain, $match, $logaction);
> -}
> -
>  sub ruleset_add_chain_policy {
>      my ($ruleset, $chain, $ipversion, $vmid, $policy, $loglevel, $accept_action) = @_;
>  
> @@ -2053,15 +2045,11 @@ sub ruleset_add_chain_policy {
>  
>  	ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Drop");
>  
> -	ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
> -
> -	ruleset_addrule($ruleset, $chain, "", "-j DROP");
> +	ruleset_addrule($ruleset, $chain, "", "-j DROP", $loglevel, "policy $policy: ", $vmid);
>      } elsif ($policy eq 'REJECT') {
>  	ruleset_addrule($ruleset, $chain, "", "-j PVEFW-Reject");
>  
> -	ruleset_addlog($ruleset, $chain, $vmid, "policy $policy: ", $loglevel);
> -
> -	ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject");
> +	ruleset_addrule($ruleset, $chain, "", "-g PVEFW-reject", $loglevel, "policy: $policy", $vmid);

typo: "policy: $policy" vs "policy $policy: "

>      } else {
>  	# should not happen
>  	die "internal error: unknown policy '$policy'";
> @@ -2093,8 +2081,7 @@ sub ruleset_chain_add_input_filters {
>      if ($cluster_conf->{ipset}->{blacklist}){
>  	if (!ruleset_chain_exist($ruleset, "PVEFW-blacklist")) {
>  	    ruleset_create_chain($ruleset, "PVEFW-blacklist");
> -	    ruleset_addlog($ruleset, "PVEFW-blacklist", 0, "DROP: ", $loglevel) if $loglevel;
> -	    ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP");
> +	    ruleset_addrule($ruleset, "PVEFW-blacklist", "", "-j DROP", $loglevel, "DROP: ");
>  	}
>  	my $ipset_chain = compute_ipset_chain_name(0, 'blacklist', $ipversion);
>  	ruleset_addrule($ruleset, $chain, "-m set --match-set ${ipset_chain} src", "-j PVEFW-blacklist");
> -- 
> 2.7.4