Add option to ignore unknown DHCP clients #1980

DL6ER · 2024-05-30T17:07:42Z

What does this implement/fix?

Add new dhcp.ignoreUnknownClients option which has been requested on Discourse. It works as follows:

This PR also features a second commit updating forgotten changes into test/pihole.toml and adds a CI test to prevent forgetting to update this file in the future.

Related issue or feature (if applicable): https://discourse.pi-hole.net/t/add-checkbox-to-block-dhcp-for-unknown-hosts/70485

Pull request in docs with documentation (if applicable): N/A

By submitting this pull request, I confirm the following:

I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
I have commented my proposed changes within the code.
I am willing to help maintain this change if there are issues with it later.
It is compatible with the EUPL 1.2 license
I have squashed any insignificant commits. (git rebase)

Checklist:

The code change is tested and works locally.
I based my code and PRs against the repositories developmental branch.
I signed off all commits. Pi-hole enforces the DCO for all contributions
I signed all my commits. Pi-hole requires signatures to verify authorship
I have read the above and my PR is ready for review.

Signed-off-by: DL6ER <dl6er@dl6er.de>

pralor-bot · 2024-05-30T17:08:03Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/add-checkbox-to-block-dhcp-for-unknown-hosts/70485/5

…ave been made in between and add a test ensuring they remain in sync in the future Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER · 2024-05-30T17:18:35Z

test/run.sh

@@ -23,13 +23,12 @@ done
 rm -rf /etc/pihole /var/log/pihole /dev/shm/FTL-*

 # Create necessary directories and files
-mkdir -p /home/pihole /etc/pihole /run/pihole /var/log/pihole
+mkdir -p /home/pihole /etc/pihole /run/pihole /var/log/pihole /etc/pihole/config_backups


/etc/pihole/config_backups was not created, causing:

2024-05-30 17:09:24.091 [1119/T1132] DEBUG_CONFIG: Copying /etc/pihole/pihole.toml -> /etc/pihole/config_backups/pihole.toml.1 2024-05-30 17:09:24.091 [1119/T1132] WARNING: copy_file(): Failed to open "/etc/pihole/config_backups/pihole.toml.1" for writing: Permission denied 2024-05-30 17:09:24.091 [1119/T1132] WARNING: Rotation /etc/pihole/pihole.toml -(COPY)> /etc/pihole/config_backups/pihole.toml.1 failed 2024-05-30 17:09:24.091 [1119/T1132] WARNING: chown_pihole(): Failed to change ownership of "/etc/pihole/config_backups/pihole.toml.1" to 1000:1000: Permission denied

during CI runs. It just looked like worth fixing

DL6ER · 2024-05-30T17:18:52Z

test/run.sh

-touch /var/log/pihole/HTTP_info.log /var/log/pihole/PH7.log /etc/pihole/dhcp.leases
-chown pihole:pihole /etc/pihole /run/pihole /var/log/pihole/pihole.log /var/log/pihole/FTL.log /run/pihole-FTL.pid /run/pihole-FTL.port
-chown pihole:pihole /var/log/pihole/HTTP_info.log /var/log/pihole/PH7.log /etc/pihole/dhcp.leases
+touch /var/log/pihole/HTTP_info.log /etc/pihole/dhcp.leases


Removed PH7.log

DL6ER · 2024-05-30T17:19:42Z

test/run.sh

-chown pihole:pihole /etc/pihole /run/pihole /var/log/pihole/pihole.log /var/log/pihole/FTL.log /run/pihole-FTL.pid /run/pihole-FTL.port
-chown pihole:pihole /var/log/pihole/HTTP_info.log /var/log/pihole/PH7.log /etc/pihole/dhcp.leases
+touch /var/log/pihole/HTTP_info.log /etc/pihole/dhcp.leases
+chown -R pihole:pihole /etc/pihole /run/pihole /var/log/pihole


Simplified chown by recursively chowning the same directories which are also owned by pihole:pihole on regular installs, ensures we cannot forget to set appropriate permissions if we add new files

DL6ER · 2024-05-30T17:19:56Z

test/run.sh

-  echo -n "PH7.log: "
-  curl_to_tricorder /var/log/pihole/PH7.log
-  echo ""


Removed PH7.log here as well

DL6ER · 2024-05-30T17:20:12Z

test/test_suite.bats

