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

[Bernhard Bitsch <bbitsch@ipfire.org>]

[-- 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)";
>>>
>