spamd - code style cleanup and multiple fixes

- Code style and indentation fixes
- Use install_cron_job() instead of re-implementing it
- Use restart_service()
- Clean up user/groups after itself on uninstall (fixes Bug #4929)
- Fix input validation (no errors shown due to a typo in variable name)
- Better log messages plus some cosmetics

1 files changed, 93 insertions, 118 deletions

diff --git a/config/spamd/spamd.inc b/config/spamd/spamd.inc
index a72d8770..98cdc561 100644
--- a/config/spamd/spamd.inc
+++ b/config/spamd/spamd.inc
@@ -1,9 +1,9 @@
 <?php
-/* $Id$ */
 /*
 	spamd.inc
-	part of the SpamD package for pfSense
+	part of pfSense (https://www.pfSense.org/)
 	Copyright (C) 2008 Scott Ullrich
+	Copyright (C) 2015 ESF, LLC
 	All rights reserved.
 
 	Redistribution and use in source and binary forms, with or without
@@ -26,29 +26,28 @@
 	CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 	ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 	POSSIBILITY OF SUCH DAMAGE.
-
 */
-
-if(!function_exists("filter_configure")) 
+if (!function_exists("filter_configure")) {
 	require_once("filter.inc");
+}
 
 function sync_package_spamd() {
-	global $config, $g;
-	$pf_version = substr(trim(file_get_contents("/etc/version")),0,3);
+	global $config, $localpath;
+	$pf_version = substr(trim(file_get_contents("/etc/version")), 0, 3);
 	if ($pf_version != "2.2") {
 		$localpath = "/usr/local";
 	} else {
 		$ARCH = php_uname("m");
 		$localpath = "/usr/pbi/spamd-$ARCH/local";
 	}
-	
+
 	conf_mount_rw();
 	$fd = fopen("/etc/spamd.conf","w");
 	/* all header */
 	fwrite($fd, "all:\\\n\t:whitelist:blacklist");
-	if($config['installedpackages']['spamdsources']['config']) {
-		foreach($config['installedpackages']['spamdsources']['config'] as $spamd) {
-			if($spamd['providername']) {
+	if ($config['installedpackages']['spamdsources']['config']) {
+		foreach ($config['installedpackages']['spamdsources']['config'] as $spamd) {
+			if ($spamd['providername']) {
 				fwrite($fd, ":" . remove_spaces($spamd['providername']));
 			}
 		}
@@ -64,12 +63,12 @@ function sync_package_spamd() {
 	fwrite($fd, "\t:msg=\"Sorry, you spammed us before.\":\\\n");
 	fwrite($fd, "\t:method=file:\\\n");
 	fwrite($fd, "\t:file=/var/db/blacklist.txt:\n\n");
-	log_error("Looping through each item and write out its configuration");
+	log_error("[spamd] Looping through each item and write out its configuration");
 	/* loop through each item and write out its configuration */
-	if($config['installedpackages']['spamdsources']['config'] != "") {
-		foreach($config['installedpackages']['spamdsources']['config'] as $spamd) {
-			if(remove_spaces($spamd['providername'])) {
-				if($spamd['providername']) {
+	if ($config['installedpackages']['spamdsources']['config'] != "") {
+		foreach ($config['installedpackages']['spamdsources']['config'] as $spamd) {
+			if (remove_spaces($spamd['providername'])) {
+				if ($spamd['providername']) {
 					fwrite($fd, remove_spaces($spamd['providername']) . ":\\\n");
 					fwrite($fd, "\t:" . remove_spaces($spamd['providertype']) . ":\\\n");
 					fwrite($fd, "\t:msg=\"" . rtrim($spamd['rejectmessage']) . "\":\\\n");
@@ -83,12 +82,13 @@ function sync_package_spamd() {
 		}
 	}
 	fclose($fd);
-		log_error("Creating /var/db/whitelist.txt");
+	log_error("[spamd] Creating /var/db/whitelist.txt");
 	$fd = fopen("/var/db/whitelist.txt","w");
-	if($config['installedpackages']['spamdwhitelist']['config'] != "") {
-	    foreach($config['installedpackages']['spamdwhitelist']['config'] as $spamd) {
-			if($spamd['ip'])
+	if ($config['installedpackages']['spamdwhitelist']['config'] != "") {
+		foreach ($config['installedpackages']['spamdwhitelist']['config'] as $spamd) {
+			if ($spamd['ip']) {
 				fwrite($fd, $spamd['ip'] . "\n");
+			}
 		}
 	}
 	fclose($fd);
@@ -100,38 +100,50 @@ function sync_package_spamd() {
 	$maxblack = "";
 	$stuttersecs = "";
 	$delaysecs = "";
-	log_error("Looping through spamdsettings");
-	if($config['installedpackages']['spamdsettings']['config']) {
-		foreach($config['installedpackages']['spamdsettings']['config'] as $ss) {
-			if($ss['nextmta'] <> "")
+	log_error("[spamd] Looping through spamdsettings");
+	if ($config['installedpackages']['spamdsettings']['config']) {
+		foreach ($config['installedpackages']['spamdsettings']['config'] as $ss) {
+			if ($ss['nextmta'] != "") {
 				$nextmta = $ss['nextmta'];
-			if($ss['greylistingparms'] <> "")
+			}
+			if ($ss['greylistingparms'] != "") {
 				$passtime = " -G " . $ss['greylistingparms'];
-			if($ss['identifier'] <> "")
+			}
+			if ($ss['identifier'] != "") {
 				$identifier = " -n \"" . $ss['identifier'] . "\"";
+			}
 			// Default is greylisting, turn on blacklisting if not checked.
-			if($ss['greylisting'] <> "on")
+			if ($ss['greylisting'] != "on") {
 				$greylisting = " -b";
-			if($ss['maxblack'] <> "")
+			}
+			if ($ss['maxblack'] != "") {
 				$maxblack = " -B " . $ss['maxblack'];
-			if($ss['maxcon'] <> "")
+			}
+			if ($ss['maxcon'] != "") {
 				$maxcon = " -c " . $ss['maxcon'];
-			if($ss['stuttersecs'] <> "")
+			}
+			if ($ss['stuttersecs'] != "") {
 				$stuttersecs = " -S " . $ss['stuttersecs'];
-			if($ss['delaysecs'] <> "")
+			}
+			if ($ss['delaysecs'] != "") {
 				$delaysecs = " -s " . $ss['delaysecs'];
-			if($ss['window'] <> "")
+			}
+			if ($ss['window'] != "") {
 				$window = " -w " . $ss['window'];
-			if($ss['passtime'] <> "")
+			}
+			if ($ss['passtime'] != "") {
 				$passtime = $ss['passtime'];
-			if($ss['greyexp'] <> "")
+			}
+			if ($ss['greyexp'] != "") {
 				$greyexp = $ss['greyexp'];
-			if($ss['whiteexp'] <> "")
+			}
+			if ($ss['whiteexp'] != "") {
 				$whiteexp = $ss['whiteexp'];
+			}
 		}
 	}
 	$greyparms = " -G {$passtime}:{$greyexp}:{$whiteexp}";
-	$start = "if [ `mount | grep -v grep | grep fdescfs | wc -l` -lt 1 ]; then \n" .
+	$start = "if [ `/sbin/mount | /usr/bin/grep -v grep | /usr/bin/grep fdescfs | /usr/bin/wc -l` -lt 1 ]; then \n" .
 		 "/sbin/mount -t fdescfs fdescfs /dev/fd\n" .
 		 "fi\n" .
 	     "/usr/local/sbin/spamd-setup -d &\n" .
@@ -142,32 +154,25 @@ function sync_package_spamd() {
 		 "/usr/bin/killall spamlogd\n" .
 		 "/usr/bin/killall spamd\n" .
 		 "/usr/bin/killall pflogd\n" .
-		 "sleep 2";
-	log_error("Writing rc_file");		
+		 "sleep 3";
+	log_error("[spamd] Writing rc_file");
 	write_rcfile(array(
 				"file" => "spamd.sh",
 				"start" => $start,
 				"stop" =>  $stop
 			    )
 	);
-	log_error("Installing CRON");	
-	spamd_install_cron(true);
-	log_error("Mounting RO");	
-	conf_mount_ro();
-	log_error("Restart cron");	
-	mwexec("killall -HUP cron");
-	log_error("Setting up spamd.conf symlink");	
+	log_error("[spamd] Installing cron job");
+	install_cron_job("/usr/bin/nice -n20 /usr/local/sbin/spamd-setup", true, "*/120");
+	log_error("[spamd] Setting up spamd.conf symlink");
 	unlink_if_exists("$localpath/etc/spamd/spamd.conf");
 	symlink("/etc/spamd.conf", "$localpath/etc/spamd/spamd.conf");
-	
-	log_error("Stopping spamd");
-	mwexec("/usr/local/etc/rc.d/spamd.sh stop");
-	sleep(1);
-	log_error("Starting spamd");
-	mwexec_bg("/usr/local/etc/rc.d/spamd.sh start");
-	log_error("Reconfiguring filter");
+	conf_mount_ro();
+	log_error("[spamd] Restarting spamd");
+	restart_service("spamd");
+	log_error("[spamd] Reconfiguring filter");
 	filter_configure();
-	log_error("SpamD setup completed");	
+	log_error("[spamd] Package setup completed");
 }
 
 function sync_package_spamd_whitelist() {
@@ -175,8 +180,8 @@ function sync_package_spamd_whitelist() {
 	conf_mount_rw();
 	/* write out ip to the whitelist db */
 	$fd = fopen("/var/db/whitelist.txt","w");
-	if($config['installedpackages']['spamdwhitelist']['config'] != "") {
-		foreach($config['installedpackages']['spamdwhitelist']['config'] as $spamd) {
+	if ($config['installedpackages']['spamdwhitelist']['config'] != "") {
+		foreach ($config['installedpackages']['spamdwhitelist']['config'] as $spamd) {
 			fwrite($fd, $spamd['ip'] . "\n");
 		}
 	}
@@ -219,7 +224,7 @@ function spamd_generate_rules($type) {
 					$natrules .= "rdr pass on {$wanif} proto tcp from <blacklist> to port smtp -> 127.0.0.1 port spamd\n";
 					$natrules .= "rdr pass on {$wanif} proto tcp from <spamd> to port smtp -> 127.0.0.1 port spamd\n";
 					$natrules .= "rdr pass on {$wanif} proto tcp from !<spamd-white> to port smtp -> 127.0.0.1 port spamd\n";
-					if ($nextmta <> "") {
+					if ($nextmta != "") {
 						$natrules .= "rdr pass on {$wanif} proto tcp from <spamd-white> to port smtp -> {$nextmta} port smtp\n";
 					}
 				}
@@ -229,7 +234,7 @@ function spamd_generate_rules($type) {
 
 		break;
 	}
-	
+
 	return $natrules;
 }
 
@@ -239,28 +244,29 @@ function remove_spaces($string) {
 }
 
 function sync_spamd_config_to_backup() {
-	global $g, $config;
-	if(is_array($config['installedpackages']['carpsettings']['config'])) {
-		foreach($config['installedpackages']['carpsettings']['config'] as $carp) {
-			if($carp['synchronizetoip'] != "" ) {
+	global $config;
+	if (is_array($config['installedpackages']['carpsettings']['config'])) {
+		foreach ($config['installedpackages']['carpsettings']['config'] as $carp) {
+			if ($carp['synchronizetoip'] != "" ) {
 				$synctoip = $carp['synchronizetoip'];
 				$password = $carp['password'];
-				if($config['system']['username'])
+				if ($config['system']['username']) {
 					$username = $config['system']['username'];
-				else 
+				} else {
 					$username = "admin";
+				}
 			}
 		}
 	}
-	if($synctoip and $password) {
-		if($config['system']['webgui']['protocol'] != "") {
+	if ($synctoip and $password) {
+		if ($config['system']['webgui']['protocol'] != "") {
 			$synchronizetoip = $config['system']['webgui']['protocol'];
 			$synchronizetoip .= "://";
 		}
 		$port = $config['system']['webgui']['port'];
 		/* if port is empty lets rely on the protocol selection */
-		if($port == "") {
-			if($config['system']['webgui']['protocol'] == "http") {
+		if ($port == "") {
+			if ($config['system']['webgui']['protocol'] == "http") {
 				$port = "80";
 			} else {
 				$port = "443";
@@ -270,7 +276,7 @@ function sync_spamd_config_to_backup() {
 		/* create files to sync array */
 		$filetosync = array("/var/db/spamd", "/var/db/whitelist.txt");
 		/* loop through files to sync list and sync them up */
-		foreach($filetosync as $f2s) {
+		foreach ($filetosync as $f2s) {
 			$f2c_contents = file_get_contents($f2s);
 			xmlrpc_sync_file($url, $password, $f2s, $f2c_contents, $port);
 		}
@@ -281,7 +287,7 @@ function sync_spamd_config_to_backup() {
 }
 
 function custom_php_install_command() {
-	global $config, $g;
+	global $config;
 	system("touch /var/db/whitelist.txt");
 	system("touch /var/db/blacklist.txt");
 	exec("/usr/sbin/pw usermod _spamd -g proxy -G _spamd,proxy");
@@ -289,69 +295,38 @@ function custom_php_install_command() {
 }
 
 function custom_php_deinstall_command() {
-	global $config, $g;
+	global $config;
 	unlink_if_exists("/usr/local/pkg/pf/spamd_rules.php");
-	spamd_install_cron(false);
-	filter_configure();
-}
-
-function spamd_install_cron($should_install) {
-	global $config, $g;
-	$is_installed = false;
-	if(!$config['cron']['item'])
-		return;
-	$x=0;
-	foreach($config['cron']['item'] as $item) {
-		if(strstr($item['command'], "spamd-setup")) {
-			$is_installed = true;
-			break;
-		}
-		$x++;
+	install_cron_job("/usr/bin/nice -n20 /usr/local/sbin/spamd-setup", false);
+	/* clean up user/groups */
+	if (exec("/usr/sbin/pw groupshow proxy | /usr/bin/grep _spamd")) {
+		exec ("/usr/sbin/pw groupmod proxy -d _spamd");
 	}
-	switch($should_install) {
-		case true:
-			if(!$is_installed) {
-				$cron_item = array();
-				$cron_item['minute'] = "*/120";
-				$cron_item['hour'] = "*";
-				$cron_item['mday'] = "*";
-				$cron_item['month'] = "*";
-				$cron_item['wday'] = "*";
-				$cron_item['who'] = "root";
-				$cron_item['command'] = "/usr/bin/nice -n20 /usr/local/sbin/spamd-setup";		
-				$config['cron']['item'][] = $cron_item;
-				write_config("Installed SPAMD crontab entries.");
-				configure_cron();
-			}
-		break;
-		case false:
-			if($is_installed == true) {
-				if($x > 0) {
-					unset($config['cron']['item'][$x]);
-					write_config();
-				}
-				configure_cron();
-			}
-		break;
+	if (exec("/usr/sbin/pw usershow _spamd")) {
+		exec("/usr/sbin/pw userdel _spamd");
+	}
+	if (exec("/usr/sbin/pw groupshow _spamd")) {
+		exec("/usr/sbin/pw groupdel _spamd");
 	}
+	filter_configure();
 }
 
 function spamd_validate_input($post, &$input_errors) {
-	global $config, $g;
+	global $config;
 	$nextmta = str_replace("$", "", $post['nextmta']);
-	if(stristr($nextmta, "{")) {
+	if (stristr($nextmta, "{")) {
 		/* item is an alias, make sure the name exists */
 		$nextmta = str_replace("$", "", $nextmta);
 		$found = false;
-		if($config['aliases']['alias']) {
-			foreach($config['aliases']['alias'] as $alias) {
-				if($alias['name'] == $nextmta) {
+		if ($config['aliases']['alias']) {
+			foreach ($config['aliases']['alias'] as $alias) {
+				if ($alias['name'] == $nextmta) {
 					$found = true;
 				}
 			}
 		}
-		if($found == false)
-			$intput_errors = "Could not locate alias named " . htmlentities($nextmta);
+		if ($found == false)
+			$input_errors[] = "Could not locate alias named " . htmlentities($nextmta);
 	}
 }