-#@test "Version, Tag, Branch, Hash, Date is reported" {
-#  run bash -c 'echo ">version >quit" | nc -v 127.0.0.1 4711'
-#  printf "%s\n" "${lines[@]}"
-#  [[ ${lines[1]} == "version "* ]]
-#  [[ ${lines[2]} == "tag "* ]]
-#  [[ ${lines[3]} == "branch "* ]]
-#  [[ ${lines[4]} == "hash "* ]]
-#  [[ ${lines[5]} == "date "* ]]
-#  [[ ${lines[6]} == "" ]]
-#}
-#
-#@test "DNS server port is reported over Telnet API" {
-#  run bash -c 'echo ">dns-port >quit" | nc -v 127.0.0.1 4711'
-#  printf "%s\n" "${lines[@]}"
-#  [[ ${lines[1]} == "53" ]]
-#  [[ ${lines[2]} == "" ]]
-#}
-#
-#@test "Maxlogage value is reported over Telnet API" {
-#  run bash -c 'echo ">maxlogage >quit" | nc -v 127.0.0.1 4711'
-#  printf "%s\n" "${lines[@]}"
-#  [[ ${lines[1]} == "86400" ]]
-#  [[ ${lines[2]} == "" ]]
-#}
-#


Removed old cruft

DL6ER · 2024-05-30T17:21:04Z

test/test_suite.bats

  run bash -c 'grep -c "DEBUG_CONFIG: pihole.toml unchanged" /var/log/pihole/FTL.log'
  printf "%s\n" "${lines[@]}"
-  [[ ${lines[0]} == "3" ]]
+  [[ ${lines[0]} == "4" ]]


As test/pihole.toml is now in the expected format, there is no initial reformatting and the file stays unchanged one time more

Signed-off-by: DL6ER <dl6er@dl6er.de>

rdwebdesign · 2024-05-30T22:43:44Z

Should we add this option on the web interface, Settings > DHCP page?

Something like this?

Or this should be only in "All settings" page?

DL6ER · 2024-05-31T06:21:22Z

@rdwebdesign In the end, yes. But it'll be a separate PR and I haven't had the chance to do it myself, so far. Please go ahead :-)

Add new dhcp.ignoreUnknownClients option

1a5e7f6

Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER added Discourse Feature Request Pi-hole v6.0 labels May 30, 2024

DL6ER requested a review from a team May 30, 2024 17:07

DL6ER changed the title ~~New/dhcp ignore unknown clients~~ Add option to ignore unknown DHCP clients May 30, 2024

Synchronize test/pihole.toml with the many config file changes that h…

96da0d4

…ave been made in between and add a test ensuring they remain in sync in the future Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER force-pushed the new/dhcp_ignoreUnknownClients branch from 4bd3364 to 96da0d4 Compare May 30, 2024 17:16

DL6ER commented May 30, 2024

View reviewed changes

Explicitly chown PID and remove old PORT file

1f6d9c1

Signed-off-by: DL6ER <dl6er@dl6er.de>

rdwebdesign mentioned this pull request May 31, 2024

DHCP Settings - add option to ignore unknown DHCP clients pi-hole/web#3035

Merged

1 task

rdwebdesign approved these changes May 31, 2024

View reviewed changes

DL6ER merged commit f4b9ca2 into development-v6 Jun 1, 2024
17 checks passed

DL6ER deleted the new/dhcp_ignoreUnknownClients branch June 1, 2024 04:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to ignore unknown DHCP clients #1980

Add option to ignore unknown DHCP clients #1980

DL6ER commented May 30, 2024

pralor-bot commented May 30, 2024

DL6ER May 30, 2024

DL6ER May 30, 2024

DL6ER May 30, 2024

DL6ER May 30, 2024

DL6ER May 30, 2024

DL6ER May 30, 2024

rdwebdesign commented May 30, 2024

DL6ER commented May 31, 2024

Add option to ignore unknown DHCP clients #1980

Add option to ignore unknown DHCP clients #1980

Conversation

DL6ER commented May 30, 2024

What does this implement/fix?

Checklist:

pralor-bot commented May 30, 2024

DL6ER May 30, 2024

Choose a reason for hiding this comment

DL6ER May 30, 2024

Choose a reason for hiding this comment

DL6ER May 30, 2024

Choose a reason for hiding this comment

DL6ER May 30, 2024

Choose a reason for hiding this comment

DL6ER May 30, 2024

Choose a reason for hiding this comment

DL6ER May 30, 2024

Choose a reason for hiding this comment

rdwebdesign commented May 30, 2024

DL6ER commented May 31, 2024