Skip to content

Commit

Permalink
dhcpd: move staticmap preparation to parent #4642
Browse files Browse the repository at this point in the history 
Move all the duplication out of Unbound/Dnsmasq code and just
iterate over the results there.
  • Loading branch information
@fichtner
fichtner committed Feb 24, 2021
1 parent 277ffb6 commit d0822b0
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 117 deletions.
54 changes: 53 additions & 1 deletion src/etc/inc/plugins.inc.d/dhcpd.inc
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

/*
* Copyright (C) 2014-2020 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2014-2021 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2010 Ermal Luçi
* Copyright (C) 2005-2006 Colin Smith <ethethlay@gmail.com>
* Copyright (C) 2003-2004 Manuel Kasper <mk@neon1.net>
Expand Down Expand Up @@ -1883,3 +1883,55 @@ function dhcpd_dhcrelay6_configure($verbose = false)
echo "done.\n";
}
}

function dhcpd_staticmap($inet = 4, $domain_fallback = 'not.found', $ifconfig_details = null)
{
$root = $inet == 6 ? 'dhcpdv6' : 'dhcpd';
$ipaddr = $inet == 6 ? 'ipaddrv6' : 'ipaddr';
$staticmap = [];

foreach (config_read_array($root) as $dhcpif => $dhcpifconf) {
if (!isset($dhcpifconf['staticmap']) || !isset($dhcpifconf['enable'])) {
continue;
}

$ifconf = config_read_array('interfaces', $dhcpif);
list ($ipaddrv6) = $inet == 6 && isset($ifconf['dhcpd6track6allowoverride']) ?
interfaces_primary_address6($dhcpif, null, $ifconfig_details) : [null];

foreach ($dhcpifconf['staticmap'] as $host) {
if (empty($host[$ipaddr]) || empty($host['hostname'])) {
continue;
}

if (!empty($ipaddrv6)) {
$host['ipaddrv6'] = make_ipv6_64_address($ipaddrv6, $host['ipaddrv6']);
}

// XXX: dhcpdv6 domain entries have been superseded by domainsearchlist,
// for backward compatibilty support both here.
if (!empty($host['domainsearchlist'])) {
$domain = $host['domainsearchlist'];
} elseif (!empty($host['domain'])) {
$domain = $host['domain'];
} elseif (!empty($dhcpifconf['domainsearchlist'])) {
$domain = $dhcpifconf['domainsearchlist'];
} elseif (!empty($dhcpifconf['domain'])) {
$domain = $dhcpifconf['domain'];
} else {
$domain = $domain_fallback;
}

$domain = explode(";", $domain)[0]; // XXX: first entry of domainsearchlist

$staticmap[$dhcpif] = [
'descr' => $host['descr'],
'domain' => $domain,
'hostname' => $host['hostname'],
$ipaddr => $host[$ipaddr],
];
}
}

return $staticmap;
}
70 changes: 13 additions & 57 deletions src/etc/inc/plugins.inc.d/dnsmasq.inc
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php

/*
* Copyright (C) 2014-2020 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2014-2021 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2010 Ermal Luçi
* Copyright (C) 2005-2006 Colin Smith <ethethlay@gmail.com>
* Copyright (C) 2003-2004 Manuel Kasper <mk@neon1.net>
Expand Down Expand Up @@ -223,66 +223,22 @@ function _dnsmasq_add_host_entries()
}
}

if (isset($dnsmasqcfg['regdhcpstatic']) && is_array($config['dhcpd'])) {
foreach ($config['dhcpd'] as $dhcpif => $dhcpifconf) {
if (isset($dhcpifconf['staticmap']) && isset($dhcpifconf['enable'])) {
foreach ($dhcpifconf['staticmap'] as $host) {
if (empty($host['ipaddr']) || empty($host['hostname'])) {
continue;
}

if (!empty($host['domain'])) {
$domain = $host['domain'];
} elseif (!empty($dhcpifconf['domain'])) {
$domain = $dhcpifconf['domain'];
} elseif (!empty($config['dnsmasq']['regdhcpdomain'])) {
$domain = $config['dnsmasq']['regdhcpdomain'];
} else {
$domain = $config['system']['domain'];
}

$dhosts .= "{$host['ipaddr']}\t{$host['hostname']}.{$domain} {$host['hostname']}\n";
}
}
if (isset($dnsmasqcfg['regdhcpstatic'])) {
require_once 'plugins.inc.d/dhcpd.inc'; /* XXX */

