'Register DHCP static mappings in DNS forwarder' somewhat lacking
-
Upon enabling 'Register DHCP static mappings in DNS forwarder' I was surprised to discover that pfSense only used the system's domain name rather than domain names from individual hosts and dhcp pools. Digging revealed that the system_hosts_generate() function in /etc/inc/system.inc was unfortunately written to do just that. At line 279 of snapshot 2.1-RC0 (amd64) built on Mon Jul 29 02:58:44 EDT 2013:
if ($host['ipaddr'] && $host['hostname']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']} {$host['hostname']}\n";I submit for consideration a slightly improved attempt at performing this task with sanity, giving priority first to host defined domain names then dhcp pool domain names before falling back to the system domain name:
if ($host['ipaddr'] && $host['hostname'] && $host['domain']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$host['domain']} {$host['hostname']}\n"; else if ($host['ipaddr'] && $host['hostname'] && $dhcpifconf['domain']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$dhcpifconf['domain']} {$host['hostname']}\n"; else if ($host['ipaddr'] && $host['hostname']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']} {$host['hostname']}\n";A similar modification to the dhcpv6 routine directly below that one may also be beneficial.
(edit: first code block was truncated. fixed.)
-
Uhm… You are only supposed to register FQDNs for zones defined on the nameserver. Say, I assign www.google.com FQDN to a laptop. You seriously want to register that in DNS forwarder?!? :o ??? ::)
-
From what I gather he is trying to achieve I think he is trying to say that if you define google.com as domains for your DHCP pools then yes. He does not appear to be saying accept any domains. He is saying also add dns for the domains in the DHCP pools not just the default system domain?
I can not follow the code so maybe doktornotor is saying the code changes that are shown would not limit it to to just dhcp pools so further code would be needed.
-
What I'm saying is that DNS server has no business registering records for zones it's not authoritative for. For anything beyond the very basic DNS forwarder needs, you should install unbound or some other DNS server.
-
DNS server has no business registering records for zones it's not authoritative for
I've never seen someone put so much opinion behind /etc/hosts :D
-
I can understand if some devs might not want to push a feature if they feel it will make the DNS forwarder too complicated for the intended goal of the base project. I can also see it from the opinion that if you want to setup DNS you should do it right from the beginning in a standardized manor.
Regardless though I do see how someone could find the extended feature very handy even though I personally don't need it. The DNS forwarder implementation would just be easier to setup and require less dependencies.
The administrator would be in control over what domains are updated for both setups(assuming the feature is programmed properly for the DNS forwarder implementation). I don't see that as being a downside for either use case which seemed to be the main complaint in your initial response. If the admin wanted to doe something non-standard and override an official DNS zone and allow zone updates from clients then they could easily do that with a regular DNS server setup too.
If it is easy enough to add the feature(I don't know), The code changes are not too risky to implement(I don't know), The devs are OK with the extended feature, Someone can code up the changes, Others find it useful… why not implement the enhanced feature?
-
This whole conversation sounds like people are terrified of this causing some kind of upstream DNS changes… All this feature does is control the writing of /etc/hosts for use by dnsmasq to serve local dns requests, having absolutely jack all to do with global DNS. I'm very very confused as to why everyone is so very confused about this.
A PC sends a DNS query to pfSense router, which is handled by dnsmasq
dnsmasq first checks its own configuration for a matching manually entered record and if found replies directly to the PC
dnsmasq second checks its local /etc/hosts file for a matching record and if found replies directly to the PC (The conversation ends here)
Failing that dnsmasq forwards the query upstream to another dns server (usually a real one) and forwards the reply back to the PC. -
The initial stated concern was that the implementation would allow a host and domain get updated in the firewalls /etc/hosts for a DNS domain the administrator did not intend. Everyone that is even slightly familiar with DNS servers should know that you can't update a remote third party domain hosted by someone else (unless they made a huge mistake implementing it). I think everyone would assume the code would check and not use the domain portion of a client hostname when adding/updating it in /etc/hosts. It would be based on what the domain is set for a given DHCP range. If the client matches a given DHCP pool then the domain defined with that pool would be updated with the client hostname.
That is my understanding of what is being discussed.
-
I don't think anyone is worried about upstream dns change - but what you purpose could allow someone local on the network to point fqdn to their machine for nefarious reasons.
So for example as just a client on this network, I could register www.google.com in the dns used by all other clients on this network. And they would all then come to my box for www.google.com where I could track what they are looking for, possible inflict their machine with some sort of badware, etc..
Its just not good practice to allow a machine to register in the networks dns any fqdn they want.. If the admin of the network wants to over ride www.google.com then he can do so quite easy with the gui already.
Now the ability for a host in specific pool to be registered sounds reasonable.
So what your saying is not happening is my pfsense domain is say local.lan but I create a dhcp pool on one of my segments that hands out say domainA.lan – your saying that host.domainA.lan is not being registered?
And you would like that to be an option.. That seems very reasonable a request, the admin still controls what the domains are via setup of the dhcp pools domain.
but allowing any host to register whatever fqdn it wants does not seem prudent from a security point of view.
-
This whole conversation sounds like people are terrified of this causing some kind of upstream DNS changes…
LOL. No, not at all.
I'm very very confused as to why everyone is so very confused about this.
I'm not really confused. Forwarder should act as forwarder. It's already complex enough to my taste. If you want full featured DNS server, then install one, instead of trying to host multiple domains in /etc/hosts. :P
-
I'm terrified someone might try to break my DNS forwarder to add features no one needs. :o
-
Alright, I guess I'm the only person who wants this. I'll just crawl back into my hole and continue patching my own setup to meet my standards of sanity.
Take care folks. :D
-
It's up to the developers whether they implement this or not… Just voicing my opinion about suitability of maintaining DNS "zones" split across /etc/hosts and DHCP server configuration. :)
Cheers.
-
So what your saying is not happening is my pfsense domain is say local.lan but I create a dhcp pool on one of my segments that hands out say domainA.lan – your saying that host.domainA.lan is not being registered?
Correct. Currently, the DHCP server will reply to the client with option domain-name set to "domainA.lan" in this case (so the client thinks it's "host.domainA.lan"), but pfSense will register the client as "host.local.lan" regardless of what the domain name for the DHCP range is set to. This is arguably broken.
And you would like that to be an option.. That seems very reasonable a request, the admin still controls what the domains are via setup of the dhcp pools domain.
I agree.
-
TechSmurf, can you submit your change as a pull request on GitHub?
-
but allowing any host to register whatever fqdn it wants does not seem prudent from a security point of view.
Is that what the proposed change does, though? In other words, where does $host['domain'] come from? If this is taken straight from the client's DHCP request, then yeah, that should certainly not be honored. However, I suspect this might actually be the "domain name" field for a static mapping (if one exists), as configured on pfSense; in that case, I think it would make perfect sense to include it.
-
It's up to the developers whether they implement this or not… Just voicing my opinion about suitability of maintaining DNS "zones" split across /etc/hosts and DHCP server configuration. :)
But the same argument can already be made about the existing "register DHCP […] in DNS forwarder" functionality. If this is really a concern, you could always just disable that option?!
-
At line 279 of snapshot 2.1-RC0 (amd64) built on Mon Jul 29 02:58:44 EDT 2013:
if ($host['ipaddr'] && $host['hostname']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']}I submit for consideration a slightly improved attempt at performing this task with sanity, giving priority first to host defined domain names then dhcp pool domain names before falling back to the system domain name:
if ($host['ipaddr'] && $host['hostname'] && $host['domain']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$host['domain']} {$host['hostname']}\n"; else if ($host['ipaddr'] && $host['hostname'] && $dhcpifconf['domain']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$dhcpifconf['domain']} {$host['hostname']}\n"; else if ($host['ipaddr'] && $host['hostname']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']} {$host['hostname']}\n";A similar modification to the dhcpv6 routine directly below that one may also be beneficial.
The suggested code adds 2 things, which is why there are so many posts here, as people are discussing the "merits" of the 1st part of the "if" statement - that allows the requesting host to specify a domain name of its own choosing, not desirable.
The first "else if" does the agreed useful thing, sets the domain to match the one given by the DHCP pool.
The last "else if" is the existing behaviour.
Something like this should be reasonable:if ($host['ipaddr'] && $host['hostname'] && $dhcpifconf['domain']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$dhcpifconf['domain']} {$host['hostname']}\n"; else if ($host['ipaddr'] && $host['hostname']) $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']} {$host['hostname']}\n";(I haven't tried this at all for real, I haven't looked at the surrounding code, it does need to make sure it is using the the domain name from the matching interface, and potentially the matching DHCP pool, when the interface has multiple pools…)
Edit: see the razzfazz post below, and ignore this one, I should have looked at the code properly first! -
The suggested code adds 2 things, which is why there are so many posts here, as people are discussing the "merits" of the 1st part of the "if" statement - that allows the requesting host to specify a domain name of its own choosing, not desirable.
No, that's not what it does: The foreach loop that encloses the statement iterates over all static DHCP entries, and the $host variable in this particular statement is simply the configuration map for one of these entries. As such, $host['domain'] is not a domain name supplied by the client host, but rather the domain name configured for a particular static DHCP lease on the pfSense side of things. This seems perfectly fine. As a matter of fact, leaving the first part out would lead to inconsistent behavior again where the client may end up getting a different domain name in the DHCP response than what the server registers in DNS.
Perhaps it is clearer when looking at the entire surrounding block of code:
275: if (isset($dnsmasqcfg['regdhcpstatic']) && is_array($config['dhcpd'])) { 276: foreach ($config['dhcpd'] as $dhcpif => $dhcpifconf) 277: if(is_array($dhcpifconf['staticmap']) && isset($dhcpifconf['enable'])) 278: foreach ($dhcpifconf['staticmap'] as $host) 279: if ($host['ipaddr'] && $host['hostname']) 280: $dhosts .= "{$host['ipaddr']} {$host['hostname']}.{$syscfg['domain']} {$host['hostname']}\n"; 281: } -
Yes, agree - I should have actually looked at the code first and not guessed :)
I see you have done the pull request for review - see what the devs think of this…
