On Wed, Sep 27, 2017 at 12:02:33AM +0200, Tom Weber wrote: > --- > src/PVE/Firewall.pm | 220 > ++++++++++++++++++++++++++++------------------------ > 1 file changed, 117 insertions(+), 103 deletions(-) > > diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm > index f8a9300..179617a 100644 > --- a/src/PVE/Firewall.pm > +++ b/src/PVE/Firewall.pm > @@ -142,6 +142,20 @@ my $log_level_hash = { > emerg => 0, > }; > > +# %rule > +#
+1 for the documentation, but what happened to the right sides of most of these arrows? ;-) (yeah some of these options are being abused a bit...) > +# name => optional > +# action => > +# proto => > +# sport => > +# dport => > +# log => optional, loglevel Why not call it loglevel then? Also, with this being new it should be mentioned in the commit message. > +# logmsg => optional, logmsg - overwrites default And this. > +# iface_in > +# iface_out > +# match => optional, overwrites generation of match > +# target => optional, overwrites action > + > # we need to overwrite some macros for ipv6 > my $pve_ipv6fw_macros = { > 'Ping' => [ > @@ -536,7 +550,7 @@ my $FWACCEPTMARK_OFF = "0x00000000/0x80000000"; > @@ -1948,19 +1970,24 @@ sub ruleset_generate_rule { > > # update all or nothing > > + # fixme: lots of temporary ugliness +1 for the 'temporary' in there ;-) > my @mstrs = (); > my @astrs = (); > + my @logging = (); > + my @logmsg = (); > foreach my $tmp (@$rules) { > my $m = ruleset_generate_match($ruleset, $chain, $ipversion, $tmp, > $actions, $goto, $cluster_conf, $fw_conf); > my $a = ruleset_generate_action($ruleset, $chain, $ipversion, $tmp, > $actions, $goto, $cluster_conf, $fw_conf); > if (defined $m or defined $a) { > push @mstrs, defined($m) ? $m : ""; > push @astrs, defined($a) ? $a : ""; > + push @logging, $tmp->{log}; > + push @logmsg, $tmp->{logmsg}; > } > } > > for my $i (0 .. $#mstrs) { > - ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i]); > + ruleset_addrule($ruleset, $chain, $mstrs[$i], $astrs[$i], $logging[$i], > $logmsg[$i]); > } > } > > @@ -1993,14 +2020,6 @@ sub ruleset_chain_exist { > return $ruleset->{$chain} ? 1 : undef; > } > > -sub ruleset_addrule_old { > - my ($ruleset, $chain, $rule) = @_; > - > - die "no such chain '$chain'\n" if !$ruleset->{$chain}; > - > - push @{$ruleset->{$chain}}, "-A $chain $rule"; > -} > - > sub ruleset_addrule { > my ($ruleset, $chain, $match, $action, $log, $logmsg, $vmid) = @_; > > @@ -3127,26 +3146,21 @@ sub generate_std_chains { > my $std_chains = $pve_std_chains->{$ipversion} || die "internal error"; > > my $loglevel = get_option_log_level($options, 'smurf_log_level'); > - > - my $chain; > - > - if ($ipversion == 4) { > - # same as shorewall smurflog. > - $chain = 'PVEFW-smurflog'; > - $std_chains->{$chain} = []; > - > - push @{$std_chains->{$chain}}, get_log_rule_base($chain, 0, "DROP: ", > $loglevel) if $loglevel; > - push @{$std_chains->{$chain}}, "-j DROP"; > + my $chain = 'PVEFW-smurflog'; > + if ( $loglevel && $std_chains->{$chain} ) { This shouldn't check for $loglevel as it can also have changed from non-zero to zero in which case the change would happen. Iow. you cannot turn off the smurf logs anymore. > + foreach my $r (@{$std_chains->{$chain}}) { > + $r->{log} = $loglevel; Wouldn't it be better to just give ruleset_generate_rule an optional default loglevel parameter for when rules don't define their own {log}? > + } > } > > # same as shorewall logflags action. > $loglevel = get_option_log_level($options, 'tcp_flags_log_level'); > $chain = 'PVEFW-logflags'; > - $std_chains->{$chain} = []; > - > - # fixme: is this correctly logged by pvewf-logger? (ther is no > --log-ip-options for NFLOG) > - push @{$std_chains->{$chain}}, get_log_rule_base($chain, 0, "DROP: ", > $loglevel) if $loglevel; > - push @{$std_chains->{$chain}}, "-j DROP"; > + if ( $loglevel && $std_chains->{$chain} ) { > + foreach my $r (@{$std_chains->{$chain}}) { > + $r->{log} = $loglevel; > + } > + } > > foreach my $chain (keys %$std_chains) { > ruleset_create_chain($ruleset, $chain); > @@ -3154,7 +3168,7 @@ sub generate_std_chains { > if (ref($rule)) { > ruleset_generate_rule($ruleset, $chain, $ipversion, $rule); > } else { > - ruleset_addrule_old($ruleset, $chain, $rule); > + die "rule $rule as string - should not happen"; > } > } > } > -- > 2.7.4 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel