Improve conditional forwarding settings #1208

DL6ER · 2020-04-09T07:27:05Z

By submitting this pull request, I confirm the following:

I have read and understood the contributors guide, as well as this entire template.
I have made only one major change in my proposed changes.
I have commented my proposed changes within the code.
I have tested my proposed changes, and have included unit tests where possible.
I am willing to help maintain this change if there are issues with it later.
I give this submission freely and claim no ownership.
It is compatible with the EUPL 1.2 license
I have squashed any insignificant commits. (git rebase)

What does this PR aim to accomplish?:

"Conditional forwarding" can be used to tell FTL to send queries to local devices (either with a local domain) and PTR requests for private ranges to a dedicated device (typically the router of the network) instead of the configured upstreams.
While this works for most users, it is not very flexible, as it implies three severe limitations (assuming the router is 10.1.2.3):

It is limited to IPv4
It automatically assumes a Class D network. i.e. 10.1.2.0/24. This is often correct (typically for 192.168.x.0/24 networks, but may also be wrong!
It requires the subnet to be the same Class D network the router is in (this may be a fair assumption, however).

How does this PR accomplish the above?:

This PR aims at making conditional forwarding more flexible.

It supports IPv4 and IPv6
It adds a dedicated box for the local subnet to be used (can be, e.g., 10.0.0.0/8 in above's example)
The router can be specified independently of the subnet, it could even be a foreign device (like "ask 13.225.3.2 for all IPs in range 10.0.0.0/8).

This feature is intended for Pi-hole v5.1 to ensure we have proper time for testing once it hits development

What documentation changes (if any) are needed to support this PR?:

It has to be checked whether/where we document conditional forwarding.

…et according to their needs. Signed-off-by: DL6ER <[email protected]>

LordFlashmeow · 2020-05-19T18:46:30Z

You've given detailed instructions on how to set it up for IPv4, but nothing for IPv6. Is any documentation going to be added for that?

DL6ER · 2020-05-22T10:43:29Z

Hmmm, do we really need it? It is basically the same thing with the some exception of you using an IPv6 address.

DL6ER · 2020-05-22T10:44:23Z

Also: I submitted a patch some longer time ago to the dnsmasq mailing list which removed the limitations on the allowed CIDR size, however, no response to this as of yet.

LordFlashmeow · 2020-05-22T17:02:55Z

My only concern is that IPv6 is a bit of a mystery compared to IPv4 (at least for me).

When I was looking for a way to set up conditional forwarding IPv6, all the discourse posts used a ULA in addition to the router's IP like this

Assume your ipv6 ULA-Prefix is fd0a:1234:5678::/48
also assume fd0a:1234:5678::1 is router’s ipv6 address

Add an entry into /etc/dnsmasq.d/01-pihole.conf as follows:

server=/8.7.6.5.4.3.2.1.a.0.d.f.ip6.arpa/fd0a:1234:5678::1

Sorry if I'm missing something, but would the ULA be unneeded with this update?

pralor-bot · 2020-05-31T21:07:45Z

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

https://discourse.pi-hole.net/t/ula-is-recursively-not-resolved/17711/8

pralor-bot · 2020-06-01T10:42:24Z

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

https://discourse.pi-hole.net/t/ipv6-conditional-forwarding/21541/4

DL6ER · 2020-06-04T06:43:13Z

@LordFlashmeow

Sorry if I'm missing something, but would the ULA be unneeded with this update?

No, you would simple enter

LordFlashmeow · 2020-06-04T07:00:19Z

Ok, in theory that makes sense. In practice, I guess we’ll see... :)

Signed-off-by: DL6ER <[email protected]>

settings.php

XhmikosR · 2020-06-04T18:31:44Z

I haven't tested this, just some general markup comments :)

Signed-off-by: DL6ER <[email protected]>

settings.php

Co-authored-by: Adam Warner <[email protected]>

Changes were made

PromoFaux · 2020-07-02T10:00:18Z

scripts/pi-hole/php/savesettings.php

+					{
+						$error .= "Conditional forwarding subnet (\"".htmlspecialchars($cidr)."\") is invalid!<br>".
+						          "This field requires CIDR notation for local subnets (e.g., 192.168.0.0/16).<br>".
+						          "Please use only subnets /8, /16, /24, and /32.<br>";


@DL6ER This works as expected, I get an error if I try to use a /23. But I do have a question, why is /23 invalid?

My network spans 192.168.0.1-192.168.1.254

(I should have checked this before merging I guess!)

https://discourse.pi-hole.net/t/support-prefix-12-for-conditonal-forwarding/33975?u=yubiuser

Thanks.... I tried a cursory search on discourse to see if it had already been discussed. I failed, clearly!

In any case, it works if I use /16

I think there was a mailinglist patch or question to allow for non-natural masks.

Yeah, for future reference:

http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013985.html

http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013997.html

Yeah. TL;DR dnsmasq only supports multiples of 8 for IPv4 (8, 16, 24) and multiples of 4 for IPv6 (4, 8, 12, ...). I submitted patches to allow arbitrary prefix lengths but nothing happened to them. As odd prefix-lengths cause dnsmasq to fail on startup, they are forbidden for now (until my two patches are implemented)

I do this every now and then, usually it goes unnoticed :-)

pralor-bot · 2020-07-15T21:47:40Z

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

https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1

pralor-bot · 2020-07-17T20:14:23Z

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

https://discourse.pi-hole.net/t/text-change-on-the-settings-dns-page/27022/11

DL6ER added the Enhancement label Apr 9, 2020

DL6ER added this to the v5.1 milestone Apr 9, 2020

DL6ER requested a review from a team April 9, 2020 07:27

DL6ER mentioned this pull request Apr 9, 2020

Improve conditional forwarding pi-hole/pi-hole#3260

Merged

8 tasks

Improve conditional forwarding settings so users can specify the subn…

373d59f

…et according to their needs. Signed-off-by: DL6ER <[email protected]>

DL6ER force-pushed the new/rev-server branch from 63a8f5e to 373d59f Compare April 10, 2020 08:54

PromoFaux added the Merge Conflicts label May 17, 2020

Merge branch 'devel' into new/rev-server

540ebdd

Signed-off-by: DL6ER <[email protected]>

DL6ER force-pushed the new/rev-server branch from 3ccfcb4 to 540ebdd Compare June 4, 2020 07:25

Add note that IPv6 is supported as well.

700b575

Signed-off-by: DL6ER <[email protected]>

PromoFaux requested a review from XhmikosR June 4, 2020 17:03

XhmikosR reviewed Jun 4, 2020

View reviewed changes

settings.php Outdated Show resolved Hide resolved

settings.php Outdated Show resolved Hide resolved

XhmikosR removed the Merge Conflicts label Jun 4, 2020

Review comments on code style

78e1f3c

Signed-off-by: DL6ER <[email protected]>

PromoFaux previously requested changes Jun 23, 2020

View reviewed changes

settings.php Show resolved Hide resolved

Update settings.php

156b7b4

Co-authored-by: Adam Warner <[email protected]>

DL6ER requested a review from PromoFaux June 23, 2020 19:26

PromoFaux approved these changes Jul 2, 2020

View reviewed changes

PromoFaux merged commit c42c06b into devel Jul 2, 2020

PromoFaux deleted the new/rev-server branch July 2, 2020 09:51

PromoFaux reviewed Jul 2, 2020

View reviewed changes

PromoFaux mentioned this pull request Jul 5, 2020

Pi-hole web v5.1 #1480

Merged

boostchicken mentioned this pull request Jul 13, 2020

Use conditional Forwarding on the PiHole, and other DNS settings unifi-utilities/unifios-utilities#4

Closed

binaryanomaly mentioned this pull request Jan 7, 2024

Conditional forwarding (to external DHCP server) configuration UI does not support multiple entries for i.e. adding IPv6 config pi-hole/pi-hole#5539

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve conditional forwarding settings #1208

Improve conditional forwarding settings #1208

DL6ER commented Apr 9, 2020

LordFlashmeow commented May 19, 2020

DL6ER commented May 22, 2020

DL6ER commented May 22, 2020

LordFlashmeow commented May 22, 2020

pralor-bot commented May 31, 2020

pralor-bot commented Jun 1, 2020

DL6ER commented Jun 4, 2020

LordFlashmeow commented Jun 4, 2020

XhmikosR commented Jun 4, 2020

PromoFaux Jul 2, 2020

yubiuser Jul 2, 2020

PromoFaux Jul 2, 2020

PromoFaux Jul 2, 2020

dschaper Jul 2, 2020

PromoFaux Jul 2, 2020

DL6ER Jul 3, 2020 •

edited

Loading

PromoFaux Jul 3, 2020

DL6ER Jul 3, 2020

pralor-bot commented Jul 15, 2020

pralor-bot commented Jul 17, 2020

Improve conditional forwarding settings #1208

Improve conditional forwarding settings #1208

Conversation

DL6ER commented Apr 9, 2020

LordFlashmeow commented May 19, 2020

DL6ER commented May 22, 2020

DL6ER commented May 22, 2020

LordFlashmeow commented May 22, 2020

pralor-bot commented May 31, 2020

pralor-bot commented Jun 1, 2020

DL6ER commented Jun 4, 2020

LordFlashmeow commented Jun 4, 2020

XhmikosR commented Jun 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DL6ER Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pralor-bot commented Jul 15, 2020

pralor-bot commented Jul 17, 2020

DL6ER Jul 3, 2020 •

edited

Loading