Re: [PATCH] ids.cgi: Restart suricata if necessary when altering the ruleset.

[Michael Tremer <michael.tremer@ipfire.org>]

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

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(a)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(a)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
>