From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject:
 Re: [PATCH] ids.cgi: Restart suricata if necessary when altering the ruleset.
Date: Wed, 01 Apr 2020 11:53:36 +0100
Message-ID: <A03C60CF-054E-46FF-9515-9200B67B7E4B@ipfire.org>
In-Reply-To: <20200401083240.2704-1-stefan.schantl@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============5882105720642459029=="
List-Id: <development.lists.ipfire.org>

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

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:
>=20
> 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.
>=20
> 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.
>=20
> Fixes #12340.
>=20
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> html/cgi-bin/ids.cgi | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>=20
> 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;
>=20
> +	# 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);
>=20
> +			# Check if the state of the rulefile has been changed.
> +			unless ($cgiparams{$rulefile} eq $idsrules{$rulefile}{'Rulefile'}{'Stat=
e'}) {
> +				# A restart of suricata is required to apply the changes of the used r=
ulefiles.
> +				$suricata_restart_required =3D 1;
> +			}
> +
> 			# Drop item from cgiparams hash.
> 			delete $cgiparams{$rulefile};
> 		}
> @@ -513,8 +522,14 @@ if ($cgiparams{'RULESET'} eq $Lang::tr{'save'}) {
>=20
> 	# 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");
> +		}
> 	}
>=20
> 	# Reload page.
> --=20
> 2.26.0
>=20


--===============5882105720642459029==--