From cd5e9be46721092e85025593e981a18bd7a362c9 Mon Sep 17 00:00:00 2001 From: Tom Eastep Date: Sun, 2 Dec 2012 12:20:24 -0800 Subject: [PATCH] Carefully suppress duplicate rules in all tables Signed-off-by: Tom Eastep --- Shorewall/Perl/Shorewall/Chains.pm | 139 ++++++++---------------- Shorewall/manpages/shorewall.conf.xml | 27 +++-- Shorewall6/manpages/shorewall6.conf.xml | 23 ++-- 3 files changed, 79 insertions(+), 110 deletions(-) diff --git a/Shorewall/Perl/Shorewall/Chains.pm b/Shorewall/Perl/Shorewall/Chains.pm index d51d400a8..719fce778 100644 --- a/Shorewall/Perl/Shorewall/Chains.pm +++ b/Shorewall/Perl/Shorewall/Chains.pm @@ -3370,6 +3370,13 @@ sub combine_dports { \@rules; } +my %bad_match = ( conntrack => 1, + dscp => 1, + ecn => 1, + mark => 1, + set => 1, + tos => 1, + u32 => 1 ); # # Delete duplicate rules from the passed chain. # @@ -3382,105 +3389,61 @@ sub delete_duplicates { my $lastrule = @_; my $baseref = pop; my $ruleref; - my $duplicate = 0; - while ( @_ && ! $duplicate ) { - { + while ( @_ ) { + my $docheck; + my $duplicate = 0; + + if ( $baseref->{mode} == CAT_MODE ) { my $ports1; - my @keys1 = sort( keys( %$baseref ) ); - my $rulenum = @_; - my $duplicate = 0; + my @keys1 = sort( keys( %$baseref ) ); + my $rulenum = @_; + my $adjacent = 1; + + { + RULE: - if ( $baseref->{mode} == CAT_MODE ) { - { - RULE: + while ( --$rulenum >= 0 ) { + $ruleref = $_[$rulenum]; - while ( --$rulenum >= 0 ) { - $ruleref = $_[$rulenum]; + last unless $ruleref->{mode} == CAT_MODE; - last unless $ruleref->{mode} == CAT_MODE; + my @keys2 = sort(keys( %$ruleref ) ); - my @keys2 = sort(keys( %$ruleref ) ); + next unless @keys1 == @keys2 ; - next unless @keys1 == @keys2 ; - - my $keynum = 0; + my $keynum = 0; + if ( $adjacent > 0 ) { for my $key ( @keys1 ) { next RULE unless $key eq $keys2[$keynum++]; next RULE unless compare_values( $baseref->{$key}, $ruleref->{$key} ); } - - $duplicate = 1; - } - } - } - - if ( $duplicate ) { - trace( $chainref, 'D', $lastrule, $baseref ) if $debug; - } else { - unshift @rules, $baseref; - } - - $baseref = pop @_; - $lastrule--; - } - } - - unshift @rules, $baseref if $baseref; - - \@rules; -} - -sub delete_adjacent_duplicates { - my @rules; - my $chainref = shift; - my $lastrule = @_; - my $baseref = pop; - my $ruleref; - my $duplicate = 0; - - while ( @_ && ! $duplicate ) { - { - my $ports1; - my @keys1 = sort( keys( %$baseref ) ); - my $rulenum = @_; - my $duplicate = 0; - - if ( $baseref->{mode} == CAT_MODE ) { - { - RULE: - - while ( --$rulenum >= 0 ) { - $ruleref = $_[$rulenum]; - - last unless $ruleref->{mode} == CAT_MODE; - - my @keys2 = sort(keys( %$ruleref ) ); - - last RULE unless @keys1 == @keys2 ; - - my $keynum = 0; - + } else { for my $key ( @keys1 ) { - last RULE unless $key eq $keys2[$keynum++]; - last RULE unless compare_values( $baseref->{$key}, $ruleref->{$key} ); + last RULE if $bad_match{$key}; + next RULE unless $key eq $keys2[$keynum++]; + next RULE unless compare_values( $baseref->{$key}, $ruleref->{$key} ); } - - $duplicate = 1; } + + $duplicate = 1; + $adjacent++; + + } continue { + $adjacent--; } } - - if ( $duplicate ) { - trace( $chainref, 'D', $lastrule, $baseref ) if $debug; - } else { - unshift @rules, $baseref; - } - - $baseref = pop @_; - $lastrule--; } + + if ( $duplicate ) { + trace( $chainref, 'D', $lastrule, $baseref ) if $debug; + } else { + unshift @rules, $baseref; + } + + $baseref = pop @_; + $lastrule--; } unshift @rules, $baseref if $baseref; @@ -3497,19 +3460,7 @@ sub optimize_level16( $$$ ) { progress_message "\n Table $table pass $passes, $chains referenced user chains, level 16..."; for my $chainref ( @chains ) { - if ( $table eq 'raw' ) { - # - # Helpers in rules have the potential for generating lots of duplicate iptables rules - # in the raw table. This step eliminates those duplicates - # - $chainref->{rules} = delete_duplicates( $chainref, @{$chainref->{rules}} ); - } else { - # - # Conservative version that only deletes adjacent duplicates - # - $chainref->{rules} = delete_adjacent_duplicates( $chainref, @{$chainref->{rules}} ); - } - + $chainref->{rules} = delete_duplicates( $chainref, @{$chainref->{rules}} ); } $passes++; diff --git a/Shorewall/manpages/shorewall.conf.xml b/Shorewall/manpages/shorewall.conf.xml index 9b9eb50b0..3315a3e35 100644 --- a/Shorewall/manpages/shorewall.conf.xml +++ b/Shorewall/manpages/shorewall.conf.xml @@ -96,7 +96,7 @@ role="bold">none} - + @@ -106,7 +106,7 @@ role="bold">none} - + @@ -116,7 +116,7 @@ role="bold">none} - + @@ -126,7 +126,7 @@ role="bold">none} - + @@ -523,7 +523,7 @@
- + If CONFIG_PATH is not given or if it is set to the empty value then the contents of /usr/share/shorewall/configpath are @@ -930,7 +930,7 @@ net all DROP infothen the chain name is 'net2all' - +
If this variable is not set or is given an empty value @@ -1140,7 +1140,7 @@ net all DROP infothen the chain name is 'net2all' - +
For example, using the default LOGFORMAT, the log prefix for @@ -1157,7 +1157,7 @@ net all DROP infothen the chain name is 'net2all' control your firewall after you enable this option. - + Do not use this option if the resulting log messages will @@ -1724,6 +1724,15 @@ net all DROP infothen the chain name is 'net2all' 'Others and'. Empty comments at the end of a group of combined comments are replaced by 'and others'. + Beginning in Shorewall 4.5.10, this option also suppresses + duplicate adjacent rules and duplicate non-adjacent rules that + don't include mark, connmark, dscp, ecn, set, tos + or u32 matches. + Example 1: @@ -1821,7 +1830,7 @@ net all DROP infothen the chain name is 'net2all' role="bold">" - + diff --git a/Shorewall6/manpages/shorewall6.conf.xml b/Shorewall6/manpages/shorewall6.conf.xml index 373d628fb..6976ec74a 100644 --- a/Shorewall6/manpages/shorewall6.conf.xml +++ b/Shorewall6/manpages/shorewall6.conf.xml @@ -82,7 +82,7 @@ role="bold">none} - + @@ -92,7 +92,7 @@ role="bold">none} - + @@ -102,7 +102,7 @@ role="bold">none} - + @@ -112,7 +112,7 @@ role="bold">none} - + @@ -1006,7 +1006,7 @@ net all DROP infothen the chain name is 'net2all' - +
For example, using the default LOGFORMAT, the log prefix for @@ -1023,7 +1023,7 @@ net all DROP infothen the chain name is 'net2all' control your firewall after you enable this option. - + Do not use this option if the resulting log messages will @@ -1525,6 +1525,15 @@ net all DROP infothen the chain name is 'net2all' 'Others and'. Empty comments at the end of a group of combined comments are replaced by 'and others'. + Beginning in Shorewall 4.5.10, this option also suppresses + duplicate adjacent rules and duplicate non-adjacent rules that + don't include mark, connmark, dscp, ecn, set, tos + or u32 matches. + Example 1: @@ -1622,7 +1631,7 @@ net all DROP infothen the chain name is 'net2all' role="bold">" - +