$domain = $config['system']['domain'];
if (!empty($config['dnsmasq']['regdhcpdomain'])) {
$domain = $config['dnsmasq']['regdhcpdomain'];
}
}

if (isset($dnsmasqcfg['regdhcpstatic']) && is_array($config['dhcpdv6'])) {
$ifconfig_details = legacy_interfaces_details();

foreach ($config['dhcpdv6'] as $dhcpif => $dhcpifconf) {
if (isset($dhcpifconf['staticmap']) && isset($dhcpifconf['enable'])) {
list ($ipaddrv6) = isset($dhcpifconf['dhcpd6track6allowoverride']) ?
interfaces_primary_address6($dhcpif, null, $ifconfig_details) : [null];
foreach ($dhcpifconf['staticmap'] as $host) {
if (empty($host['ipaddrv6']) || empty($host['hostname'])) {
continue;
}

if (!empty($ipaddrv6)) {
$host['ipaddrv6'] = make_ipv6_64_address($ipaddrv6, $host['ipaddrv6']);
}

// XXX: dhcpdv6 domain entries have been superseded by domainsearchlist,
// for backward compatibilty support both here.
if (!empty($host['domainsearchlist'])) {
$domain = $host['domainsearchlist'];
} elseif (!empty($host['domain'])) {
$domain = $host['domain'];
} elseif (!empty($dhcpifconf['domainsearchlist'])) {
$domain = $dhcpifconf['domainsearchlist'];
} elseif (!empty($dhcpifconf['domain'])) {
$domain = $dhcpifconf['domain'];
} elseif (!empty($config['dnsmasq']['regdhcpdomain'])) {
$domain = $config['dnsmasq']['regdhcpdomain'];
} else {
$domain = $config['system']['domain'];
}

$domain = explode(";", $domain)[0]; // XXX: first entry of domainsearchlist
$dhosts .= "{$host['ipaddrv6']}\t{$host['hostname']}.{$domain} {$host['hostname']}\n";
}
}
foreach (dhcpd_staticmap(4, $domain, $ifconfig_details) as $dhcpif => $host) {
$dhosts .= "{$host['ipaddr']}\t{$host['hostname']}.{$host['domain']} {$host['hostname']}\n";
}

foreach (dhcpd_staticmap(6, $domain, $ifconfig_details) as $dhcpif => $host) {
$dhosts .= "{$host['ipaddrv6']}\t{$host['hostname']}.{$host['domain']} {$host['hostname']}\n";
}
}

Expand Down
73 changes: 14 additions & 59 deletions src/etc/inc/plugins.inc.d/unbound.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/*
* Copyright (C) 2018 Fabian Franz
* Copyright (C) 2015-2020 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2015-2021 Franco Fichtner <franco@opnsense.org>
* Copyright (C) 2015 Manuel Faux <mfaux@conf.at>
* Copyright (C) 2014 Warren Baker <warren@decoy.co.za>
* Copyright (C) 2004-2007 Scott Ullrich <sullrich@gmail.com>
Expand Down Expand Up @@ -647,67 +647,22 @@ function unbound_add_host_entries($ifconfig_details = null)
}
}

if (isset($config['unbound']['regdhcpstatic']) && is_array($config['dhcpd'])) {
foreach ($config['dhcpd'] as $dhcpif => $dhcpifconf) {
if (isset($dhcpifconf['staticmap']) && isset($dhcpifconf['enable'])) {
foreach ($dhcpifconf['staticmap'] as $host) {
if (empty($host['ipaddr']) || empty($host['hostname'])) {
continue;
}

if ($host['domain']) {
$domain = $host['domain'];
} elseif ($dhcpifconf['domain']) {
$domain = $dhcpifconf['domain'];
} else {
$domain = $config['system']['domain'];
}

$unbound_entries .= "local-data-ptr: \"{$host['ipaddr']} {$host['hostname']}.{$domain}\"\n";
$unbound_entries .= "local-data: \"{$host['hostname']}.{$domain} IN A {$host['ipaddr']}\"\n";
if (!empty($host['descr']) && $unboundcfg['txtsupport'] == 'on') {
$unbound_entries .= "local-data: '{$host['hostname']}.{$domain} TXT \"" . addslashes($host['descr']) . "\"'\n";
}
}
if (isset($config['unbound']['regdhcpstatic'])) {
require_once 'plugins.inc.d/dhcpd.inc'; /* XXX */

foreach (dhcpd_staticmap(4, $config['system']['domain'], $ifconfig_details) as $dhcpif => $host) {
$unbound_entries .= "local-data-ptr: \"{$host['ipaddr']} {$host['hostname']}.{$host['domain']}\"\n";
$unbound_entries .= "local-data: \"{$host['hostname']}.{$host['domain']} IN A {$host['ipaddr']}\"\n";
if (!empty($host['descr']) && $unboundcfg['txtsupport'] == 'on') {
$unbound_entries .= "local-data: '{$host['hostname']}.{$host['domain']} TXT \"" . addslashes($host['descr']) . "\"'\n";
}
}
}

