Re: [PATCH] location-importer.in: Conduct sanity checks per DROP list

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

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

Hello Matthias,

This patch is against libloc:

  https://git.ipfire.org/?p=location/libloc.git;a=summary

-Michael

> On 19 Aug 2022, at 09:21, Matthias Fischer <matthias.fischer(a)ipfire.org> wrote:
> 
> Hi,
> 
> being curious I wanted to test this patch, but couldn't find
> 'location-importer.in' in 'src/scripts'. No chance to apply this patch.
> 
> Finally I found '.../build/usr/bin/location-importer'. Hm. Ok.
> 
> But how do I apply this patch?
> 
> Still being curious... ;-)
> 
> Matthias
> 
> On 18.08.2022 23:42, Peter Müller wrote:
>> Previously, the lack of distinction between different DROP lists caused
>> only the last one to be persisted.
>> 
>> Signed-off-by: Peter Müller <peter.mueller(a)ipfire.org>
>> ---
>> src/scripts/location-importer.in | 58 +++++++++++++++++++++-----------
>> 1 file changed, 39 insertions(+), 19 deletions(-)
>> 
>> diff --git a/src/scripts/location-importer.in b/src/scripts/location-importer.in
>> index 8d47497..e4a9ab0 100644
>> --- a/src/scripts/location-importer.in
>> +++ b/src/scripts/location-importer.in
>> @@ -1427,18 +1427,21 @@ class CLI(object):
>> 	def _update_overrides_for_spamhaus_drop(self):
>> 		downloader = location.importer.Downloader()
>> 
>> -		ip_urls = [
>> -					"https://www.spamhaus.org/drop/drop.txt",
>> -					"https://www.spamhaus.org/drop/edrop.txt",
>> -					"https://www.spamhaus.org/drop/dropv6.txt"
>> +		ip_lists = [
>> +					("Spamhaus DROP list", "https://www.spamhaus.org/drop/drop.txt"),
>> +					("Spamhaus EDROP list", "https://www.spamhaus.org/drop/edrop.txt"),
>> +					("Spamhaus DROPv6 list", "https://www.spamhaus.org/drop/dropv6.txt")
>> 				]
>> 
>> -		asn_urls = [
>> -					"https://www.spamhaus.org/drop/asndrop.txt"
>> +		asn_lists = [
>> +					("Spamhaus ASN-DROP list", "https://www.spamhaus.org/drop/asndrop.txt")
>> 				]
>> 
>> -		for url in ip_urls:
>> -			# Fetch IP list
>> +		for list in ip_lists:
>> +			name = list[0]
>> +			url = list[1]
>> +
>> +			# Fetch IP list from given URL
>> 			f = downloader.retrieve(url)
>> 
>> 			# Split into lines
>> @@ -1448,11 +1451,11 @@ class CLI(object):
>> 			# downloads.
>> 			if len(fcontent) > 10:
>> 				self.db.execute("""
>> -					DELETE FROM autnum_overrides WHERE source = 'Spamhaus ASN-DROP list';
>> -					DELETE FROM network_overrides WHERE source = 'Spamhaus DROP lists';
>> -				""")
>> +					DELETE FROM network_overrides WHERE source = '%s';
>> +				""" % name,
>> +				)
>> 			else:
>> -				log.error("Spamhaus DROP URL %s returned likely bogus file, ignored" % url)
>> +				log.error("%s (%s) returned likely bogus file, ignored" % (name, url))
>> 				continue
>> 
>> 			# Iterate through every line, filter comments and add remaining networks to
>> @@ -1475,8 +1478,8 @@ class CLI(object):
>> 
>> 					# Sanitize parsed networks...
>> 					if not self._check_parsed_network(network):
>> -						log.warning("Skipping bogus network found in Spamhaus DROP URL %s: %s" % \
>> -							(url, network))
>> +						log.warning("Skipping bogus network found in %s (%s): %s" % \
>> +							(name, url, network))
>> 						continue
>> 
>> 					# Conduct SQL statement...
>> @@ -1488,14 +1491,31 @@ class CLI(object):
>> 						) VALUES (%s, %s, %s)
>> 						ON CONFLICT (network) DO UPDATE SET is_drop = True""",
>> 						"%s" % network,
>> -						"Spamhaus DROP lists",
>> +						name,
>> 						True
>> 					)
>> 
>> -		for url in asn_urls:
>> +		for list in asn_lists:
>> +			name = list[0]
>> +			url = list[1]
>> +
>> 			# Fetch URL
>> 			f = downloader.retrieve(url)
>> 
>> +			# Split into lines
>> +			fcontent = f.readlines()
>> +
>> +			# Conduct a very basic sanity check to rule out CDN issues causing bogus DROP
>> +			# downloads.
>> +			if len(fcontent) > 10:
>> +				self.db.execute("""
>> +					DELETE FROM autnum_overrides WHERE source = '%s';
>> +				""" % name,
>> +				)
>> +			else:
>> +				log.error("%s (%s) returned likely bogus file, ignored" % (name, url))
>> +				continue
>> +
>> 			# Iterate through every line, filter comments and add remaining ASNs to
>> 			# the override table in case they are valid...
>> 			with self.db.transaction():
>> @@ -1518,8 +1538,8 @@ class CLI(object):
>> 
>> 					# Filter invalid ASNs...
>> 					if not self._check_parsed_asn(asn):
>> -						log.warning("Skipping bogus ASN found in Spamhaus DROP URL %s: %s" % \
>> -							(url, asn))
>> +						log.warning("Skipping bogus ASN found in %s (%s): %s" % \
>> +							(name, url, asn))
>> 						continue
>> 
>> 					# Conduct SQL statement...
>> @@ -1531,7 +1551,7 @@ class CLI(object):
>> 						) VALUES (%s, %s, %s)
>> 						ON CONFLICT (number) DO UPDATE SET is_drop = True""",
>> 						"%s" % asn,
>> -						"Spamhaus ASN-DROP list",
>> +						name,
>> 						True
>> 					)
>> 
>