From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Schantl <stefan.schantl@ipfire.org>
To: development@lists.ipfire.org
Subject: [PATCH] update-ipblocklists: Fix loading new blocklists after update
Date: Tue, 28 Mar 2023 18:05:42 +0200
Message-ID: <20230328160542.132432-1-stefan.schantl@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1766506979500151064=="
List-Id: <development.lists.ipfire.org>

--===============1766506979500151064==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

* The script needs to run with root permissions in order to
  do the ipset operations. So remove code to drop the permissions
  on startup.

* Adjust execute calls to use the proper functions from
  general functions.

* Add some code to set the correct ownership (nobody:nobody) for
  changed files during script runtime.

Fixes #13072.

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/cfgroot/ipblocklist-functions.pl | 27 ++++++++++++++++++++++++
 src/scripts/update-ipblocklists         | 28 +++++++++++--------------
 2 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/config/cfgroot/ipblocklist-functions.pl b/config/cfgroot/ipblock=
list-functions.pl
index ecabf42e8..bd026a01d 100644
--- a/config/cfgroot/ipblocklist-functions.pl
+++ b/config/cfgroot/ipblocklist-functions.pl
@@ -383,4 +383,31 @@ sub get_holdoff_rate($) {
 	return $value;
 }
=20
+#
+## sub set_ownership(file)
+##
+## Function to set the correct ownership (nobody:nobody) to a given file.
+##
+#
+sub set_ownership($) {
+	my ($file) =3D @_;
+
+	# User and group of the WUI.
+	my $uname =3D "nobody";
+	my $grname =3D "nobody";
+
+	# The chown function implemented in perl requies the user and group as numm=
eric id's.
+	my $uid =3D getpwnam($uname);
+	my $gid =3D getgrnam($grname);
+
+	# Check if the given file exists.
+	unless ($file) {
+		# Stop the script and print error message.
+		die "The given $file does not exist. Cannot change the ownership!\n";
+	}
+
+	# Change ownership of the file.
+	chown($uid, $gid, "$file");
+}
+
 1;
diff --git a/src/scripts/update-ipblocklists b/src/scripts/update-ipblocklists
index 9918cac41..a17b47999 100644
--- a/src/scripts/update-ipblocklists
+++ b/src/scripts/update-ipblocklists
@@ -32,19 +32,6 @@ require "${General::swroot}/lang.pl";
 # Hash to store the settings.
 my %settings =3D ();
=20
-# The user and group name as which this script should be run.
-my $run_as =3D 'nobody';
-
-# Get user and group id of the user.
-my ( $uid, $gid ) =3D ( getpwnam $run_as )[ 2, 3 ];
-
-# Check if the script currently runs as root.
-if ( $> =3D=3D 0 ) {
-	# Drop privileges and switch to the specified user and group.
-	POSIX::setgid( $gid );
-	POSIX::setuid( $uid );
-}
-
 # Establish the connection to the syslog service.
 openlog('ipblocklist', 'cons', 'user');
=20
@@ -122,6 +109,12 @@ foreach my $blocklist (@blocklists) {
 			&_log_to_syslog("<ERROR> Could not update $blocklist blocklist - Unexpect=
ed error\!");
 		}
 	} else {
+		# Get the filename of the blocklist.
+		my $ipset_db_file =3D &IPblocklist::get_ipset_db_file($blocklist);
+
+		# Set the correct ownership.
+		&IPblocklist::set_ownership($ipset_db_file);
+
 		# Log successfull update.
 		&_log_to_syslog("<INFO> Successfully updated $blocklist blocklist.");
=20
@@ -132,22 +125,25 @@ foreach my $blocklist (@blocklists) {
=20
 # Check if a blocklist has been updated and therefore needs to be reloaded.
 if (@updated_blocklists) {
+	# Set correct ownership to the modified file.
+	&IPblocklist::set_ownership($IPblocklist::modified_file);
+
 	# Loop through the array.
 	foreach my $updated_blocklist (@updated_blocklists) {
 		# Get the blocklist file.
 		my $ipset_db_file =3D &IPblocklist::get_ipset_db_file($updated_blocklist);
=20
 		# Call safe system function to reload/update the blocklist.
-		&General::system("ipset", "restore", "-f", "$ipset_db_file");
+		&General::safe_system("ipset", "restore", "-f", "$ipset_db_file");
=20
 		# The set name contains a "v4" as suffix.
 		my $set_name =3D "$updated_blocklist" . "v4";
=20
 		# Swap the sets to use the new one.
-		&General::system("ipset", "swap", "$set_name", "$updated_blocklist");
+		&General::safe_system("ipset", "swap", "$set_name", "$updated_blocklist");
=20
 		# Destroy the old blocklist.
-		&General::system("ipset", "destroy", "$set_name");
+		&General::safe_system("ipset", "destroy", "$set_name");
 	}
 }
=20
--=20
2.30.2


--===============1766506979500151064==--