[PATCH 1/5] suricata: correct rule actions in IPS mode

[Stefan Schantl <stefan.schantl@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 11152 bytes --]

From: Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk>

In IPS mode rule actions need to be have the action 'drop' for the
protection to work, however this is not appropriate for all rules.
Modify the generator for oinkmaster-modify-sids.conf to leave
rules with the action 'alert' here this is appropriate.  Also add
a script to be run on update to correct existing downloaded rules.

Fixes #12086

Signed-off-by: Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk>
Tested-by: Peter Müller <peter.mueller(a)ipfire.org>
Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/cfgroot/ids-functions.pl             | 44 +++++++++--
 config/rootfiles/common/configroot          |  1 +
 config/rootfiles/core/133/update.sh         |  3 +
 config/suricata/convert-ids-modifysids-file | 84 +++++++++++++++++++++
 html/cgi-bin/ids.cgi                        | 22 +++++-
 lfs/configroot                              |  1 +
 6 files changed, 148 insertions(+), 7 deletions(-)
 create mode 100644 config/suricata/convert-ids-modifysids-file

diff --git a/config/cfgroot/ids-functions.pl b/config/cfgroot/ids-functions.pl
index 88734a3ca..e1caa6e58 100644
--- a/config/cfgroot/ids-functions.pl
+++ b/config/cfgroot/ids-functions.pl
@@ -243,7 +243,7 @@ sub downloadruleset {
 	# Load perl module to deal with temporary files.
 	use File::Temp;
 
-	# Generate temporay file name, located in "/var/tmp" and with a suffix of ".tar.gz".
+	# Generate temporary file name, located in "/var/tmp" and with a suffix of ".tar.gz".
 	my $tmp = File::Temp->new( SUFFIX => ".tar.gz", DIR => "/var/tmp/", UNLINK => 0 );
 	my $tmpfile = $tmp->filename();
 
@@ -293,6 +293,9 @@ sub downloadruleset {
 	# Overwrite existing rules tarball with the new downloaded one.
 	move("$tmpfile", "$rulestarball");
 
+	# Set correct ownership for the rulesdir and files.
+	set_ownership("$rulestarball");
+
 	# If we got here, everything worked fine. Return nothing.
 	return;
 }
@@ -726,8 +729,8 @@ sub write_used_rulefiles_file(@) {
 #
 ## Function to generate and write the file for modify the ruleset.
 #
-sub write_modify_sids_file($) {
-	my ($ruleaction) = @_;
+sub write_modify_sids_file($$) {
+	my ($ruleaction,$rulefile) = @_;
 
 	# Open modify sid's file for writing.
 	open(FILE, ">$modify_sids_file") or die "Could not write to $modify_sids_file. $!\n";
@@ -737,8 +740,39 @@ sub write_modify_sids_file($) {
 
 	# Check if the traffic only should be monitored.
 	unless($ruleaction eq "alert") {
-		# Tell oinkmaster to switch all rules from alert to drop.
-		print FILE "modifysid \* \"alert\" \| \"drop\"\n";
+		# Suricata is in IPS mode, which means that the rule actions have to be changed
+		# from 'alert' to 'drop', however not all rules should be changed.  Some rules
+		# exist purely to set a flowbit which is used to convey other information, such
+		# as a specific type of file being downloaded, to other rulewhich then check for
+		# malware in that file.  Rules which fall into the first category should stay as
+		# alert since not all flows of that type contain malware.
+
+		if($rulefile eq 'registered' or $rulefile eq 'subscripted' or $rulefile eq 'community') {
+			# These types of rulesfiles contain meta-data which gives the action that should
+			# be used when in IPS mode.  Do the following:
+			#
+			# 1. Disable all rules and set the action to 'drop'
+			# 2. Set the action back to 'alert' if the rule contains 'flowbits:noalert;'
+			#    This should give rules not in the policy a reasonable default if the user
+			#    manually enables them.
+			# 3. Enable rules and set actions according to the meta-data strings.
+
+			my $policy = 'balanced';  # Placeholder to allow policy to be changed.
+
+			print FILE <<END;
+modifysid * "^#?(?:alert|drop)" | "#drop"
+modifysid * "^#drop(.+flowbits:noalert;)" | "#alert\${1}"
+modifysid * "^#(?:alert|drop)(.+policy $policy-ips alert)" | "alert\${1}"
+modifysid * "^#(?:alert|drop)(.+policy $policy-ips drop)" | "drop\${1}"
+END
+		} else {
+			# These rulefiles don't have the metadata, so set rules to 'drop' unless they
+			# contain the string 'flowbits:noalert;'.
+			print FILE <<END;
+modifysid * "^(#?)(?:alert|drop)" | "\${1}drop"
+modifysid * "^(#?)drop(.+flowbits:noalert;)" | "\${1}alert\${2}"
+END
+		}
 	}
 
 	# Close file handle.
diff --git a/config/rootfiles/common/configroot b/config/rootfiles/common/configroot
index a7f27fe55..56b0257bc 100644
--- a/config/rootfiles/common/configroot
+++ b/config/rootfiles/common/configroot
@@ -3,6 +3,7 @@ usr/sbin/convert-outgoingfw
 usr/sbin/convert-portfw
 usr/sbin/convert-snort
 usr/sbin/convert-xtaccess
+usr/sbin/convert-ids-modifysids-file
 usr/sbin/firewall-policy
 #var/ipfire
 var/ipfire/addon-lang
diff --git a/config/rootfiles/core/133/update.sh b/config/rootfiles/core/133/update.sh
index 9d708f092..a05ad0741 100644
--- a/config/rootfiles/core/133/update.sh
+++ b/config/rootfiles/core/133/update.sh
@@ -62,6 +62,9 @@ telinit u
 # Regenerate /etc/ipsec.conf
 sudo -u nobody /srv/web/ipfire/cgi-bin/vpnmain.cgi
 
+# Modify suricata modify-sids file
+/usr/sbin/convert-ids-modifysids-file
+
 # Start services
 /usr/local/bin/ipsecctrl S
 /etc/init.d/suricata restart
diff --git a/config/suricata/convert-ids-modifysids-file b/config/suricata/convert-ids-modifysids-file
new file mode 100644
index 000000000..8b70aa0fc
--- /dev/null
+++ b/config/suricata/convert-ids-modifysids-file
@@ -0,0 +1,84 @@
+#!/usr/bin/perl
+###############################################################################
+#                                                                             #
+# IPFire.org - A linux based firewall                                         #
+# Copyright (C) 2019 IPFire Development Team <info(a)ipfire.org>                #
+#                                                                             #
+# This program is free software: you can redistribute it and/or modify        #
+# it under the terms of the GNU General Public License as published by        #
+# the Free Software Foundation, either version 3 of the License, or           #
+# (at your option) any later version.                                         #
+#                                                                             #
+# This program is distributed in the hope that it will be useful,             #
+# but WITHOUT ANY WARRANTY; without even the implied warranty of              #
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the               #
+# GNU General Public License for more details.                                #
+#                                                                             #
+# You should have received a copy of the GNU General Public License           #
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.       #
+#                                                                             #
+###############################################################################
+
+use strict;
+
+require '/var/ipfire/general-functions.pl';
+require "${General::swroot}/ids-functions.pl";
+
+# Hash which contains the IDS (suricata) settings.
+my %idssettings;
+
+# Hash which contains the RULES settings.
+my %rulessettings;
+
+#
+## Step 1: Read IDS and rules settings.
+#
+
+exit unless(-f $IDS::ids_settings_file and -f $IDS::rules_settings_file);
+
+# Read IDS settings.
+&General::readhash("$IDS::ids_settings_file", \%idssettings);
+
+# Read rules settings.
+&General::readhash("$IDS::rules_settings_file", \%rulessettings);
+
+#
+## Step 2: Generate and write the file to modify the ruleset.
+#
+
+my $IDS_action = "drop";
+
+# Check if the traffic only should be monitored.
+if ($idssettings{"MONITOR_TRAFFIC_ONLY"} eq "on") {
+	# Switch IDS action to alert only.
+	$IDS_action = "alert";
+}
+
+# Call subfunction and pass the desired IDS action.
+&IDS::write_modify_sids_file($IDS_action, $rulessettings{RULES});
+
+# Set correct ownership.
+&IDS::set_ownership("$IDS::modify_sids_file");
+
+#
+## Step 3: Call oinkmaster to extract and setup the rules structures.
+#
+
+# Check if a rulestarball is present.
+if (-f $IDS::rulestarball) {
+	# Launch oinkmaster by calling the subfunction.
+	&IDS::oinkmaster();
+
+	# Set correct ownership for the rulesdir and files.
+	&IDS::set_ownership("$IDS::rulespath");
+}
+
+#
+## Step 4: Start the IDS if enabled.
+#
+
+# Check if the IDS should be started.
+if($idssettings{"ENABLE_IDS"} eq "on") {
+	# Call suricatactrl and reload the rules.
+	&IDS::call_suricatactrl("reload");
+}
diff --git a/html/cgi-bin/ids.cgi b/html/cgi-bin/ids.cgi
index 00db6a0c3..1791e9beb 100644
--- a/html/cgi-bin/ids.cgi
+++ b/html/cgi-bin/ids.cgi
@@ -359,7 +359,7 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 				$errormessage = "$Lang::tr{'could not download latest updates'} - $Lang::tr{'system is offline'}";
 			}
 
-			# Check if enought free disk space is availabe.
+			# Check if enough free disk space is availabe.
 			if(&IDS::checkdiskspace()) {
 				$errormessage = "$Lang::tr{'not enough disk space'}";
 			}
@@ -370,6 +370,22 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 				# a new ruleset.
 				&working_notice("$Lang::tr{'ids working'}");
 
+				&General::readhash("$IDS::ids_settings_file", \%idssettings);
+
+				# Temporary variable to set the ruleaction.
+				# Default is "drop" to use suricata as IPS.
+				my $ruleaction="drop";
+
+				# Check if the traffic only should be monitored.
+				if($idssettings{'MONITOR_TRAFFIC_ONLY'} eq 'on') {
+					# Switch the ruleaction to "alert".
+					# Suricata acts as an IDS only.
+					$ruleaction="alert";
+				}
+
+				# Write the modify sid's file and pass the taken ruleaction.
+				&IDS::write_modify_sids_file($ruleaction, $cgiparams{'RULES'});
+
 				# Call subfunction to download the ruleset.
 				if(&IDS::downloadruleset()) {
 					$errormessage = $Lang::tr{'could not download latest updates'};
@@ -609,8 +625,10 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 		$ruleaction="alert";
 	}
 
+	&General::readhash("$IDS::rules_settings_file", \%rulessettings);
+
 	# Write the modify sid's file and pass the taken ruleaction.
-	&IDS::write_modify_sids_file($ruleaction);
+	&IDS::write_modify_sids_file($ruleaction, $rulessettings{'RULES'});
 
 	# Check if "MONITOR_TRAFFIC_ONLY" has been changed.
 	if($cgiparams{'MONITOR_TRAFFIC_ONLY'} ne $oldidssettings{'MONITOR_TRAFFIC_ONLY'}) {
diff --git a/lfs/configroot b/lfs/configroot
index d4eb545f0..227d09239 100644
--- a/lfs/configroot
+++ b/lfs/configroot
@@ -135,6 +135,7 @@ $(TARGET) :
 
 	# Install snort to suricata converter.
 	cp $(DIR_SRC)/config/suricata/convert-snort	/usr/sbin/convert-snort
+	cp $(DIR_SRC)/config/suricata/convert-ids-modifysids-file   /usr/sbin/convert-ids-modifysids-file
 
 	# Add conntrack helper default settings
 	for proto in FTP H323 IRC SIP TFTP; do \
-- 
2.20.1