[PATCH] update-ids-ruleset: Always drop the lock file if it has been created during runtime.

[PATCH] update-ids-ruleset: Always drop the lock file if it has been created during runtime.

[Stefan Schantl]

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

In some situations or if an error happened, the lock file could be
keep on the system. In such a case the IDS page would be locked forever
until user interaction or reboot of the system.

Now the script checks if it has created such a lock and release it when
the script exists.

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 src/scripts/update-ids-ruleset | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/scripts/update-ids-ruleset b/src/scripts/update-ids-ruleset
index 10a270907..c2970d20b 100644
--- a/src/scripts/update-ids-ruleset
+++ b/src/scripts/update-ids-ruleset
@@ -26,6 +26,9 @@ require '/var/ipfire/general-functions.pl';
 require "${General::swroot}/ids-functions.pl";
 require "${General::swroot}/lang.pl";
 
+# Variable to store if the process has written a lockfile.
+my $locked;
+
 # Hash to store the configured providers.
 my %providers = ();
 
@@ -77,6 +80,9 @@ if(&IDS::checkdiskspace()) {
 # Lock the IDS page.
 &IDS::lock_ids_page();
 
+# The script has requested a lock, so set locket to "1".
+$locked = "1";
+
 # Grab the configured providers.
 &General::readhasharray("$IDS::providers_settings_file", \%providers);
 
@@ -114,13 +120,20 @@ foreach my $id (keys %providers) {
 # Set correct ownership for the rulesdir and files.
 &IDS::set_ownership("$IDS::rulespath");
 
-# Unlock the IDS page.
-&IDS::unlock_ids_page();
-
 # Check if the IDS is running.
 if(&IDS::ids_is_running()) {
 	# Call suricatactrl to perform a reload.
 	&IDS::call_suricatactrl("reload");
 }
 
+# Custom END declaration to release a IDS page lock
+# when the script has created one.
+END {
+	# Check if a lock has been requested.
+	if ($locked) {
+		# Unlock the IDS page.
+		&IDS::unlock_ids_page();
+	}
+}
+
 1;
-- 
2.30.2

Re: [PATCH] update-ids-ruleset: Always drop the lock file if it has been created during runtime.

[Michael Tremer]

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

This looks good :)

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 3 Mar 2022, at 04:49, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> In some situations or if an error happened, the lock file could be
> keep on the system. In such a case the IDS page would be locked forever
> until user interaction or reboot of the system.
> 
> Now the script checks if it has created such a lock and release it when
> the script exists.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> src/scripts/update-ids-ruleset | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/src/scripts/update-ids-ruleset b/src/scripts/update-ids-ruleset
> index 10a270907..c2970d20b 100644
> --- a/src/scripts/update-ids-ruleset
> +++ b/src/scripts/update-ids-ruleset
> @@ -26,6 +26,9 @@ require '/var/ipfire/general-functions.pl';
> require "${General::swroot}/ids-functions.pl";
> require "${General::swroot}/lang.pl";
> 
> +# Variable to store if the process has written a lockfile.
> +my $locked;
> +
> # Hash to store the configured providers.
> my %providers = ();
> 
> @@ -77,6 +80,9 @@ if(&IDS::checkdiskspace()) {
> # Lock the IDS page.
> &IDS::lock_ids_page();
> 
> +# The script has requested a lock, so set locket to "1".
> +$locked = "1";
> +
> # Grab the configured providers.
> &General::readhasharray("$IDS::providers_settings_file", \%providers);
> 
> @@ -114,13 +120,20 @@ foreach my $id (keys %providers) {
> # Set correct ownership for the rulesdir and files.
> &IDS::set_ownership("$IDS::rulespath");
> 
> -# Unlock the IDS page.
> -&IDS::unlock_ids_page();
> -
> # Check if the IDS is running.
> if(&IDS::ids_is_running()) {
> 	# Call suricatactrl to perform a reload.
> 	&IDS::call_suricatactrl("reload");
> }
> 
> +# Custom END declaration to release a IDS page lock
> +# when the script has created one.
> +END {
> +	# Check if a lock has been requested.
> +	if ($locked) {
> +		# Unlock the IDS page.
> +		&IDS::unlock_ids_page();
> +	}
> +}
> +
> 1;
> -- 
> 2.30.2
> 

Re: [PATCH] update-ids-ruleset: Always drop the lock file if it has been created during runtime.

[Peter Müller]

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

Acked-by: Peter Müller <peter.mueller(a)ipfire.org>

> In some situations or if an error happened, the lock file could be
> keep on the system. In such a case the IDS page would be locked forever
> until user interaction or reboot of the system.
> 
> Now the script checks if it has created such a lock and release it when
> the script exists.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
>  src/scripts/update-ids-ruleset | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/src/scripts/update-ids-ruleset b/src/scripts/update-ids-ruleset
> index 10a270907..c2970d20b 100644
> --- a/src/scripts/update-ids-ruleset
> +++ b/src/scripts/update-ids-ruleset
> @@ -26,6 +26,9 @@ require '/var/ipfire/general-functions.pl';
>  require "${General::swroot}/ids-functions.pl";
>  require "${General::swroot}/lang.pl";
>  
> +# Variable to store if the process has written a lockfile.
> +my $locked;
> +
>  # Hash to store the configured providers.
>  my %providers = ();
>  
> @@ -77,6 +80,9 @@ if(&IDS::checkdiskspace()) {
>  # Lock the IDS page.
>  &IDS::lock_ids_page();
>  
> +# The script has requested a lock, so set locket to "1".
> +$locked = "1";
> +
>  # Grab the configured providers.
>  &General::readhasharray("$IDS::providers_settings_file", \%providers);
>  
> @@ -114,13 +120,20 @@ foreach my $id (keys %providers) {
>  # Set correct ownership for the rulesdir and files.
>  &IDS::set_ownership("$IDS::rulespath");
>  
> -# Unlock the IDS page.
> -&IDS::unlock_ids_page();
> -
>  # Check if the IDS is running.
>  if(&IDS::ids_is_running()) {
>  	# Call suricatactrl to perform a reload.
>  	&IDS::call_suricatactrl("reload");
>  }
>  
> +# Custom END declaration to release a IDS page lock
> +# when the script has created one.
> +END {
> +	# Check if a lock has been requested.
> +	if ($locked) {
> +		# Unlock the IDS page.
> +		&IDS::unlock_ids_page();
> +	}
> +}
> +
>  1;