Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve conditional forwarding settings #1208

Merged
merged 5 commits into from
Jul 2, 2020
Merged

Improve conditional forwarding settings #1208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
373d59f
Improve conditional forwarding settings so users can specify the subn…
DL6ER Apr 9, 2020
540ebdd
Merge branch 'devel' into new/rev-server
DL6ER Jun 4, 2020
700b575
Add note that IPv6 is supported as well.
DL6ER Jun 4, 2020
78e1f3c
Review comments on code style
DL6ER Jun 21, 2020
156b7b4
Update settings.php
DL6ER Jun 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions scripts/pi-hole/js/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,17 +229,6 @@ $(function () {
$('[data-toggle="tooltip"]').tooltip({ html: true, container: "body" });
});

// Change "?tab=" parameter in URL for save and reload
$(".nav-tabs a").on("shown.bs.tab", function (e) {
var tab = e.target.hash.substring(1);
window.history.pushState("", "", "?tab=" + tab);
if (tab === "piholedhcp") {
window.location.reload();
}

window.scrollTo(0, 0);
});

// Auto dismissal for info notifications
$(function () {
var alInfo = $("#alInfo");
Expand All @@ -255,6 +244,25 @@ $(function () {
input.setAttribute("autocorrect", "off");
input.setAttribute("autocapitalize", "off");
input.setAttribute("spellcheck", false);

// En-/disable conditional forwarding input fields based
// on the checkbox state
$('input[name="rev_server"]').click(function () {
$('input[name="rev_server_cidr"]').prop("disabled", !this.checked);
$('input[name="rev_server_target"]').prop("disabled", !this.checked);
$('input[name="rev_server_domain"]').prop("disabled", !this.checked);
});
});

// Change "?tab=" parameter in URL for save and reload
$(".nav-tabs a").on("shown.bs.tab", function (e) {
var tab = e.target.hash.substring(1);
window.history.pushState("", "", "?tab=" + tab);
if (tab === "piholedhcp") {
window.location.reload();
}

window.scrollTo(0, 0);
});

// Bar/Smooth chart toggle
Expand Down
57 changes: 44 additions & 13 deletions scripts/pi-hole/php/savesettings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,30 @@ function validIP($address){
return !filter_var($address, FILTER_VALIDATE_IP) === false;
}

function validCIDRIP($address){
// This validation strategy has been taken from ../js/groups-common.js
$isIPv6 = strpos($address, ":") !== false;
if($isIPv6) {
// One IPv6 element is 16bit: 0000 - FFFF
$v6elem = "[0-9A-Fa-f]{1,4}";
// CIDR for IPv6 is any multiple of 4 from 4 up to 128 bit
$v6cidr = "(4";
for ($i=8; $i <= 128; $i+=4) {
$v6cidr .= "|$i";
}
$v6cidr .= ")";
$validator = "/^(((?:$v6elem))((?::$v6elem))*::((?:$v6elem))((?::$v6elem))*|((?:$v6elem))((?::$v6elem)){7})\/$v6cidr$/";
return preg_match($validator, $address);
} else {
// One IPv4 element is 8bit: 0 - 256
$v4elem = "(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]?|0)";
// Note that rev-server accepts only /8, /16, /24, and /32
$allowedv4cidr = "(8|16|24|32)";
$validator = "/^$v4elem\.$v4elem\.$v4elem\.$v4elem\/$allowedv4cidr$/";
return preg_match($validator, $address);
}
}

// Check for existance of variable
// and test it only if it exists
function istrue(&$argument) {
Expand Down Expand Up @@ -323,28 +347,35 @@ function addStaticDHCPLease($mac, $ip, $hostname) {
$extra .= "no-dnssec";
}

// Check if Conditional Forwarding is requested
if(isset($_POST["conditionalForwarding"]))
// Check if rev-server is requested
if(isset($_POST["rev_server"]))
{
$conditionalForwardingIP = trim($_POST["conditionalForwardingIP"]);
$conditionalForwardingDomain = trim($_POST["conditionalForwardingDomain"]);
// Validate CIDR IP
$cidr = trim($_POST["rev_server_cidr"]);
if (!validCIDRIP($cidr))
{
$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>";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, it works if I use /16

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@DL6ER DL6ER Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPv5

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

// Validate conditional forwarding IP
if (!validIP($conditionalForwardingIP))
// Validate target IP
$target = trim($_POST["rev_server_target"]);
if (!validIP($target))
{
$error .= "Conditional forwarding IP (".htmlspecialchars($conditionalForwardingIP).") is invalid!<br>";
$error .= "Conditional forwarding target IP (\"".htmlspecialchars($target)."\") is invalid!<br>";
}

// Validate conditional forwarding domain name
if(!validDomain($conditionalForwardingDomain))
// Validate conditional forwarding domain name (empty is okay)
$domain = trim($_POST["rev_server_domain"]);
if(strlen($domain) > 0 && !validDomain($domain))
{
$error .= "Conditional forwarding domain name (".htmlspecialchars($conditionalForwardingDomain).") is invalid!<br>";
$error .= "Conditional forwarding domain name (\"".htmlspecialchars($domain)."\") is invalid!<br>";
}

if(!$error)
{
$addressArray = explode(".", $conditionalForwardingIP);
$reverseAddress = $addressArray[2].".".$addressArray[1].".".$addressArray[0].".in-addr.arpa";
$extra .= " conditional_forwarding ".$conditionalForwardingIP." ".$conditionalForwardingDomain." $reverseAddress";
$extra .= " rev-server ".$cidr." ".$target." ".$domain;
}
}

Expand Down
100 changes: 66 additions & 34 deletions settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,13 @@
} else {
$DNSinterface = "single";
}
if (isset($setupVars["CONDITIONAL_FORWARDING"]) && ($setupVars["CONDITIONAL_FORWARDING"] == 1)) {
$conditionalForwarding = true;
$conditionalForwardingDomain = $setupVars["CONDITIONAL_FORWARDING_DOMAIN"];
$conditionalForwardingIP = $setupVars["CONDITIONAL_FORWARDING_IP"];
if (isset($setupVars["REV_SERVER"]) && ($setupVars["REV_SERVER"] == 1)) {
$rev_server = true;
$rev_server_cidr = $setupVars["REV_SERVER_CIDR"];
$rev_server_target = $setupVars["REV_SERVER_TARGET"];
$rev_server_domain = $setupVars["REV_SERVER_DOMAIN"];
} else {
$conditionalForwarding = false;
$rev_server = false;
}
?>

Expand Down Expand Up @@ -975,38 +976,69 @@ function convertseconds($argument)
when enabling DNSSEC. A DNSSEC resolver test can be found
<a href="https://dnssec.vs.uni-due.de/" rel="noopener" target="_blank">here</a>.</p>
</div>
<strong>Conditional Forwarding</strong>
<p>If not configured as your DHCP server, Pi-hole won't be able to
<p>Validate DNS replies and cache DNSSEC data. When forwarding DNS
queries, Pi-hole requests the DNSSEC records needed to validate
the replies. If a domain fails validation or the upstream does not
support DNSSEC, this setting can cause issues resolving domains.
Use Google, Cloudflare, DNS.WATCH, Quad9, or another DNS
server which supports DNSSEC when activating DNSSEC. Note that
the size of your log might increase significantly
when enabling DNSSEC. A DNSSEC resolver test can be found
<a href="https://dnssec.vs.uni-due.de/" rel="noopener" target="_blank">here</a>.</p>
<br>
<h4>Conditional forwarding</h4>
DL6ER marked this conversation as resolved.
Show resolved Hide resolved
<p>If not configured as your DHCP server, Pi-hole typically won't be able to
determine the names of devices on your local network. As a
result, tables such as Top Clients will only show IP addresses.</p>
<p>One solution for this is to configure Pi-hole to forward these
requests to your DHCP server (most likely your router), but only for devices on your
home network. To configure this we will need to know the IP
address of your DHCP server and the name of your local network.</p>
<p>Note: The local domain name must match the domain name specified
in your DHCP server, likely found within the DHCP settings.</p>
<div>
<input type="checkbox" name="conditionalForwarding" id="conditionalForwarding" value="conditionalForwarding" <?php if(isset($conditionalForwarding) && ($conditionalForwarding == true)){ ?>checked<?php } ?>>
<label for="conditionalForwarding"><strong>Use Conditional Forwarding</strong></label>
</div>
<div class="input-group">
<table class="table table-bordered">
<tr>
<th>IP of your router</th>
<th>Local domain name</th>
</tr>
<tr>
<div class="input-group">
<td>
<input type="text" name="conditionalForwardingIP" class="form-control" autocomplete="off" spellcheck="false" autocapitalize="none" autocorrect="off"
<?php if(isset($conditionalForwardingIP)){ ?>value="<?php echo $conditionalForwardingIP; ?>"<?php } ?>>
</td>
<td><input type="text" name="conditionalForwardingDomain" class="form-control" data-mask autocomplete="off" spellcheck="false" autocapitalize="none" autocorrect="off"
<?php if(isset($conditionalForwardingDomain)){ ?>value="<?php echo $conditionalForwardingDomain; ?>"<?php } ?>>
</td>
</div>
</tr>
</table>
requests to your DHCP server (most likely your router), but only for devices on your
home network. To configure this we will need to know the IP
address of your DHCP server and which addresses belong to your local network.
Exemplary inout is given below as placeholder in the text boxes (if empty).</p>
<p>If your local network spans 192.168.0.1 - 192.168.0.255, then you will have to input
<code>192.168.0.0/24</code>. If your local network is 192.168.47.1 - 192.168.47.255, it will
be <code>192.168.47.0/24</code> and similar. If your network is larger, the CIDR has to be
different, for instance a range of 10.8.0.1 - 10.8.255.255 results in <code>10.8.0.0/16</code>,
whereas an even wider network of 10.0.0.1 - 10.255.255.255 results in <code>10.0.0.0/8</code>.
Setting up IPv6 ranges is exactly similar to setting up IPv4 here and fully supported.
Feel free to reach out to us on our
<a href="https://discourse.pi-hole.net" target="_blank">Discourse forum</a>
in case you need any assistance setting up local host name resolution for your particular system.</p>
<p>You can also specify a local domain name (like <code>fritz.box</code>) to ensure queries to
devices ending in your local domain name will not leave your network, however, this is optional.
The local domain name must match the domain name specified
in your DHCP server for this to work. You can likely find it within the DHCP settings.</p>
<div class="form-group">
<div>
<input type="checkbox" name="rev_server" id="rev_server" value="rev_server" <?php if(isset($rev_server) && ($rev_server == true)){ ?>checked<?php } ?>>
<label for="rev_server"><strong>Use Conditional Forwarding</strong></label>
</div>
<div class="input-group">
<table class="table table-bordered">
<tr>
<th>Local network in <a href="https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing" target="_blank">CIDR notation</a></th>
<th>IP address of your DHCP server (router)</th>
<th>Local domain name (optional)</th>
</tr>
<tr>
<td>
<input type="text" name="rev_server_cidr" placeholder="192.168.0.0/16" class="form-control" autocomplete="off" spellcheck="false" autocapitalize="none" autocorrect="off"
<?php if(isset($rev_server_cidr)){ ?>value="<?php echo $rev_server_cidr; ?>"<?php } ?>
<?php if(!isset($rev_server) || !$rev_server){ ?>disabled<?php } ?>>
</td>
<td>
<input type="text" name="rev_server_target" placeholder="192.168.0.1" class="form-control" autocomplete="off" spellcheck="false" autocapitalize="none" autocorrect="off"
<?php if(isset($rev_server_target)){ ?>value="<?php echo $rev_server_target; ?>"<?php } ?>
<?php if(!isset($rev_server) || !$rev_server){ ?>disabled<?php } ?>>
</td>
<td>
<input type="text" name="rev_server_domain" placeholder="local" class="form-control" data-mask autocomplete="off" spellcheck="false" autocapitalize="none" autocorrect="off"
<?php if(isset($rev_server_domain)){ ?>value="<?php echo $rev_server_domain; ?>"<?php } ?>
<?php if(!isset($rev_server) || !$rev_server){ ?>disabled<?php } ?>>
</td>
</tr>
</table>
</div>
</div>
</div>
</div>
Expand Down