ip_range_to_cidr converts all ranges to /1 CIDR

[bitKrakenCode]

This problem was found while looking for a workaround for issue #1, as I just wanted to one-time copy the list to my npm config.

The function "ip_range_to_cidr" is transforming any IP range to a 1 bit subnet mask, which is ... kinda bad, for obvious reasons.

some examples:	CSV Input	should be CIDR	generated CIDR	resulting range
2.16.12.0,2.16.13.255,CH	2.16.12.0/23	2.16.12.0/1	0.0.0.0-127.255.255.255
2.16.92.0,2.16.92.255,CH	2.16.92.0/24	2.16.92.0/1	0.0.0.0-127.255.255.255
217.193.203.8,217.193.203.239,CH	217.193.203.8/24(?)	217.193.203.8/1	128.0.0.0-255.255.255.255

Working on a fix atm.

[bitKrakenCode]

here is my fix:

def ip_range_to_cidr(ip_start, ip_end):
    # calculate all containing subnets
    subnets = [str(ipaddr) for ipaddr in ipaddress.summarize_address_range(ipaddress.IPv4Address(ip_start), ipaddress.IPv4Address(ip_end))]
    # return list of subnet(s)
    return subnets

(I think the function is obsolete now, as the one liner can just sit inside the code)

this function may return a list of subnets now (like in the example above "217.193.203.8,217.193.203.239" will not result in a clean subnet, and will therefore end up in multiple clean subnets). therefore we need to loop trough the subnet list:

access_rule_list = [npmclient.Access_Rule_Client(address=range, directive='allow') for row in filtered_entries.itertuples(index=False) for range in ip_range_to_cidr(row.start_ip, row.end_ip)]

hopefully the additional subnets this will not make #1 much worse 😬

[bitKrakenCode]

hopefully the additional subnets this will not make https://github.com/lavinir/npm-geoaccesslists/issues/1 much worse 😬

well ...

Found 3692 entries for ['CH']
Subnet count: 5508

... sorry. it's an improvement anyway imho.

[mixpc]

@bitKrakenCode thank you tor opening this topic as I noticed the same:

This is my sample list:

1.32.233.0,1.32.233.255,GB
2.0.0.0,2.1.255.255,GB
2.16.27.0,2.16.27.255,GB
2.16.34.0,2.16.34.255,GB

which resulted in:

allow 1.32.233.0/1
allow 2.0.0.0/1
allow 2.16.27.0/1
allow 2.16.34.0/1
deny all

but a network calculator for the first allow: 1.32.233.0/1 shows

Address:   2.16.34.0
HostMin:   0.0.0.1
HostMax:   127.255.255.254

So thank you for your workaround code! /24 netmasks are ok now

However 2.0.0.0,2.1.255.255,GB results in

2.0.0.0/15
HostMin:   2.0.0.1
HostMax:   2.1.255.254

Not much of a big problem, though.

Did you managed to overcome the issue mentioned in #1 ? For a given country, it seems Network Proxy Manager will not be able to create such a big allow list...

[bitKrakenCode]

@mixpc you are welcome!

However 2.0.0.0,2.1.255.255,GB results in

2.0.0.0/15 HostMin: 2.0.0.1 HostMax: 2.1.255.254

"HostMin" / "HostMax" just means, that the range of usable IPs for your clients are 2.0.0.1 to 2.1.255.254. 2.0.0.0 is the network address, 2.1.255.255 is the broadcast address: you can't assign them to a client in your network. they are still included in 2.0.0.0/15

Did you managed to overcome the issue mentioned in https://github.com/lavinir/npm-geoaccesslists/issues/1 ? For a given country, it seems Network Proxy Manager will not be able to create such a big allow list...

I just tried it for the first time in a while: seems to be working now. but maybe I have some limit set in my script: so not 100% sure about that (but the total count for the access list seems to be fine now).

[bitKrakenCode]

@lavinir this issue could be fixed with my suggested code. you can change it yourself or give me access to the repo to open a PR for it: whatever you prefer. thanks!

[mixpc]

@bitKrakenCode thank you for your feedback. Just to make it clear, with your workaround I have been able to add IPs to the ACL. However, after running several tests I have noticed that when the ip-list-file file has more than 1,000 lines (on average) add-npm-geofilter.py will output the Error adding access list. (413) from #1 If the file has about 1,000 lines or less, it will be added to the ACL successfully. If the source ip-list-file has more than 1,000 lines (on average) the only solution I have found is split it in chunks and create as many ACLs as chunks, and then add each chunk to a different ACL. And then, yes, nginx proxy manager, will be able to take all those IPs addresses into account. I don't know whether the limit to the approx number of lines comes from the nginx proxy manager api or add-npm-geofilter.py or some other limitation.

[bitKrakenCode]

@mixpc strange. I'm pretty sure, I didn't implement a workaround for this on my system and it seems to work with a 50'000+ address list. what version of npm are you using?

[mixpc]

Hello,

services:
  nginx-proxy-manager:
   image: jc21/nginx-proxy-manager:latest

v2.11.1

Tried again from scratch. Just interested in ip v4 addresses (shorter list) so 13,972 lines in ip_fr.txt from dbip-country-lite-2024-02.csv

python add-npm-geofilter.py --npm-host 192.168.1.2 --npm-port 81 --npm-email my@email.com --npm-password 123 --npm-accesslist-name FR --allowed-countries FR --ip-list-file ip_fr.txt
Found 13972 entries for ['FR']
Getting auth token
Adding access list
adding to ruleid : 9
Error adding access list. (413)

I can try with other countries if needed. Maybe just downgrade nginx-proxy-manager to any given version?

Edit: adding to ruleid : 9 As I have oher eight ACLs from previous tests, that I may delete if needed, no problem.

[bitKrakenCode]

@mixpc ok lol 🙃 I did implement a workaround by inserting the list directly in the sqlite DB. so #1 is probably not fixed for now. sorry

If you want to implement it too: replace this line (should be line nr 64): npmclient.add_access_list(args.npm_accesslist_name, token, existing_rule_id, *access_rule_list) with this code (and adjust the path to the sqlite file):

import sqlite3
import datetime

connection = sqlite3.connect("/path/to/nginxproxymanager/database.sqlite")
cursor = connection.cursor()

# remove existing networks of rule list
cursor.execute(f"DELETE FROM access_list_client WHERE access_list_id = {existing_rule_id}")

# add each entry to the DB
for row in access_rule_list:
    query_value_meta = '{}'
    cursor.execute(f"INSERT INTO access_list_client (created_on, modified_on, access_list_id, address, directive, meta) VALUES ('{datetime.datetime.now()}', '{datetime.datetime.now()}', {existing_rule_id}, '{row.address}', '{row.directive}', '{query_value_meta}')")

connection.commit()
print("synced to DB")
connection.close()

I will probably add the workaround to #1 too.

[mixpc]

@bitKrakenCode Excellent! No need for sorries; glad to provide feedback!

sudo python add-npm-geofilter.py --npm-host 192.168.1.2 --npm-port 81 --npm-email my@email.com --npm-password 123 --npm-accesslist-name FR --allowed-countries FR --ip-list-file ip_fr.txt
Found 13972 entries for ['FR']
Getting auth token
Adding access list
synced to DB

Amazing! Thank you @bitKrakenCode 😃