ids.cgi: Restart suricata if necessary when altering the ruleset.

Message ID 20200401083240.2704-1-stefan.schantl@ipfire.org
State Accepted
Commit af8e5145fa969f0c99c9650c16e05bc71d7297b1
Headers
Series ids.cgi: Restart suricata if necessary when altering the ruleset. |

Commit Message

Stefan Schantl April 1, 2020, 8:32 a.m. UTC
  Suricata does support re-reading it's configuration files and therfore
we need to restart it, if one or more ruleset files should be loaded or
not loaded anymore.

If simple some rules inside the same files are activated or deactivated
we are still fine to call the reload method to send suricata the signal
to reload its ruleset.

Fixes #12340.

Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
---
 html/cgi-bin/ids.cgi | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
  

Comments

Michael Tremer April 1, 2020, 10:53 a.m. UTC | #1
Thanks for working on this patch.

I think it solves the problem. However, I do not think that we should restart suricata here.

Can we file a ticket upstream and see if this is the intended behaviour and, if so, why?

It would be great if we could revert this patch sooner or later.

Best,
-Michael

> On 1 Apr 2020, at 09:32, Stefan Schantl <stefan.schantl@ipfire.org> wrote:
> 
> Suricata does support re-reading it's configuration files and therfore
> we need to restart it, if one or more ruleset files should be loaded or
> not loaded anymore.
> 
> If simple some rules inside the same files are activated or deactivated
> we are still fine to call the reload method to send suricata the signal
> to reload its ruleset.
> 
> Fixes #12340.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
> ---
> html/cgi-bin/ids.cgi | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/html/cgi-bin/ids.cgi b/html/cgi-bin/ids.cgi
> index 2a8a7cb26..c3e5eefdb 100644
> --- a/html/cgi-bin/ids.cgi
> +++ b/html/cgi-bin/ids.cgi
> @@ -412,6 +412,9 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
> 	# Hash to store the user-enabled and disabled sids.
> 	my %enabled_disabled_sids;
> 
> +	# Store if a restart of suricata is required.
> +	my $suricata_restart_required;
> +
> 	# Loop through the hash of idsrules.
> 	foreach my $rulefile(keys %idsrules) {
> 		# Check if the rulefile is enabled.
> @@ -419,6 +422,12 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
> 			# Add rulefile to the array of enabled rulefiles.
> 			push(@enabled_rulefiles, $rulefile);
> 
> +			# Check if the state of the rulefile has been changed.
> +			unless ($cgiparams{$rulefile} eq $idsrules{$rulefile}{'Rulefile'}{'State'}) {
> +				# A restart of suricata is required to apply the changes of the used rulefiles.
> +				$suricata_restart_required = 1;
> +			}
> +
> 			# Drop item from cgiparams hash.
> 			delete $cgiparams{$rulefile};
> 		}
> @@ -513,8 +522,14 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
> 
> 	# Check if the IDS is running.
> 	if(&IDS::ids_is_running()) {
> -		# Call suricatactrl to perform a reload.
> -		&IDS::call_suricatactrl("reload");
> +		# Check if a restart of suricata is required.
> +		if ($suricata_restart_required) {
> +			# Call suricatactrl to perform the restart.
> +			&IDS::call_suricatactrl("restart");
> +		} else {
> +			# Call suricatactrl to perform a reload.
> +			&IDS::call_suricatactrl("reload");
> +		}
> 	}
> 
> 	# Reload page.
> -- 
> 2.26.0
>
  

Patch

diff --git a/html/cgi-bin/ids.cgi b/html/cgi-bin/ids.cgi
index 2a8a7cb26..c3e5eefdb 100644
--- a/html/cgi-bin/ids.cgi
+++ b/html/cgi-bin/ids.cgi
@@ -412,6 +412,9 @@  if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 	# Hash to store the user-enabled and disabled sids.
 	my %enabled_disabled_sids;
 
+	# Store if a restart of suricata is required.
+	my $suricata_restart_required;
+
 	# Loop through the hash of idsrules.
 	foreach my $rulefile(keys %idsrules) {
 		# Check if the rulefile is enabled.
@@ -419,6 +422,12 @@  if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 			# Add rulefile to the array of enabled rulefiles.
 			push(@enabled_rulefiles, $rulefile);
 
+			# Check if the state of the rulefile has been changed.
+			unless ($cgiparams{$rulefile} eq $idsrules{$rulefile}{'Rulefile'}{'State'}) {
+				# A restart of suricata is required to apply the changes of the used rulefiles.
+				$suricata_restart_required = 1;
+			}
+
 			# Drop item from cgiparams hash.
 			delete $cgiparams{$rulefile};
 		}
@@ -513,8 +522,14 @@  if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
 
 	# Check if the IDS is running.
 	if(&IDS::ids_is_running()) {
-		# Call suricatactrl to perform a reload.
-		&IDS::call_suricatactrl("reload");
+		# Check if a restart of suricata is required.
+		if ($suricata_restart_required) {
+			# Call suricatactrl to perform the restart.
+			&IDS::call_suricatactrl("restart");
+		} else {
+			# Call suricatactrl to perform a reload.
+			&IDS::call_suricatactrl("reload");
+		}
 	}
 
 	# Reload page.