Re: [PATCH 6/6] geoip-functions.pl: Re-write code to lookup the iso country code of a given IP-address.

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hey Stefan,

I merged all patches from above. The first one needed some smaller modifications but I did that for you to save some time here.

Read below for a problem in this patch:

> On 10 Jan 2019, at 12:00, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> Drop the usage of the old legacy GeoIP perl module which was not able to handle the
> new GeoLite2 databases.
> 
> Write some code to directly access the databases and extract the required data.
> 
> Usage of the GeoIP2 perl module would provide a lot of more functionality which is not
> used/needed. Unfortunately ir requires at lot of additional perl modules which are
> not available on IPFire and would only be build and shipped for this module. Buildig all
> of them will slow down the entire build process, mess up the system and requires a lot
> more space on disk.
> 
> Fixes #11962.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/cfgroot/geoip-functions.pl | 75 ++++++++++++++++++++++++++++---
> 1 file changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/config/cfgroot/geoip-functions.pl b/config/cfgroot/geoip-functions.pl
> index be50d5e14..e8ce8377f 100644
> --- a/config/cfgroot/geoip-functions.pl
> +++ b/config/cfgroot/geoip-functions.pl
> @@ -23,21 +23,82 @@
> 
> package GeoIP;
> 
> -use Geo::IP::PurePerl;
> +require '/var/ipfire/network-functions.pl';
> +
> use Locale::Codes::Country;
> 
> -my $database;
> +# Path where all the GeoIP related databases are stored.
> +my $geoip_database_dir = "/var/lib/GeoIP";
> +
> +# Database which contains all IPv4 networks.
> +my $address_ipv4_database = "GeoLite2-Country-Blocks-IPv4.csv";
> +
> +# Database wich contains the locations data.
> +my $location_database = "GeoLite2-Country-Locations-en.csv";
> 
> sub lookup($) {
> 	my $address = shift;
> +	my $location_id;
> +	my $country_code;
> +
> +	# Check if the given address is valid.
> +	unless(&Network::check_ip_address($address)) {
> +		return;
> +	}
> +
> +	# Open the address database.
> +	open(ADDRESS, "$geoip_database_dir/$address_ipv4_database") or die "Could not open $geoip_database_dir/$address_ipv4_database. $!\n";
> +
> +	# Loop through the file.
> +	while(my $line = <ADDRESS>) {
> +		# Remove newlines.
> +		chomp($line);
> +
> +		# Split the line content.
> +		my ($network, $geoname_id, $registered_country_geoname_id, $represented_country_geoname_id, $is_anonymous_proxy, $is_satellite_provider) = split(/\,/, $line);
> +
> +		# Check if the given address is part of the current processed network.
> +		if (&Network::ip_address_in_network($address, $network)) {
> +			# Store the geoname_id for this address.
> +			$location_id = $geoname_id;
> +
> +			# Break loop.
> +			last;

You cannot simply break here. You are returning the first match which might not be the smallest subnet in the whole database.

What you need to do is to collect all matches in a hash (subnet and ID) and then search for the smallest subnet which will be the correct match.

I think that that can be implemented in a hand full of lines, so hopefully you can work on this before the end of today. That would be great.

I merged this patch now anyways so that we have a chance to test this all and for that a slightly inaccurate result might not be a big problem.

> +		}
> +	}
> +
> +	# Return nothing if no location_id could be found.
> +	return unless($location_id);
> +
> +	# Close filehandle.
> +	close(ADDRESS);
> +
> +	# Open the location database.
> +	open(LOCATION, "$geoip_database_dir/$location_database") or die "Could not open $geoip_database_dir/$location_database. $!\n";
> 
> -	# Load the database into memory if not already done
> -	if (!$database) {
> -		$database = Geo::IP::PurePerl->new(GEOIP_MEMORY_CACHE);
> +	# Loop through the file.
> +	while(my $line = <LOCATION>) {
> +		# Remove newlines.
> +		chomp($line);
> +
> +		# Split the line content.
> +		my ($geoname_id, $locale_code, $continent_code, $continent_name, $country_iso_code, $country_name, $is_in_european_union) = split(/\,/, $line);
> +
> +		# Check if the correct location_id has been found.
> +		if ($geoname_id eq $location_id) {
> +			# Store the county code.
> +			$country_code = $country_iso_code;
> +
> +			# Break loop.
> +			last;
> +		}
> 	}
> 
> -	# Return the name of the country
> -	return $database->country_code_by_name($address);
> +	# Close filehandle.
> +	close(LOCATION);
> +
> +	# Return the obtained country code.
> +	return $country_code;
> }
> 
> # Function to get the flag icon for a specified country code.
> -- 
> 2.19.1

Best,
-Michael

>