Subnetting is not working with most relevant entry

mgg · May 4, 2020, 8:10am

Hi @yubiuser,
yes I think that should be noted in the manual.

At the moment (5.0) a client must only match one IP Range to get the expected group assignment.

DL6ER · May 4, 2020, 9:45am

Even wen we add this, you can still add multiple CIDR masks which are unique but still the same. For instance,

127.0.0.1/24
127.0.0.123/24

are actually the same thing. Due to algorithm optimization reasons, this will still lead to unpredictable behavior as in you cannot forecast which of the two most exact matches will be the chosen one. Most often, it will be the first added one (as it comes first in the index table), however, it may also be the second one in case you had a lot of groups and clients at some point, deleted them and added new ones. Your database will still be fast, however, it may not be in sequential order any longer.

yubiuser · May 4, 2020, 9:50am

Mhh.. I think this might be a potential source of user's misconfiguration. Can you add a check if an identical subnet is already defined? (Like it is for entries in general by SQL's unique constrain)

DL6ER · May 4, 2020, 11:15am

No, for one because SQL had no idea what IP addresses are and how they could be unique under certain conditions and also because users could still add such entries through the CLI/direct database interactions.

I have something more in the pipeline to address such issues...

yubiuser · May 4, 2020, 11:17am

DL6ER · May 8, 2020, 7:19pm

6 posts were split to a new topic: Client subnet mask incorrect

DL6ER · May 11, 2020, 6:36pm

This will ensure we will use the most recent + make the choice deterministic even in case of duplicates

github.com/pi-hole/FTL

Always chose best (= maximum match) subnet for clients

pi-hole:development ← pi-hole:tweak/best_subnet

opened 06:32PM - 11 May 20 UTC

DL6ER

+70 -34

**By submitting this pull request, I confirm the following:** - [X] I have re…ad and understood the [contributors guide](https://github.com/pi-hole/pi-hole/blob/master/CONTRIBUTING.md). - [X] I have checked that [another pull request](https://github.com/pi-hole/FTL/pulls) for this purpose does not exist. - [X] I have considered, and confirmed that this submission will be valuable to others. - [X] I accept that this submission may not be used, and the pull request closed at the will of the maintainer. - [X] I give this submission freely, and claim no ownership to its content. **How familiar are you with the codebase?:** ## 10 --- This allows to configure specific settings for a whole range of devices but still exclude others. Complain softly (no error) if multiple configured subnets match with the same number of relevant bits. We log in case there are multiple groups defined matching equally which one FTL chose. It will deterministic chose the most matching *last added one*. ![Screenshot at 2020-05-11 20-25-22](https://user-images.githubusercontent.com/16748619/81597148-8e180280-93c5-11ea-9a23-8d834d38fd4d.png) resulting in the following log line (only the last line is logged in non-DEBUG mode, I kept all for illustrative proposes here): ```plain [2020-05-11 20:26:55.425 18858] SQL: Comparing 127.0.0.1 vs. 127.0.0.0/8 (subnet 255.0.0.0) - !! MATCH !! [2020-05-11 20:26:55.425 18858] SQL: Comparing 127.0.0.1 vs. 127.0.0.0/8 (subnet 255.0.0.0) - !! MATCH !! [2020-05-11 20:26:55.425 18858] SQL: Comparing 127.0.0.1 vs. 127.0.0.1/8 (subnet 255.0.0.0) - !! MATCH !! [2020-05-11 20:26:55.425 18858] SQL: Comparing 127.0.0.1 vs. 127.0.0.1/8 (subnet 255.0.0.0) - !! MATCH !! [2020-05-11 20:26:55.425 18858] SQL: Comparing 127.0.0.1 vs. 127.0.0.1/8 (subnet 255.0.0.0) - !! MATCH !! [2020-05-11 20:26:55.425 18858] SUBNET WARNING: Client 127.0.0.1 is managed by 2 groups (IDs 1,37), all describing /8 subnets. FTL chose the most recent entry 127.0.0.1/8 (ID 37) for this client. ``` This will also show up on the dashboard when the new Pi-hole diagnostics system has been merged: ![Screenshot_2020-05-11 Pi-hole - ubuntu-server(1)](https://user-images.githubusercontent.com/16748619/81597737-78570d00-93c6-11ea-9072-d5a772ec0509.png)

@yubiuser Check out the web interface screeshot in this PR to see what I meant with "something more in the pipeline".

mgg · May 12, 2020, 5:36am

This is awesome !
If you want some help on testing just give me a ping.

Regards Markus

yubiuser · May 12, 2020, 7:49am

That's awesome!

One small hint: I've tried this in conjunction with new/diagnostics and it is already great that the icon changes when new messages arrive. But the triangle icon is always present - subconsciously I always think something might be wrong even when it is not. Maybe you could change the default symbol to something else not triggering a brain warning.

DL6ER · May 12, 2020, 6:58pm

Something seems to be broken for you. I don't see the triangle when there are no messages available. Note that there should also be a small orange label with the number of available warnings.

Please ensure you're on the most recent version.

yubiuser · May 12, 2020, 7:34pm

Switched back from tweak/debug_token and the triangle is gone.

The only thing that remains brocken is this HEAD>>>> MASTER seen on API/Web_Interface.

Great feature!

PromoFaux · May 12, 2020, 7:41pm

I usually only reserve this for desperate times, but sometimes it's easier just to nuke things:

sudo rm -rf /var/www/html/admin
sudo git clone https://github.com/pi-hole/adminLTE /var/www/html/admin

yubiuser · May 12, 2020, 7:48pm

Works until I switch to pihole checkout web new/diagnostics

from sudo cat /var/log/lighttpd/error.log

2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Notice:  Undefined index: PORTFILE in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Warning:  file_get_contents(): Filename cannot be empty in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Notice:  Undefined index: PORTFILE in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Warning:  file_get_contents(): Filename cannot be empty in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Notice:  Undefined index: PORTFILE in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Warning:  file_get_contents(): Filename cannot be empty in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Notice:  Undefined index: PORTFILE in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Warning:  file_get_contents(): Filename cannot be empty in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39
2020-05-12 09:47:31: (mod_fastcgi.c.421) FastCGI-stderr: PHP Notice:  Undefined index: PORTFILE in /var/www/html/admin/scripts/pi-hole/php/FTL.php on line 39

github.com

pi-hole/AdminLTE/blob/2edaaab7358a7848cd96b80780285abbad924612/scripts/pi-hole/php/FTL.php#L39


	return $piholeFTLConfig;
}
function connectFTL($address, $port=4711)
{
	if($address == "127.0.0.1")
	{
		$config = piholeFTLConfig();
		// Read port
		$portfile = file_get_contents($config["PORTFILE"]);
		if(is_numeric($portfile))
			$port = intval($portfile);
	}
	// Open Internet socket connection
	$socket = @fsockopen($address, $port, $errno, $errstr, 1.0);
	return $socket;
}
function sendRequestFTL($requestin)

