firewall: Cleanup rules reloading.

This has been messed up a lot because there were multiple files which indicated that a reload is needed; shell commands were used to create and remove the indicator file; some functions were duplicated.
2026-04-13 04:22:58 +02:00 · 2013-10-22 18:53:48 +02:00
parent 0eadfdad4a
commit 0e43079789
5 changed files with 60 additions and 96 deletions
--- a/config/cfgroot/general-functions.pl
+++ b/config/cfgroot/general-functions.pl
@@ -1137,4 +1137,23 @@ sub write_file_utf8 ($) {
 	return; 
 }

+my $FIREWALL_RELOAD_INDICATOR = "${General::swroot}/forward/reread";
+
+sub firewall_config_changed() {
+	open FILE, ">$FIREWALL_RELOAD_INDICATOR" or die "Could not open $FIREWALL_RELOAD_INDICATOR";
+	close FILE;
+}
+
+sub firewall_needs_reload() {
+	if (-e "$FIREWALL_RELOAD_INDICATOR") {
+		return 1;
+	}
+
+	return 0;
+}
+
+sub firewall_reload() {
+	system("/usr/local/bin/forwardfwctrl");
+}
+
 1;
--- a/html/cgi-bin/forwardfw.cgi
+++ b/html/cgi-bin/forwardfw.cgi
@@ -385,7 +385,7 @@ if ($fwdfwsettings{'ACTION'} eq 'saverule')
 		&newrule;
 	}else{
 		if($fwdfwsettings{'nosave2'} ne 'on'){
-			&rules;
+			&General::firewall_config_changed();
 		}
 		&base;
 	}
@@ -404,7 +404,7 @@ if ($fwdfwsettings{'ACTION'} eq $Lang::tr{'fwdfw toggle'})
 		}
 	}
 	&General::writehasharray($fwdfwsettings{'config'}, \%togglehash);
-	&rules;
+	&General::firewall_config_changed();
 	&base;
 }
 if ($fwdfwsettings{'ACTION'} eq $Lang::tr{'fwdfw togglelog'})
@@ -417,12 +417,12 @@ if ($fwdfwsettings{'ACTION'} eq $Lang::tr{'fwdfw togglelog'})
 		}
 	}
 	&General::writehasharray($fwdfwsettings{'config'}, \%togglehash);
-	&rules;
+	&General::firewall_config_changed();
 	&base;
 }
 if ($fwdfwsettings{'ACTION'} eq $Lang::tr{'fwdfw reread'})
 {
-	&reread_rules;
+	&General::firewall_reload();
 	&base;
 }
 if ($fwdfwsettings{'ACTION'} eq 'editrule')
@@ -469,7 +469,7 @@ sub addrule
 					<td align="right">
 END

-	if (-f "${General::swroot}/forward/reread") {
+	if (&General::firewall_needs_reload()) {
 		print <<END;
 			<input type='submit' name='ACTION' value='$Lang::tr{'fwdfw reread'}' style='font-weight: bold; color: green;'>
 END
@@ -1012,7 +1012,7 @@ sub deleterule
 	delete $delhash{$last_key};

 	&General::writehasharray($fwdfwsettings{'config'}, \%delhash);
-	&rules;
+	&General::firewall_config_changed();

 	if($fwdfwsettings{'nobase'} ne 'on'){
 		&base;
@@ -1028,7 +1028,7 @@ sub disable_rule
 		}
 	}
 	&General::writehasharray("$configfwdfw", \%configfwdfw);
-	&rules;
+	&General::firewall_config_changed();
 }
 sub dec_counter
 {
@@ -2102,7 +2102,7 @@ sub pos_up
 		}
 	}
 	&General::writehasharray($fwdfwsettings{'config'}, \%uphash);
-	&rules;
+	&General::firewall_config_changed();
 }
 sub pos_down
 {
@@ -2129,22 +2129,7 @@ sub pos_down
 		}
 	}
 	&General::writehasharray($fwdfwsettings{'config'}, \%downhash);
-	&rules;
-}
-sub rules
-{
-	if (!-f "${General::swroot}/forward/reread"){
-		system("touch ${General::swroot}/forward/reread");
-		system("touch ${General::swroot}/fwhosts/reread");
-	}
-}
-sub reread_rules
-{
-	system("/usr/local/bin/forwardfwctrl");
-	if ( -f "${General::swroot}/forward/reread"){
-		system("rm ${General::swroot}/forward/reread");
-		system("rm ${General::swroot}/fwhosts/reread");
-	}
+	&General::firewall_config_changed();
 }
 sub saverule
 {
@@ -2286,7 +2271,7 @@ sub saverule
 				$fwdfwsettings{'oldrulenumber'}--;
 			}
 			&General::writehasharray("$config", $hash);
