[1/2] rules.pl: Allow dynamic destory of loaded but unused ipset sets.

Message ID 20220227134903.1828-1-stefan.schantl@ipfire.org
State Accepted
Commit 2801213dcc97329d5ab24ec0483fdbc5020e0247
Headers
Series [1/2] rules.pl: Allow dynamic destory of loaded but unused ipset sets. |

Commit Message

Stefan Schantl Feb. 27, 2022, 1:49 p.m. UTC
  Instead of stupidly destroying all ipsets, we now grab the already loaded sets
and compare them with the loaded sets during runtime of the script.

So we are now able to determine which sets are not longer required and
safely can destroy (unload) at a later time.

This saves us from taking care about dropping/flushing rules which are
based on ipset before we can destroy them - because only unused sets are
affected.

Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
Inspired-by: Tim FitzGeorge <ipfr@tfitzgeorge.me.uk>
---
 config/firewall/rules.pl | 68 ++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 16 deletions(-)
  

Comments

Michael Tremer March 1, 2022, 1:23 p.m. UTC | #1
Hello,

> On 27 Feb 2022, at 13:49, Stefan Schantl <stefan.schantl@ipfire.org> wrote:
> 
> Instead of stupidly destroying all ipsets, we now grab the already loaded sets
> and compare them with the loaded sets during runtime of the script.
> 
> So we are now able to determine which sets are not longer required and
> safely can destroy (unload) at a later time.
> 
> This saves us from taking care about dropping/flushing rules which are
> based on ipset before we can destroy them - because only unused sets are
> affected.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
> Inspired-by: Tim FitzGeorge <ipfr@tfitzgeorge.me.uk>

LOL. This isn’t an official tag :)

But nevertheless:

Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>

