[PATCH] firewalllog.dat: Proper display protocol names.

[PATCH] firewalllog.dat: Proper display protocol names.

[Stefan Schantl]

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

In some cases iptables logs the protocol number instead of the name.
When accessing the logs via the WUI, this number has been displayed as used
protocol, which is very hard to read and understand.

This commit adds a new function to the general-functions.pl, which
generates a hash to translate the protocol number into the protocol
name.

Fixes #11282.

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 config/cfgroot/general-functions.pl   | 36 +++++++++++++++++++++++++++
 html/cgi-bin/logs.cgi/firewalllog.dat |  8 ++++++
 2 files changed, 44 insertions(+)

diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
index 550afcf82..529585863 100644
--- a/config/cfgroot/general-functions.pl
+++ b/config/cfgroot/general-functions.pl
@@ -1363,6 +1363,42 @@ sub formatBytes {
 	return sprintf("%.2f %s", $bytes, $unit);
 }
 
+# Function to collect and generate a hash for translating protocol numbers into
+# their names.
+sub generateProtoTransHash () {
+	# File which contains the protocol definitions.
+	my $protocols_file = "/etc/protocols";
+
+	my %protocols = ();
+
+	# Open protocols file.
+	open(FILE, "$protocols_file") or die "Could not open $protocols_file. $!\n";
+
+	# Loop through the file.
+	while (my $line = <FILE>) {
+		# Skip comments.
+		next if ($line =~ /\#/);
+
+		# Skip blank  lines.
+		next if ($line =~ /^\s*$/);
+
+		# Remove any newlines.
+		chomp($line);
+
+		# Split line content.
+		my ($protocol_lc, $number, $protocol_uc, $comment) = split(' ', $line);
+
+		# Add proto details to the hash of protocols.
+		$protocols{$number} = $protocol_uc;
+	}
+
+	# Close file handle.
+	close(FILE);
+
+	# Return the hash.
+	return %protocols;
+}
+
 # Cloud Stuff
 
 sub running_in_cloud() {
diff --git a/html/cgi-bin/logs.cgi/firewalllog.dat b/html/cgi-bin/logs.cgi/firewalllog.dat
index e326d65c0..73596d8cd 100644
--- a/html/cgi-bin/logs.cgi/firewalllog.dat
+++ b/html/cgi-bin/logs.cgi/firewalllog.dat
@@ -325,6 +325,8 @@ print <<END
 END
 ;
 
+# Generate hash to translate protocol numbers into protocol names.
+my %protocols = &General::generateProtoTransHash();
 
 $lines = 0;
 foreach $_ (@log)
@@ -354,6 +356,12 @@ foreach $_ (@log)
 	# Get the country code.
 	my $ccode = &Location::Functions::lookup_country_code($srcaddr);
 
+	# Lookup if the grabbed protocol is part of the protocols hash.
+	if (exists ($protocols{$proto})) {
+		# Translate protocol number into protocol name.
+		$proto = $protocols{$proto};
+	}
+
 	my $servi = uc(getservbyport($srcport, lc($proto)));
 	if ($servi ne '' && $srcport < 1024) {
 		$srcport = "$srcport($servi)"; 
-- 
2.30.2

Re: [PATCH] firewalllog.dat: Proper display protocol names.

[Michael Tremer]

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

Thanks!

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

> On 13 Jul 2021, at 17:58, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> In some cases iptables logs the protocol number instead of the name.
> When accessing the logs via the WUI, this number has been displayed as used
> protocol, which is very hard to read and understand.
> 
> This commit adds a new function to the general-functions.pl, which
> generates a hash to translate the protocol number into the protocol
> name.
> 
> Fixes #11282.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/cfgroot/general-functions.pl   | 36 +++++++++++++++++++++++++++
> html/cgi-bin/logs.cgi/firewalllog.dat |  8 ++++++
> 2 files changed, 44 insertions(+)
> 
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
> index 550afcf82..529585863 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -1363,6 +1363,42 @@ sub formatBytes {
> 	return sprintf("%.2f %s", $bytes, $unit);
> }
> 
> +# Function to collect and generate a hash for translating protocol numbers into
> +# their names.
> +sub generateProtoTransHash () {
> +	# File which contains the protocol definitions.
> +	my $protocols_file = "/etc/protocols";
> +
> +	my %protocols = ();
> +
> +	# Open protocols file.
> +	open(FILE, "$protocols_file") or die "Could not open $protocols_file. $!\n";
> +
> +	# Loop through the file.
> +	while (my $line = <FILE>) {
> +		# Skip comments.
> +		next if ($line =~ /\#/);
> +
> +		# Skip blank  lines.
> +		next if ($line =~ /^\s*$/);
> +
> +		# Remove any newlines.
> +		chomp($line);
> +
> +		# Split line content.
> +		my ($protocol_lc, $number, $protocol_uc, $comment) = split(' ', $line);
> +
> +		# Add proto details to the hash of protocols.
> +		$protocols{$number} = $protocol_uc;
> +	}
> +
> +	# Close file handle.
> +	close(FILE);
> +
> +	# Return the hash.
> +	return %protocols;
> +}
> +
> # Cloud Stuff
> 
> sub running_in_cloud() {
> diff --git a/html/cgi-bin/logs.cgi/firewalllog.dat b/html/cgi-bin/logs.cgi/firewalllog.dat
> index e326d65c0..73596d8cd 100644
> --- a/html/cgi-bin/logs.cgi/firewalllog.dat
> +++ b/html/cgi-bin/logs.cgi/firewalllog.dat
> @@ -325,6 +325,8 @@ print <<END
> END
> ;
> 
> +# Generate hash to translate protocol numbers into protocol names.
> +my %protocols = &General::generateProtoTransHash();
> 
> $lines = 0;
> foreach $_ (@log)
> @@ -354,6 +356,12 @@ foreach $_ (@log)
> 	# Get the country code.
> 	my $ccode = &Location::Functions::lookup_country_code($srcaddr);
> 
> +	# Lookup if the grabbed protocol is part of the protocols hash.
> +	if (exists ($protocols{$proto})) {
> +		# Translate protocol number into protocol name.
> +		$proto = $protocols{$proto};
> +	}
> +
> 	my $servi = uc(getservbyport($srcport, lc($proto)));
> 	if ($servi ne '' && $srcport < 1024) {
> 		$srcport = "$srcport($servi)"; 
> -- 
> 2.30.2
> 

Re: [PATCH] firewalllog.dat: Proper display protocol names.

[Bernhard Bitsch]

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

A little correction, see below.

Reviewed-by: Bernhard Bitsch <bbitsch(a)ipfire.org>

Am 13.07.2021 um 18:58 schrieb Stefan Schantl:
> In some cases iptables logs the protocol number instead of the name.
> When accessing the logs via the WUI, this number has been displayed as used
> protocol, which is very hard to read and understand.
> 
> This commit adds a new function to the general-functions.pl, which
> generates a hash to translate the protocol number into the protocol
> name.
> 
> Fixes #11282.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
>   config/cfgroot/general-functions.pl   | 36 +++++++++++++++++++++++++++
>   html/cgi-bin/logs.cgi/firewalllog.dat |  8 ++++++
>   2 files changed, 44 insertions(+)
> 
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
> index 550afcf82..529585863 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -1363,6 +1363,42 @@ sub formatBytes {
>   	return sprintf("%.2f %s", $bytes, $unit);
>   }
>   
> +# Function to collect and generate a hash for translating protocol numbers into
> +# their names.
> +sub generateProtoTransHash () {
> +	# File which contains the protocol definitions.
> +	my $protocols_file = "/etc/protocols";
> +
> +	my %protocols = ();
> +
> +	# Open protocols file.
> +	open(FILE, "$protocols_file") or die "Could not open $protocols_file. $!\n";
> +
> +	# Loop through the file.
> +	while (my $line = <FILE>) {
> +		# Skip comments.
> +		next if ($line =~ /\#/);

This should read (all lines contain comments):
+		next if ($line =~ /^#/);

> +
> +		# Skip blank  lines.
> +		next if ($line =~ /^\s*$/);
> +
> +		# Remove any newlines.
> +		chomp($line);
> +
> +		# Split line content.
> +		my ($protocol_lc, $number, $protocol_uc, $comment) = split(' ', $line);
> +
> +		# Add proto details to the hash of protocols.
> +		$protocols{$number} = $protocol_uc;
> +	}
> +
> +	# Close file handle.
> +	close(FILE);
> +
> +	# Return the hash.
> +	return %protocols;
> +}
> +
>   # Cloud Stuff
>   
>   sub running_in_cloud() {
> diff --git a/html/cgi-bin/logs.cgi/firewalllog.dat b/html/cgi-bin/logs.cgi/firewalllog.dat
> index e326d65c0..73596d8cd 100644
> --- a/html/cgi-bin/logs.cgi/firewalllog.dat
> +++ b/html/cgi-bin/logs.cgi/firewalllog.dat
> @@ -325,6 +325,8 @@ print <<END
>   END
>   ;
>   
> +# Generate hash to translate protocol numbers into protocol names.
> +my %protocols = &General::generateProtoTransHash();
>   
>   $lines = 0;
>   foreach $_ (@log)
> @@ -354,6 +356,12 @@ foreach $_ (@log)
>   	# Get the country code.
>   	my $ccode = &Location::Functions::lookup_country_code($srcaddr);
>   
> +	# Lookup if the grabbed protocol is part of the protocols hash.
> +	if (exists ($protocols{$proto})) {
> +		# Translate protocol number into protocol name.
> +		$proto = $protocols{$proto};
> +	}
> +
>   	my $servi = uc(getservbyport($srcport, lc($proto)));
>   	if ($servi ne '' && $srcport < 1024) {
>   		$srcport = "$srcport($servi)";
> 

Re: [PATCH] firewalllog.dat: Proper display protocol names.

[Stefan Schantl]

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

Hello Bernhard,

good find - thanks a lot.

I'll send a second patch to the list.

- Stefan
> A little correction, see below.
> 
> Reviewed-by: Bernhard Bitsch <bbitsch(a)ipfire.org>
> 
> Am 13.07.2021 um 18:58 schrieb Stefan Schantl:
> > In some cases iptables logs the protocol number instead of the
> > name.
> > When accessing the logs via the WUI, this number has been displayed
> > as used
> > protocol, which is very hard to read and understand.
> > 
> > This commit adds a new function to the general-functions.pl, which
> > generates a hash to translate the protocol number into the protocol
> > name.
> > 
> > Fixes #11282.
> > 
> > Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> > ---
> >   config/cfgroot/general-functions.pl   | 36
> > +++++++++++++++++++++++++++
> >   html/cgi-bin/logs.cgi/firewalllog.dat |  8 ++++++
> >   2 files changed, 44 insertions(+)
> > 
> > diff --git a/config/cfgroot/general-functions.pl
> > b/config/cfgroot/general-functions.pl
> > index 550afcf82..529585863 100644
> > --- a/config/cfgroot/general-functions.pl
> > +++ b/config/cfgroot/general-functions.pl
> > @@ -1363,6 +1363,42 @@ sub formatBytes {
> >         return sprintf("%.2f %s", $bytes, $unit);
> >   }
> >   
> > +# Function to collect and generate a hash for translating protocol
> > numbers into
> > +# their names.
> > +sub generateProtoTransHash () {
> > +       # File which contains the protocol definitions.
> > +       my $protocols_file = "/etc/protocols";
> > +
> > +       my %protocols = ();
> > +
> > +       # Open protocols file.
> > +       open(FILE, "$protocols_file") or die "Could not open
> > $protocols_file. $!\n";
> > +
> > +       # Loop through the file.
> > +       while (my $line = <FILE>) {
> > +               # Skip comments.
> > +               next if ($line =~ /\#/);
> 
> This should read (all lines contain comments):
> +               next if ($line =~ /^#/);
> 
> > +
> > +               # Skip blank  lines.
> > +               next if ($line =~ /^\s*$/);
> > +
> > +               # Remove any newlines.
> > +               chomp($line);
> > +
> > +               # Split line content.
> > +               my ($protocol_lc, $number, $protocol_uc, $comment)
> > = split(' ', $line);
> > +
> > +               # Add proto details to the hash of protocols.
> > +               $protocols{$number} = $protocol_uc;
> > +       }
> > +
> > +       # Close file handle.
> > +       close(FILE);
> > +
> > +       # Return the hash.
> > +       return %protocols;
> > +}
> > +
> >   # Cloud Stuff
> >   
> >   sub running_in_cloud() {
> > diff --git a/html/cgi-bin/logs.cgi/firewalllog.dat b/html/cgi-
> > bin/logs.cgi/firewalllog.dat
> > index e326d65c0..73596d8cd 100644
> > --- a/html/cgi-bin/logs.cgi/firewalllog.dat
> > +++ b/html/cgi-bin/logs.cgi/firewalllog.dat
> > @@ -325,6 +325,8 @@ print <<END
> >   END
> >   ;
> >   
> > +# Generate hash to translate protocol numbers into protocol names.
> > +my %protocols = &General::generateProtoTransHash();
> >   
> >   $lines = 0;
> >   foreach $_ (@log)
> > @@ -354,6 +356,12 @@ foreach $_ (@log)
> >         # Get the country code.
> >         my $ccode =
> > &Location::Functions::lookup_country_code($srcaddr);
> >   
> > +       # Lookup if the grabbed protocol is part of the protocols
> > hash.
> > +       if (exists ($protocols{$proto})) {
> > +               # Translate protocol number into protocol name.
> > +               $proto = $protocols{$proto};
> > +       }
> > +
> >         my $servi = uc(getservbyport($srcport, lc($proto)));
> >         if ($servi ne '' && $srcport < 1024) {
> >                 $srcport = "$srcport($servi)";
> > 

Re: [PATCH] firewalllog.dat: Proper display protocol names.

[Bernhard Bitsch]

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

Hello Stefan,

nevermind, that's the purpose of code review. ;)

- Bernhard

Am 15.07.2021 um 10:17 schrieb Stefan Schantl:
> Hello Bernhard,
> 
> good find - thanks a lot.
> 
> I'll send a second patch to the list.
> 
> - Stefan
>> A little correction, see below.
>>
>> Reviewed-by: Bernhard Bitsch <bbitsch(a)ipfire.org>
>>
>> Am 13.07.2021 um 18:58 schrieb Stefan Schantl:
>>> In some cases iptables logs the protocol number instead of the
>>> name.
>>> When accessing the logs via the WUI, this number has been displayed
>>> as used
>>> protocol, which is very hard to read and understand.
>>>
>>> This commit adds a new function to the general-functions.pl, which
>>> generates a hash to translate the protocol number into the protocol
>>> name.
>>>
>>> Fixes #11282.
>>>
>>> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
>>> ---
>>>    config/cfgroot/general-functions.pl   | 36
>>> +++++++++++++++++++++++++++
>>>    html/cgi-bin/logs.cgi/firewalllog.dat |  8 ++++++
>>>    2 files changed, 44 insertions(+)
>>>
>>> diff --git a/config/cfgroot/general-functions.pl
>>> b/config/cfgroot/general-functions.pl
>>> index 550afcf82..529585863 100644
>>> --- a/config/cfgroot/general-functions.pl
>>> +++ b/config/cfgroot/general-functions.pl
>>> @@ -1363,6 +1363,42 @@ sub formatBytes {
>>>          return sprintf("%.2f %s", $bytes, $unit);
>>>    }
>>>    
>>> +# Function to collect and generate a hash for translating protocol
>>> numbers into
>>> +# their names.
>>> +sub generateProtoTransHash () {
>>> +       # File which contains the protocol definitions.
>>> +       my $protocols_file = "/etc/protocols";
>>> +
>>> +       my %protocols = ();
>>> +
>>> +       # Open protocols file.
>>> +       open(FILE, "$protocols_file") or die "Could not open
>>> $protocols_file. $!\n";
>>> +
>>> +       # Loop through the file.
>>> +       while (my $line = <FILE>) {
>>> +               # Skip comments.
>>> +               next if ($line =~ /\#/);
>>
>> This should read (all lines contain comments):
>> +               next if ($line =~ /^#/);
>>
>>> +
>>> +               # Skip blank  lines.
>>> +               next if ($line =~ /^\s*$/);
>>> +
>>> +               # Remove any newlines.
>>> +               chomp($line);
>>> +
>>> +               # Split line content.
>>> +               my ($protocol_lc, $number, $protocol_uc, $comment)
>>> = split(' ', $line);
>>> +
>>> +               # Add proto details to the hash of protocols.
>>> +               $protocols{$number} = $protocol_uc;
>>> +       }
>>> +
>>> +       # Close file handle.
>>> +       close(FILE);
>>> +
>>> +       # Return the hash.
>>> +       return %protocols;
>>> +}
>>> +
>>>    # Cloud Stuff
>>>    
>>>    sub running_in_cloud() {
>>> diff --git a/html/cgi-bin/logs.cgi/firewalllog.dat b/html/cgi-
>>> bin/logs.cgi/firewalllog.dat
>>> index e326d65c0..73596d8cd 100644
>>> --- a/html/cgi-bin/logs.cgi/firewalllog.dat
>>> +++ b/html/cgi-bin/logs.cgi/firewalllog.dat
>>> @@ -325,6 +325,8 @@ print <<END
>>>    END
>>>    ;
>>>    
>>> +# Generate hash to translate protocol numbers into protocol names.
>>> +my %protocols = &General::generateProtoTransHash();
>>>    
>>>    $lines = 0;
>>>    foreach $_ (@log)
>>> @@ -354,6 +356,12 @@ foreach $_ (@log)
>>>          # Get the country code.
>>>          my $ccode =
>>> &Location::Functions::lookup_country_code($srcaddr);
>>>    
>>> +       # Lookup if the grabbed protocol is part of the protocols
>>> hash.
>>> +       if (exists ($protocols{$proto})) {
>>> +               # Translate protocol number into protocol name.
>>> +               $proto = $protocols{$proto};
>>> +       }
>>> +
>>>          my $servi = uc(getservbyport($srcport, lc($proto)));
>>>          if ($servi ne '' && $srcport < 1024) {
>>>                  $srcport = "$srcport($servi)";
>>>
>