-			&rules;
+			&General::firewall_config_changed();
 		}elsif($fwdfwsettings{'rulepos'} > $fwdfwsettings{'oldrulenumber'}){
 			my %tmp=();
 			my $val=$fwdfwsettings{'rulepos'}-$fwdfwsettings{'oldrulenumber'};
@@ -2313,7 +2298,7 @@ sub saverule
 				$fwdfwsettings{'oldrulenumber'}++;
 			}
 			&General::writehasharray("$config", $hash);
-			&rules;
+			&General::firewall_config_changed();
 		}
 	}
 }
--- a/html/cgi-bin/fwhosts.cgi
+++ b/html/cgi-bin/fwhosts.cgi
@@ -114,11 +114,6 @@ print<<END;
 END

 ## ACTION ####
-if ($fwhostsettings{'ACTION'} eq $Lang::tr{'fwdfw reread'})
-{
-	&reread_rules;
-	&showmenu;
-}
 # Update
 if ($fwhostsettings{'ACTION'} eq 'updatenet' )
 {
@@ -254,7 +249,7 @@ if ($fwhostsettings{'ACTION'} eq 'updateservice')
 	}
 	$fwhostsettings{'updatesrv'} = '';
 	if($needrules eq 'on'){
-		&rules;
+		&General::firewall_config_changed();
 	}
 	&addservice;
 }
@@ -416,7 +411,7 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' )
 			$fwhostsettings{'NETREMARK'}='';
 			#check if an edited net affected groups and need to reload rules
 			if ($needrules eq 'on'){
-				&rules;
+				&General::firewall_config_changed();
 			}
 			&addnet;
 			&viewtablenet;
@@ -552,7 +547,7 @@ if ($fwhostsettings{'ACTION'} eq 'savehost')
 			 $fwhostsettings{'HOSTREMARK'}='';
 			#check if we need to update rules while host was edited
 			if($needrules eq 'on'){
-				&rules;
+				&General::firewall_config_changed();
 			}
 			&addhost;
 			&viewtablehost;
@@ -727,7 +722,7 @@ if ($fwhostsettings{'ACTION'} eq 'savegrp')
 		#check if ruleupdate is needed
 		if($count > 0 )
 		{
-			&rules;
+			&General::firewall_config_changed();
 		}
 		&addgrp;
 		&viewtablegrp;
@@ -829,7 +824,7 @@ if ($fwhostsettings{'ACTION'} eq 'saveservicegrp')
 		$fwhostsettings{'updatesrvgrp'}='on';
 	}
 	if ($count gt 0){
-		&rules;
+		&General::firewall_config_changed();
 	}
 	&addservicegrp;
 	&viewtableservicegrp;
@@ -944,7 +939,9 @@ if ($fwhostsettings{'ACTION'} eq 'deletegrphost')
 		}
 	}
 	&General::writehasharray("$configgrp", \%customgrp);
-	if ($fwhostsettings{'grpcnt'} > 0){&rules;}
+	if ($fwhostsettings{'grpcnt'} > 0){
+		&General::firewall_config_changed();
+	}
 	if ($fwhostsettings{'update'} eq 'on'){
 		$fwhostsettings{'remark'}= $grpremark;
 		$fwhostsettings{'grp_name'}=$grpname;
@@ -1023,7 +1020,7 @@ if ($fwhostsettings{'ACTION'} eq 'delgrpservice')
 		}
 	}
 	&General::writehasharray("$configsrvgrp", \%customservicegrp);
-	&rules;
+	&General::firewall_config_changed();
 	if ($fwhostsettings{'updatesrvgrp'} eq 'on'){
 		$fwhostsettings{'SRVGRP_NAME'}=$grpname;
 		$fwhostsettings{'SRVGRP_REMARK'}=$grpremark;
@@ -1112,11 +1109,7 @@ if($fwhostsettings{'ACTION'} eq '')
 	&showmenu;
 }
 ###  FUNCTIONS  ###
-sub showmenu
-{
-	if (-f "${General::swroot}/forward/reread"){
-		print "<table border='1' rules='groups' bgcolor='lightgreen' width='100%'><form method='post'><td><div style='font-size:11pt; font-weight: bold;vertical-align: middle; '><input type='submit' name='ACTION' value='$Lang::tr{'fwdfw reread'}' style='font-face: Comic Sans MS; color: green; font-weight: bold; font-size: 14pt;'>&nbsp &nbsp $Lang::tr{'fwhost reread'}</td></tr></table></form><br>";
-	}
+sub showmenu {
 	&Header::openbox('100%', 'left',$Lang::tr{'fwhost menu'});
 	print "$Lang::tr{'fwhost welcome'}";
 	print<<END;
@@ -2080,24 +2073,8 @@ sub getipforgroup
 		}
 	}
 }
