From 334bdd16d6afa99fdfa62bb0ddbfb8aa4b346a11 Mon Sep 17 00:00:00 2001 From: Tom Eastep Date: Sun, 2 Dec 2012 10:40:14 -0800 Subject: [PATCH] Carefully suppress duplicate rules in all tables Signed-off-by: Tom Eastep --- Shorewall/Perl/Shorewall/Chains.pm | 136 +++++++----------------- Shorewall/manpages/shorewall.conf.xml | 22 ++-- Shorewall6/manpages/shorewall6.conf.xml | 18 ++-- 3 files changed, 64 insertions(+), 112 deletions(-) diff --git a/Shorewall/Perl/Shorewall/Chains.pm b/Shorewall/Perl/Shorewall/Chains.pm index d51d400a8..acbfeea71 100644 --- a/Shorewall/Perl/Shorewall/Chains.pm +++ b/Shorewall/Perl/Shorewall/Chains.pm @@ -3382,105 +3382,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 ) { + $docheck = 1; + # + # We must not suppress duplicate rules that match on things that can + # be altered by other rules in the chain + # + for ( qw( mark connmark dscp tos set ecn u32 ) ) { + $docheck = 0, last if exists $baseref->{$_}; + } + } else { + $docheck = 0; + } + + if ( $docheck ) { my $ports1; my @keys1 = sort( keys( %$baseref ) ); my $rulenum = @_; - my $duplicate = 0; + + { + 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; - - for my $key ( @keys1 ) { - next RULE unless $key eq $keys2[$keynum++]; - next RULE unless compare_values( $baseref->{$key}, $ruleref->{$key} ); - } - - $duplicate = 1; + 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; - - for my $key ( @keys1 ) { - last RULE unless $key eq $keys2[$keynum++]; - last 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--; + if ( $duplicate ) { + trace( $chainref, 'D', $lastrule, $baseref ) if $debug; + } else { + unshift @rules, $baseref; } + + $baseref = pop @_; + $lastrule--; } unshift @rules, $baseref if $baseref; @@ -3497,19 +3453,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..6ce4a3d48 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,10 @@ 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 rules in a chain that don't include mark, connmark, + dscp, tos, set, ecn or u32 matches. + Example 1: @@ -1821,7 +1825,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..06bb0b857 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,10 @@ 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 rules in a chain that don't include mark, connmark, + dscp, tos, set, ecn or u32 matches. + Example 1: @@ -1622,7 +1626,7 @@ net all DROP infothen the chain name is 'net2all' role="bold">" - +