Re: [PATCH] Core Update 139: fix syntax of generated Suricata DNS server file

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

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

Looks good for me.

Reviewed-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> The YAML syntax of /var/ipfire/suricata/suricata-dns-servers.yaml was
> invalid and caused Suricata to crash after upgrading to Core Update
> 139.
> 
> Due to strange NFQUEUE behaviour, this caused IPsec traffic to be
> emitted to the internet directly. While this patch represents a quick
> solution for Core Update 139, another one is needed for changing the
> IPtables chain order to avoid similar information leaks in future.
> 
> Thanks to Michael for his debugging effort.
> 
> Fixes #12260
> Partially fixes #12257
> 
> Cc: Michael Tremer <michael.tremer(a)ipfire.org>
> Cc: Stefan Schantl <stefan.schantl(a)ipfire.org>
> Signed-off-by: Peter Müller <peter.mueller(a)ipfire.org>
> ---
>  config/cfgroot/ids-functions.pl | 51 +++++++++++++++++++++--------
> ------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/config/cfgroot/ids-functions.pl b/config/cfgroot/ids-
> functions.pl
> index 54d86f70f..89ad90c2e 100644
> --- a/config/cfgroot/ids-functions.pl
> +++ b/config/cfgroot/ids-functions.pl
> @@ -17,7 +17,7 @@
>  # along with IPFire; if not, write to the Free
> Software                    #
>  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-
> 1307 USA #
>  #                                                                   
>        #
> -# Copyright (C) 2018 IPFire Team <info(a)ipfire.org>.                 
>        #
> +# Copyright (C) 2018-2019 IPFire Team <info(a)ipfire.org>             
>        #
>  #                                                                   
>        #
>  ####################################################################
> ########
>  
> @@ -706,7 +706,7 @@ sub generate_dns_servers_file() {
>  	open (FILE, "${General::swroot}/red/dns") or die "Could not
> read DNS configuration from ${General::swroot}/red/dns. $!\n";
>  
>  	# Read-in whole file content and store it in a temporary array.
> -	my @file_content = <FILE>;
> +	my @file_content = split(' ', <FILE>);
>  
>  	# Close file handle.
>  	close(FILE);
> @@ -714,31 +714,32 @@ sub generate_dns_servers_file() {
>  	# Format dns servers declaration.
>  	my $line = "\"\[";
>  
> -	# Loop through the array which contains the file content.
> -	foreach my $server (@file_content) {
> -		# Remove newlines.
> -		chomp($server);
> -
> -		# Check if the current DNS configuration is using the
> local recursor mode.
> -		if ($server eq "local recursor") {
> -			# The responsible DNS servers on red are
> directly used, and because we are not able
> -			# to specify each single DNS server address
> here, we currently have to thread each
> -			# address which is not part of the HOME_NET as
> possible DNS server.
> -			$line = "$line" . "!\$HOME_NET";
> -		} else {
> +	# Check if the current DNS configuration is using the local
> recursor mode.
> +	if ($file_content[0] eq "local" && $file_content[1] eq
> "recursor") {
> +		# The responsible DNS servers on red are directly used,
> and because we are not able
> +		# to specify each single DNS server address here, we
> currently have to thread each
> +		# address which is not part of the HOME_NET as possible
> DNS server.
> +		$line = "$line" . "!\$HOME_NET";
> +
> +	} else {
> +		# Loop through the array which contains the file
> content.
> +		foreach my $server (@file_content) {
> +			# Remove newlines.
> +			chomp($server);
> +
>  			# Add the DNS server to the line.
>  			$line = "$line" . "$server";
> +
> +			# Check if the current DNS server was the last
> in the array.
> +			if ($server ne $file_content[-1]) {
> +				# Add "," for the next DNS server.
> +				$line = "$line" . "\,";
> +			}
>  		}
> +	}
>  
> -                # Check if the current DNS server was the last in
> the array.
> -                if ($server eq $file_content[-1]) {
> -                        # Close the line.
> -                        $line = "$line" . "\]\"";
> -                } else {
> -                        # Add "," for the next DNS server.
> -                        $line = "$line" . "\,";
> -                }
> -        }
> +	# Close the line...
> +	$line = "$line" . "\]\"";
>  
>  	# Open file to store the used DNS server addresses.
>  	open(FILE, ">$dns_servers_file") or die "Could not open
> $dns_servers_file. $!\n";
> @@ -866,7 +867,7 @@ sub get_suricata_version($) {
>  	# Remove newlines.
>          chomp($version_string);
>  
> -	# Grab the version from the version string. 
> +	# Grab the version from the version string.
>  	$version_string =~ /([0-9]+([.][0-9]+)+)/;
>  
>  	# Splitt the version into single chunks.
> @@ -882,7 +883,7 @@ sub get_suricata_version($) {
>  	} else {
>  		# Return the full version string.
>  		return "$major_ver.$minor_ver.$patchlevel";
> -	} 
> +	}
>  }
>  
>  #

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]