From 5b2f960db326fd1518e252d383385f575ead2cde Mon Sep 17 00:00:00 2001
From: Tom Eastep <teastep@shorewall.net>
Date: Fri, 30 Dec 2011 07:22:14 -0800
Subject: [PATCH] Disallow :P in CLASSIFY rules and complain if :F is used when
 the SOURCE or DEST is $FW.

Signed-off-by: Tom Eastep <teastep@shorewall.net>
---
 Shorewall/Perl/Shorewall/Tc.pm   | 30 +++++++++++++++++++-----------
 manpages/shorewall-tcrules.xml   | 12 ++----------
 manpages6/shorewall6-tcrules.xml | 10 +---------
 3 files changed, 22 insertions(+), 30 deletions(-)

diff --git a/Shorewall/Perl/Shorewall/Tc.pm b/Shorewall/Perl/Shorewall/Tc.pm
index f36e703eb..44672be67 100644
--- a/Shorewall/Perl/Shorewall/Tc.pm
+++ b/Shorewall/Perl/Shorewall/Tc.pm
@@ -104,8 +104,7 @@ my  %flow_keys = ( 'src'            => 1,
 		   'sk-gid'         => 1,
 		   'vlan-tag'       => 1 );
 
-my %designator = ( P => 'tcpre' ,
-		   F => 'tcfor' ,
+my %designator = ( F => 'tcfor' ,
 		   T => 'tcpost' );
 
 my  %tosoptions = ( 'tos-minimize-delay'       => '0x10/0x10' ,
@@ -211,7 +210,8 @@ sub process_tc_rule( ) {
 
     fatal_error "Invalid MARK ($originalmark)" unless supplied $mark;
 
-    my $chain  = $globals{MARKING_CHAIN};
+    my $chain    = $globals{MARKING_CHAIN};
+    my $classid  = 0;
 
     if ( $remainder ) { 
 	if ( $originalmark =~ /^\w+\(?.*\)$/ ) {
@@ -221,33 +221,40 @@ sub process_tc_rule( ) {
 		unless ( $mark =~ /^([0-9a-fA-F]+)$/ &&
 			 $designator =~ /^([0-9a-fA-F]+)$/ && 
 			 ( $chain = $designator{$remainder} ) );
-	    $mark  = join( ':', $mark, $designator );
+	    $mark    = join( ':', $mark, $designator );
+	    $classid = 1;
 	}
     }
 
     my $target = 'MARK --set-mark';
     my $tcsref;
     my $connmark = 0;
-    my $classid  = 0;
     my $device   = '';
     my $fw       = firewall_zone;
     my $list;
 
     if ( $source ) {
 	if ( $source eq $fw ) {
-	    $chain = 'tcout';
+	    if ( $classid ) {
+		fatal_error ":F is not allowed when the SOURCE is the firewall" if $chain eq 'tcfor';
+	    } else {
+		$chain = 'tcout';
+	    }
 	    $source = '';
-	} else {
-	    $chain = 'tcout' if $source =~ s/^($fw)://;
+	} elsif ( $source =~ s/^($fw):// ) {
+	    fatal_error ":F is not allowed when the SOURCE is the firewall" if $chain eq 'tcfor';
+	    $chain = 'tcout';
 	}
     }
 
     if ( $dest ) {
 	if ( $dest eq $fw ) {
+	    fatal_error 'A CLASSIFY rule may not have $FW as the DEST' if $classid;
 	    $chain = 'tcin';
 	    $dest  = '';
-	} else {
-	    $chain = 'tcin' if $dest =~ s/^($fw)://;
+	} elsif ( $dest =~ s/^($fw):// ) {
+	    fatal_error 'A CLASSIFY rule may not have $FW as the DEST' if $classid;
+	    $chain = 'tcin';
 	}
     }
 
@@ -268,8 +275,9 @@ sub process_tc_rule( ) {
 	    require_capability ('CONNMARK' , "CONNMARK Rules", '' ) if $connmark;
 
 	} else {
-	    unless ( $remainder ) {
+	    unless ( $classid ) {
 		fatal_error "Invalid MARK ($originalmark)" unless $mark =~ /^([0-9a-fA-F]+)$/ and $designator =~ /^([0-9a-fA-F]+)$/;
+		fatal_error 'A CLASSIFY rule may not have $FW as the DEST' if $chain eq 'tcin';
 		$chain = 'tcpost';
 		$mark  = $originalmark;
 	    }
diff --git a/manpages/shorewall-tcrules.xml b/manpages/shorewall-tcrules.xml
index 75c6dc590..cf433474d 100644
--- a/manpages/shorewall-tcrules.xml
+++ b/manpages/shorewall-tcrules.xml
@@ -205,7 +205,7 @@
 
               <para>Beginning with Shorewall 4.4.27, the classid may be
               optionally followed by ':' and a capital letter designating the
-              chain where classification is to occur. </para>
+              chain where classification is to occur.</para>
 
               <variablelist>
                 <varlistentry>
@@ -216,19 +216,11 @@
                   </listitem>
                 </varlistentry>
 
-                <varlistentry>
-                  <term>P</term>
-
-                  <listitem>
-                    <para>PREROUTING chain.</para>
-                  </listitem>
-                </varlistentry>
-
                 <varlistentry>
                   <term>T</term>
 
                   <listitem>
-                    <para>POSTROUTING chain.</para>
+                    <para>POSTROUTING chain (default).</para>
                   </listitem>
                 </varlistentry>
               </variablelist>
diff --git a/manpages6/shorewall6-tcrules.xml b/manpages6/shorewall6-tcrules.xml
index fa3a1f932..125ed8fbd 100644
--- a/manpages6/shorewall6-tcrules.xml
+++ b/manpages6/shorewall6-tcrules.xml
@@ -216,19 +216,11 @@
                   </listitem>
                 </varlistentry>
 
-                <varlistentry>
-                  <term>P</term>
-
-                  <listitem>
-                    <para>PREROUTING chain.</para>
-                  </listitem>
-                </varlistentry>
-
                 <varlistentry>
                   <term>T</term>
 
                   <listitem>
-                    <para>POSTROUTING chain.</para>
+                    <para>POSTROUTING chain (default).</para>
                   </listitem>
                 </varlistentry>
               </variablelist>