Project

General

Profile

Bug #5041

dhcpd.conf in-addr.arpa zones incorrect in some circumstances

Added by Mike Steiner about 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Category:
DHCP Server
Target version:
Start date:
08/27/2015
Due date:
% Done:

100%

Estimated time:
Affected Version:
All
Affected Architecture:

Description

I just noticed DNS pointer entries don't work for one of my pfSense installations.

It looks like the dhcpd.conf is generated wrong when the subnet is 192.168.0.0
In my example I set an option to update the DNS server and the pointer records don't show on the Windows DNS server (where it works with other installations). Checking the dhcpd.conf it has the following lines (192.168.0.7 is the DNS server) for DNS updates:

}
ddns-update-style interim;
update-static-leases on;
zone testdomain.local. {
    primary 192.168.0.7;
}
zone 168.192.in-addr.arpa {
    primary 192.168.0.7;
}

The "0." is missing for the reverse lookup zone. It should be:
}
ddns-update-style interim;
update-static-leases on;
zone testdomain.local. {
    primary 192.168.0.7;
}
zone +*0.*+168.192.in-addr.arpa {
    primary 192.168.0.7;
}

Looks like this only affects networks with x.x.0.x
Another system (with the same configuration but different subnet 192.168.15.0/24) works well and the dhcpd.conf looks like this:
}
ddns-update-style interim;
update-static-leases on;
zone anothertestdomain.local. {
    primary 192.168.15.10;
}
zone 15.168.192.in-addr.arpa {
    primary 192.168.15.10;
}

Associated revisions

Revision cf688344 (diff)
Added by Renato Botelho over 3 years ago

Fix reverse DNS zone names on dhcpd conf. It should fix #5041

History

#1 Updated by Mike Steiner about 4 years ago

Oops, something went wrong with the formatting. The "+" signs shouldn't be there of course.

#2 Updated by Mike Steiner about 4 years ago

Looks like the bug is here in services.inc:

if (isset($dhcpifconf['ddnsupdate'])) {
$need_ddns_updates = true;
$newzone = array();
if($dhcpifconf['ddnsdomain'] <> "") {
$newzone['domain-name'] = $dhcpifconf['ddnsdomain'];
$dnscfg .= " ddns-domainname \"{$dhcpifconf['ddnsdomain']}\";\n";
} else {
$newzone['domain-name'] = $config['system']['domain'];
}
$revsubnet = explode(".", $subnet);
$revsubnet = array_reverse($revsubnet);
foreach ($revsubnet as $octet) {
if ($octet != "0")
break;
array_shift($revsubnet);
}
$newzone['ptr-domain'] = implode(".", $revsubnet) . ".in-addr.arpa";
}

For me (no programming skills so might be wrong) it looks like the line that shifts the array until there is no longer a "0" is causing the issue.
For a /24 net it shouldn't shift more than one octet.

#3 Updated by Chris Buechler about 4 years ago

  • Subject changed from Bug in generated dhcpd.conf to dhcpd.conf in-addr.arpa zones incorrect in some circumstances
  • Description updated (diff)
  • Category set to DHCP Server
  • Status changed from New to Confirmed
  • Target version set to 2.3
  • Affected Version set to All

Yeah it needs to actually calculate the network address, not try to find a 0.

#4 Updated by Phillip Davis about 4 years ago

For your 192.168.0.0/24 case and other odd cases like 192.168.1.128/25 and... this pull request should fix it:
https://github.com/pfsense/pfsense/pull/1827
Note that there are other cases with subnet mask bit counts <24 that are not nice multiples of 8 - and it is not entirely clear what to do about those, so I have made code that should not change the outcome for those sort of subnets - e.g. 172.17.0.0/20 or 192.168.16.0/20 ...

#5 Updated by Jim Pingle about 4 years ago

It's always ugly for anything other than /24, /16, and /8's. http://tools.ietf.org/html/rfc2317

Long story short the zone needs to match what's on the server, so perhaps a freeform field is a better fit rather than a guess in some cases. Tricky to accommodate all scenarios.

#6 Updated by Phillip Davis about 4 years ago

RFC2317 should deal just with cases of bit count >24 - tricks for delegating less than a /24 size piece of reverse lookup name space to an entity that has just a smaller piece of a /24. That will be rather difficult to achieve here.

For subnets like 10.20.16.0/20 (=10.20.16-31) the code (both versions) currently just generates:
16.20.10.in-addr.arpa

Perhaps it should generate all of:
16.20.10.in-addr.arpa
17.20.10.in-addr.arpa
...
30.20.10.in-addr.arpa
31.20.10.in-addr.arpa

For the particular case of 10.20.0.0/20 (=10.20.0-15) the code (both versions) generates:
20.10.in-addr.arpa

This is inconsistent behaviour between 10.20.0.0/20 and 10.20.16.0/20 - but that is the way it was and I didn't want to break something there for somebody. Mainly because I was not sure which is the best/correct thing to do:
20.10.in-addr.arpa
or
16.20.10.in-addr.arpa
17.20.10.in-addr.arpa
...
30.20.10.in-addr.arpa
31.20.10.in-addr.arpa

If someone tells me which way to go there, then I can easily code it.

#7 Updated by Jim Thompson about 4 years ago

  • Assignee set to Renato Botelho

assigned to Renato for tracking, but it looks like Phil might code it.

#8 Updated by Phillip Davis about 4 years ago

Happy to code the auto-expansion of stuff like:
10.20.16.0/20
into
Alternative (1):
16.20.10.in-addr.arpa
17.20.10.in-addr.arpa
...
30.20.10.in-addr.arpa
31.20.10.in-addr.arpa

But maybe also the user might need it to be:
Alternative (2):
20.10.in-addr.arpa

which covers more address space, but maybe is what happens to match the reverse zone on their DNS server.

Or maybe there has to be an advanced checkbox somewhere so the user can choose which of the above alternatives they want?
And in that case, which should be the default way to do it, Alternative (1) or (2)?

#9 Updated by Renato Botelho over 3 years ago

  • Status changed from Confirmed to Feedback
  • % Done changed from 0 to 100

#10 Updated by Jim Thompson over 3 years ago

bump (a month old now)

#11 Updated by Renato Botelho over 3 years ago

  • Status changed from Feedback to Resolved

It works

Also available in: Atom PDF