if (isset($config['unbound']['regdhcpstatic']) && is_array($config['dhcpdv6'])) {
foreach ($config['dhcpdv6'] as $dhcpif => $dhcpifconf) {
if (isset($dhcpifconf['staticmap']) && isset($dhcpifconf['enable'])) {
list ($ipaddrv6) = isset($dhcpifconf['dhcpd6track6allowoverride']) ?
interfaces_primary_address6($dhcpif, null, $ifconfig_details) : [null];
foreach ($dhcpifconf['staticmap'] as $host) {
if (empty($host['ipaddrv6']) || empty($host['hostname'])) {
continue;
}

if (!empty($ipaddrv6)) {
$host['ipaddrv6'] = make_ipv6_64_address($ipaddrv6, $host['ipaddrv6']);
}

// XXX: dhcpdv6 domain entries have been superseded by domainsearchlist,
// for backward compatibilty support both here.
if (!empty($host['domainsearchlist'])) {
$domain = $host['domainsearchlist'];
} elseif (!empty($host['domain'])) {
$domain = $host['domain'];
} elseif (!empty($dhcpifconf['domainsearchlist'])) {
$domain = $dhcpifconf['domainsearchlist'];
} elseif (!empty($dhcpifconf['domain'])) {
$domain = $dhcpifconf['domain'];
} else {
$domain = $config['system']['domain'];
}

$domain = explode(";", $domain)[0]; // XXX: first entry of domainsearchlist
$unbound_entries .= "local-data-ptr: \"{$host['ipaddrv6']} {$host['hostname']}.{$domain}\"\n";
$unbound_entries .= "local-data: \"{$host['hostname']}.{$domain} IN AAAA {$host['ipaddrv6']}\"\n";
if (!empty($host['descr']) && $unboundcfg['txtsupport'] == 'on') {
$unbound_entries .= "local-data: '{$host['hostname']}.{$domain} TXT \"" . addslashes($host['descr']) . "\"'\n";
}
}
foreach (dhcpd_staticmap(6, $config['system']['domain'], $ifconfig_details) as $dhcpif => $host) {
$unbound_entries .= "local-data-ptr: \"{$host['ipaddrv6']} {$host['hostname']}.{$host['domain']}\"\n";
$unbound_entries .= "local-data: \"{$host['hostname']}.{$host['domain']} IN AAAA {$host['ipaddrv6']}\"\n";
if (!empty($host['descr']) && $unboundcfg['txtsupport'] == 'on') {
$unbound_entries .= "local-data: '{$host['hostname']}.{$host['domain']} TXT \"" . addslashes($host['descr']) . "\"'\n";
}
}
}
Expand Down

10 comments on commit d0822b0

@fichtner
Copy link
Member Author

Choose a reason for hiding this comment

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

@marjohn56 @maurice-w testers/reviewer welcome

@marjohn56
Copy link
Member

Choose a reason for hiding this comment

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

Quick look, seems fine. I'll test it later on today... Have to do real work first. :(

@maurice-w
Copy link
Member

Choose a reason for hiding this comment

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

Hey @fichtner, I did a quick test with unbound. The interface identifier gets prefixed correctly in host_entries.conf, but only one static mapping gets added there. Seems to be always the last one.

status_dhcpv6_leases.php only shows the interface identifier, but that might be out of scope here.

Didn't test dnsmasq.

Off topic: Hitting 'Apply changes' after static mapping changes always restarts radvd. I've probably complained about this before, but was just reminded about it by repeated connection losses. :-)

@fichtner
Copy link
Member Author

Choose a reason for hiding this comment

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

@maurice-w nice catch, e73db9c should do it, better to push more things to the ticket #4642 -- l can look at both issues for a follow-up there.

@marjohn56
Copy link
Member

@marjohn56 marjohn56 commented on d0822b0 Feb 25, 2021 •
edited

Choose a reason for hiding this comment

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

Statics OK here.. Of course all these changes screw up my PD correction stuff that's in the pipeline but I can update that PR. Also same as @maurice-w, not tested with dnsmasq

@marjohn56
Copy link
Member

Choose a reason for hiding this comment

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

Just came across a little issue that I haven't seen before, but it reminded me that it was something I wanted to do a while back and never got around to doing.

Scenario:
Tracked interface is showing a /60 delegation but I'm not able to ping the router LAN GUA address from the PC. Both addresses are in the same prefix.
Turns out that although the prefix requested is set to /60 on the WAN interface the actual prefix delegated is a /56. Now, this is not uncommon for ISPs to give you a prefix value different to what is asked for, and prefix delegation size selector on WAN should not be used as the prefix size on the LAN. We should be using the actual prefix size obtained from dhcp6c on the LAN configuration. We really do not need to rely on a user entered value, which is very likely to be incorrect to set the prefix size, the mods carried out to dhcp6c allow us to fully automate this. The WAN prefix delegation size should only be used as a hint value.

@fichtner
Copy link
Member Author

Choose a reason for hiding this comment

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

@marjohn56 good idea. From experience with 6RD it doesn't really matter what the upstream subnet size is as long as it's bigger. In the worst case we can widen the WAN prefix to something not even delegated and things still work fine. This is especially useful for the single /64 case we need to "delegate". If you are interested in this please create new tickets (for what you mentioned) and generally supporting a /64 as well.

@marjohn56
Copy link
Member

Choose a reason for hiding this comment

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

I'll take it on, leave me to look at it. It's an issue that dhcp6c is involved in, it sets the addresses on the LAN. Now, it appears that the Linux version of dhcp6c does not have a SLA-LEN in the config, so I'll be taking a gander at that as well. I've started to look at dhcp6c, the issue is in prefixconf.c in the add_ifprefix() routine. Quick look suggests that checking the obtained prefix against the ifid_len and adjusting the ifid_len accordingly may be the way forward. I'll find some hours over the next few days to play in that pit and see if I can make it do what we want.

@marjohn56
Copy link
Member

Choose a reason for hiding this comment

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

I have it working. dhcp6c now auto corrects where a requested prefix does not match the delegated prefix. I'll create a new issue tomorrow and we'll discuss.

@marjohn56
Copy link
Member

Choose a reason for hiding this comment

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

Discussion open in features.

Please sign in to comment.