-sub rules
-{
-	if (!-f "${General::swroot}/fwhosts/reread"){
-		system("touch ${General::swroot}/fwhosts/reread");
-		system("touch ${General::swroot}/forward/reread");
-	}
-}
-sub reread_rules
-{
-	system ("/usr/local/bin/forwardfwctrl");
-	if ( -f "${General::swroot}/fwhosts/reread"){
-		system("rm ${General::swroot}/fwhosts/reread");
-		system("rm ${General::swroot}/forward/reread");
-	}
-	
-}
-sub decrease
-{
+
+sub decrease {
 	my $grp=$_[0];
 	&General::readhasharray("$confignet", \%customnetwork);
 	&General::readhasharray("$confighost", \%customhost);
--- a/html/cgi-bin/p2p-block.cgi
+++ b/html/cgi-bin/p2p-block.cgi
@@ -43,18 +43,12 @@ my %mainsettings=();
 &General::readhash("${General::swroot}/main/settings", \%mainsettings);
 &General::readhash("/srv/web/ipfire/html/themes/".$mainsettings{'THEME'}."/include/colors.txt", \%color);

-
-
 &Header::showhttpheaders();
 &Header::getcgihash(\%fwdfwsettings);
 &Header::openpage($Lang::tr{'fwdfw menu'}, 1, '');
-&Header::openbigbox('100%', 'center',$errormessage);
+&Header::openbigbox('100%', 'center', $errormessage);

-if ($fwdfwsettings{'ACTION'} eq ''){
-&p2pblock;
-}
-if ($fwdfwsettings{'ACTION'} eq 'togglep2p')
-{
+if ($fwdfwsettings{'ACTION'} eq 'togglep2p') {
 	open( FILE, "< $p2pfile" ) or die "Unable to read $p2pfile";
 	@p2ps = <FILE>;
 	close FILE;
@@ -72,21 +66,15 @@ if ($fwdfwsettings{'ACTION'} eq 'togglep2p')
 		print FILE "$p2pline[0];$p2pline[1];$p2pline[2];\n";
 	}
 	close FILE;
-	&rules;
-	&p2pblock;
-}
-if ($fwdfwsettings{'ACTION'} eq $Lang::tr{'fwdfw reread'})
-{
-	&reread_rules;
-	&p2pblock;
-}

+	&General::firewall_config_changed();
+	&p2pblock();
+} else {
+	&p2pblock();
+}

 sub p2pblock
 {
-	if (-f "${General::swroot}/forward/reread"){
-		print "<table border='1' rules='groups' bgcolor='lightgreen' width='100%'><form method='post'><td><div style='font-size:11pt; font-weight: bold;vertical-align: middle; '><input type='submit' name='ACTION' value='$Lang::tr{'fwdfw reread'}' style='font-face: Comic Sans MS; color: green; font-weight: bold; font-size: 14pt;'>&nbsp &nbsp $Lang::tr{'fwhost reread'}</div></td></tr></table></form><br>";
-	}
 	my $gif;
 	open( FILE, "< $p2pfile" ) or die "Unable to read $p2pfile";
 	@p2ps = <FILE>;
@@ -115,20 +103,6 @@ END
 	print"<br><br><br><table width='100%'><tr><td align='left'>$Lang::tr{'fwdfw p2p txt'}</td></tr></table>";
 	&Header::closebox();
 }
-sub rules
-{
-	if (!-f "${General::swroot}/forward/reread"){
-		system("touch ${General::swroot}/forward/reread");
-		system("touch ${General::swroot}/fwhosts/reread");
-	}
-}
-sub reread_rules
-{
-	system("/usr/local/bin/forwardfwctrl");
-	if ( -f "${General::swroot}/forward/reread"){
-		system("rm ${General::swroot}/forward/reread");
-		system("rm ${General::swroot}/fwhosts/reread");
-	}
-}
+
 &Header::closebigbox();
 &Header::closepage();
--- a/src/misc-progs/forwardfwctrl.c
+++ b/src/misc-progs/forwardfwctrl.c
@@ -5,12 +5,21 @@
 *
 */

+#include <unistd.h>
+
 #include "setuid.h"

 int main(int argc, char *argv[]) {
 	if (!(initsetuid()))
 		exit(1);

-	safe_system("/var/ipfire/forward/bin/rules.pl");
+	int retval = safe_system("/var/ipfire/forward/bin/rules.pl");
+
+	/* If rules.pl has been successfully executed, the indicator
+	 * file is removed. */
+	if (retval == 0) {
+		unlink("/var/ipfire/forward/reread");
+	}
+
 	return 0;
 }