-rw-rw-r-- 1 pihole root         172 Mai 12 20:02 pihole-FTL.conf

DL6ER · May 12, 2020, 7:54pm

This is not an issue as such, only a warning.

yubiuser · May 12, 2020, 7:56pm

Ok, but still strange that I see this

PromoFaux · May 12, 2020, 7:59pm

I've just checked out that branch and I see it too. It must be in the code from a merge gone wrong.. investigating...

PromoFaux · May 12, 2020, 8:06pm

Found the conflict and pushed a resolved version. Try an update on that branch please and confirm that the extraneous characters are gone.

yubiuser · May 12, 2020, 8:08pm

Can confirm it is gone.

yubiuser · June 9, 2020, 4:09pm

While playing with subnet assignment I came across the fact that there is no output to which groups a client belongs if different (non-identical) subnets are defined.
I created "subnet clients" /16 and /24 and assigned different groups to them. Connecting a device in both ranges, only the /24 should apply. But there is no indication (on cli and web interface) to which group the client was assigned in the end.

[2020-06-09 17:57:29.578 3137] SQL: Comparing 10.0.10.136 vs. 10.0.1.0/16 (subnet 255.255.0.0) - !! MATCH !!
[2020-06-09 17:57:29.578 3137] SQL: Comparing 10.0.10.136 vs. 10.0.1.0/16 (subnet 255.255.0.0) - !! MATCH !!
[2020-06-09 17:57:29.578 3137] SQL: Comparing 10.0.10.136 vs. 10.0.10.0/24 (subnet 255.255.255.0) - !! MATCH !!
[2020-06-09 17:57:29.578 3137] SQL: Comparing 10.0.10.136 vs. 10.0.10.0/24 (subnet 255.255.255.0) - !! MATCH !!
[2020-06-09 17:57:29.578 3137] SQL: Comparing 10.0.10.136 vs. 10.0.10.0/24 (subnet 255.255.255.0) - !! MATCH !!
[2020-06-09 17:57:29.579 3137] Querying gravity database for client 10.0.10.136 (getting groups)

It would be helpful if something like this would be added

Client 10.0.10.136 matched two different subnets (Client ID 3,4). Choosing the maximum match (Client ID 3) and assigned corresponding Group IDs (7,8)"

EDIT
I switched to new/mac_clients and saw that it is already implemented.

[2020-06-09 19:23:23.446 7022] Querying gravity database for client with IP 10.0.10.136...
[2020-06-09 19:23:23.446 7022] SQL: Comparing 10.0.10.136 vs. 10.0.1.0/16 (subnet 255.255.0.0) - !! MATCH !!
[2020-06-09 19:23:23.446 7022] SQL: Comparing 10.0.10.136 vs. 10.0.1.0/16 (subnet 255.255.0.0) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] SQL: Comparing 10.0.10.136 vs. 10.0.10.0/24 (subnet 255.255.255.0) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] SQL: Comparing 10.0.10.136 vs. 10.0.10.0/24 (subnet 255.255.255.0) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] SQL: Comparing 10.0.10.136 vs. 10.0.10.136 (subnet 255.255.255.255) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] SQL: Comparing 10.0.10.136 vs. 10.0.10.136 (subnet 255.255.255.255) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] SQL: Comparing 10.0.10.136 vs. 10.0.10.136 (subnet 255.255.255.255) - !! MATCH !!
[2020-06-09 19:23:23.447 7022] --> Found record for 10.0.10.136 in the client (ID 6)
[2020-06-09 19:23:23.447 7022] Querying gravity database for client 10.0.10.136 (getting groups)
[2020-06-09 19:23:23.447 7022] Gravity database: Client 10.0.10.136 found. Using groups [7].