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@ipfire.org Cc: Stefan Schantl stefan.schantl@ipfire.org Signed-off-by: Peter Müller peter.mueller@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@ipfire.org. # +# Copyright (C) 2018-2019 IPFire Team info@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"; - } + } }
#
Looks good for me.
Reviewed-by: Stefan Schantl stefan.schantl@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@ipfire.org Cc: Stefan Schantl stefan.schantl@ipfire.org Signed-off-by: Peter Müller peter.mueller@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@ipfire.org. # +# Copyright (C) 2018-2019 IPFire Team info@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";
- }
- }
}
#