> ---
> config/firewall/rules.pl | 68 ++++++++++++++++++++++++++++++----------
> 1 file changed, 52 insertions(+), 16 deletions(-)
> 
> diff --git a/config/firewall/rules.pl b/config/firewall/rules.pl
> index 927c1f2ba..7a7c8ed31 100644
> --- a/config/firewall/rules.pl
> +++ b/config/firewall/rules.pl
> @@ -70,7 +70,8 @@ my %confignatfw=();
> my %locationsettings = (
> 	"LOCATIONBLOCK_ENABLED" => "off"
> );
> -my %loaded_ipset_lists=();
> +my %ipset_loaded_sets = ();
> +my @ipset_used_sets = ();
> 
> my $configfwdfw		= "${General::swroot}/firewall/config";
> my $configinput	    = "${General::swroot}/firewall/input";
> @@ -114,12 +115,12 @@ undef (@dummy);
> &main();
> 
> sub main {
> +	# Get currently used ipset sets.
> +	&ipset_get_sets();
> +
> 	# Flush all chains.
> 	&flush();
> 
> -	# Destroy all existing ipsets.
> -	run("$IPSET destroy");
> -
> 	# Prepare firewall rules.
> 	if (! -z  "${General::swroot}/firewall/input"){
> 		&buildrules(\%configinputfw);
> @@ -137,6 +138,9 @@ sub main {
> 	# Reload firewall policy.
> 	run("/usr/sbin/firewall-policy");
> 
> +	# Cleanup not longer needed ipset sets.
> +	&ipset_cleanup();
> +
> 	#Reload firewall.local if present
> 	if ( -f '/etc/sysconfig/firewall.local'){
> 		run("/etc/sysconfig/firewall.local reload");
> @@ -189,9 +193,6 @@ sub flush {
> 	run("$IPTABLES -t nat -F $CHAIN_NAT_SOURCE");
> 	run("$IPTABLES -t nat -F $CHAIN_NAT_DESTINATION");
> 	run("$IPTABLES -t mangle -F $CHAIN_MANGLE_NAT_DESTINATION_FIX");
> -
> -	# Flush LOCATIONBLOCK chain.
> -	run("$IPTABLES -F LOCATIONBLOCK");
> }
> 
> sub buildrules {
> @@ -639,7 +640,8 @@ sub time_convert_to_minutes {
> }
> 
> sub locationblock {
> -	# The LOCATIONBLOCK chain now gets flushed by the flush() function.
> +	# Flush LOCATIONBLOCK chain.
> +	run("$IPTABLES -F LOCATIONBLOCK");
> 
> 	# If location blocking is not enabled, we are finished here.
> 	if ($locationsettings{'LOCATIONBLOCK_ENABLED'} ne "on") {
> @@ -669,7 +671,7 @@ sub locationblock {
> 			&ipset_restore($location);
> 
> 			# Call iptables and create rule to use the loaded ipset list.
> -			run("$IPTABLES -A LOCATIONBLOCK -m set --match-set CC_$location src -j DROP");
> +			run("$IPTABLES -A LOCATIONBLOCK -m set --match-set $location src -j DROP");
> 		}
> 	}
> }
> @@ -887,24 +889,58 @@ sub firewall_is_in_subnet {
> 	return 0;
> }
> 
> +sub ipset_get_sets () {
> +	# Get all currently used ipset lists and store them in an array.
> +	my @output = `$IPSET -n list`;
> +
> +	# Loop through the temporary array.
> +	foreach my $set (@output) {
> +		# Remove any newlines.
> +		chomp($set);
> +
> +		# Add the set the array of used sets.
> +		push(@ipset_used_sets, $set);
> +	}
> +
> +	# Display used sets in debug mode.
> +	if($DEBUG) {
> +		print "Used ipset sets:\n";
> +		print "@ipset_used_sets\n\n";
> +	}
> +}
> +
> sub ipset_restore ($) {
> -	my ($list) = @_;
> +	my ($set) = @_;
> 
> 	my $file_prefix = "ipset4";
> -	my $db_file = "$Location::Functions::ipset_db_directory/$list.$file_prefix";
> +	my $db_file = "$Location::Functions::ipset_db_directory/$set.$file_prefix";
> 
> -	# Check if the network list already has been loaded.
> -	if($loaded_ipset_lists{$list}) {
> +	# Check if the set already has been loaded.
> +	if($ipset_loaded_sets{$set}) {
> 		# It already has been loaded - so there is nothing to do.
> 		return;
> 	}
> 
> 	# Check if the generated file exists.
> 	if (-f $db_file) {
> -		# Run ipset and restore the list of the given country code.
> +		# Run ipset and restore the given set.
> 		run("$IPSET restore < $db_file");
> 
> -		# Store the restored list name to the hash to prevent from loading it again.
> -		$loaded_ipset_lists{$list} = "1";
> +		# Store the restored set to the hash to prevent from loading it again.
> +		$ipset_loaded_sets{$set} = "1";
> +	}
> +}
> +
> +sub ipset_cleanup () {
> +	# Loop through the array of used sets.
> +	foreach my $set (@ipset_used_sets) {
> +		# Check if this set is still in use.
> +		#
> +		# In this case an entry in the loaded sets hash exists.
> +		unless($ipset_loaded_sets{$set}) {
> +			# Entry does not exist, so this set is not longer
> +			# used and can be destroyed.
> +			run("$IPSET destroy $set");
> +		}
> 	}
> }
> -- 
> 2.30.2
>
  

Patch

diff --git a/config/firewall/rules.pl b/config/firewall/rules.pl
index 927c1f2ba..7a7c8ed31 100644
--- a/config/firewall/rules.pl
+++ b/config/firewall/rules.pl
@@ -70,7 +70,8 @@  my %confignatfw=();
 my %locationsettings = (
 	"LOCATIONBLOCK_ENABLED" => "off"
 );
-my %loaded_ipset_lists=();
+my %ipset_loaded_sets = ();
+my @ipset_used_sets = ();
 
 my $configfwdfw		= "${General::swroot}/firewall/config";
 my $configinput	    = "${General::swroot}/firewall/input";
@@ -114,12 +115,12 @@  undef (@dummy);
 &main();
 
 sub main {
+	# Get currently used ipset sets.
+	&ipset_get_sets();
+
 	# Flush all chains.
 	&flush();
 
-	# Destroy all existing ipsets.
-	run("$IPSET destroy");
-
 	# Prepare firewall rules.
 	if (! -z  "${General::swroot}/firewall/input"){
 		&buildrules(\%configinputfw);
@@ -137,6 +138,9 @@  sub main {
 	# Reload firewall policy.
 	run("/usr/sbin/firewall-policy");
 
+	# Cleanup not longer needed ipset sets.
+	&ipset_cleanup();
+
 	#Reload firewall.local if present
 	if ( -f '/etc/sysconfig/firewall.local'){
 		run("/etc/sysconfig/firewall.local reload");
@@ -189,9 +193,6 @@  sub flush {
 	run("$IPTABLES -t nat -F $CHAIN_NAT_SOURCE");
 	run("$IPTABLES -t nat -F $CHAIN_NAT_DESTINATION");
 	run("$IPTABLES -t mangle -F $CHAIN_MANGLE_NAT_DESTINATION_FIX");
-
-	# Flush LOCATIONBLOCK chain.
-	run("$IPTABLES -F LOCATIONBLOCK");
 }
 
 sub buildrules {
@@ -639,7 +640,8 @@  sub time_convert_to_minutes {
 }
 
 sub locationblock {
-	# The LOCATIONBLOCK chain now gets flushed by the flush() function.
+	# Flush LOCATIONBLOCK chain.
+	run("$IPTABLES -F LOCATIONBLOCK");
 
 	# If location blocking is not enabled, we are finished here.
 	if ($locationsettings{'LOCATIONBLOCK_ENABLED'} ne "on") {
@@ -669,7 +671,7 @@  sub locationblock {
 			&ipset_restore($location);
 
 			# Call iptables and create rule to use the loaded ipset list.
-			run("$IPTABLES -A LOCATIONBLOCK -m set --match-set CC_$location src -j DROP");
+			run("$IPTABLES -A LOCATIONBLOCK -m set --match-set $location src -j DROP");
 		}
 	}
 }
@@ -887,24 +889,58 @@  sub firewall_is_in_subnet {
 	return 0;
 }
 
+sub ipset_get_sets () {
+	# Get all currently used ipset lists and store them in an array.
+	my @output = `$IPSET -n list`;
+
+	# Loop through the temporary array.
+	foreach my $set (@output) {
+		# Remove any newlines.
+		chomp($set);
+
+		# Add the set the array of used sets.
+		push(@ipset_used_sets, $set);
+	}
+
+	# Display used sets in debug mode.
+	if($DEBUG) {
+		print "Used ipset sets:\n";
+		print "@ipset_used_sets\n\n";
+	}
+}
+
 sub ipset_restore ($) {
-	my ($list) = @_;
+	my ($set) = @_;
 
 	my $file_prefix = "ipset4";
-	my $db_file = "$Location::Functions::ipset_db_directory/$list.$file_prefix";
+	my $db_file = "$Location::Functions::ipset_db_directory/$set.$file_prefix";
 
-	# Check if the network list already has been loaded.
-	if($loaded_ipset_lists{$list}) {
+	# Check if the set already has been loaded.
+	if($ipset_loaded_sets{$set}) {
 		# It already has been loaded - so there is nothing to do.
 		return;
 	}
 
 	# Check if the generated file exists.
 	if (-f $db_file) {
-		# Run ipset and restore the list of the given country code.
+		# Run ipset and restore the given set.
 		run("$IPSET restore < $db_file");
 
-		# Store the restored list name to the hash to prevent from loading it again.
-		$loaded_ipset_lists{$list} = "1";
+		# Store the restored set to the hash to prevent from loading it again.
+		$ipset_loaded_sets{$set} = "1";
+	}
+}
+
+sub ipset_cleanup () {
+	# Loop through the array of used sets.
+	foreach my $set (@ipset_used_sets) {
+		# Check if this set is still in use.
+		#
+		# In this case an entry in the loaded sets hash exists.
+		unless($ipset_loaded_sets{$set}) {
+			# Entry does not exist, so this set is not longer
+			# used and can be destroyed.
+			run("$IPSET destroy $set");
+		}
 	}
 }