Re: [libvirt] [PATCHv2 2/2] network: add netmask to dhcp range of dnsmasq conf file for IPv4

dnsmasq documentation says that the *IPv4* prefix/network address/broadcast address sent to dhcp clients will be automatically determined by dnsmasq by looking at the interface it's listening on, so the original libvirt code did not add a netmask to the dnsmasq commandline (or later, the dnsmasq conf file). For *IPv6* however, dnsmasq apparently cannot automatically determine the prefix (functionally the same as a netmask), and it must be explicitly provided in the conf file (as a part of the dhcp-range option). So many years after IPv4 DHCP support had been added, when IPv6 dhcp support was added the prefix was included at the end of the dhcp-range setting, but only for IPv6. Recently a user reported (privately, because they suspected a possible security implication, which turned out to be unfounded) a bug on a host where one of the interfaces was a superset of the libvirt network where dhcp is needed (e.g., the host's ethernet is 10.0.0.20/8, and the libvirt network is 10.10.0.1/24). For some reason dnsmasq was supplying the netmask for the /8 network to clients requesting an address on the /24 interface. This seems like a bug in dnsmasq, but even if/when it gets fixed there, it looks like there is no harm in just always adding the netmask to all IPv4 dhcp-range options similar to how prefix is added to all IPv6 dhcp-range options. Signed-off-by: Laine Stump <laine(a)laine.org&gt; --- src/network/bridge_driver.c | 27 +++++++++++++++---- .../dhcp6-nat-network.conf | 2 +- .../networkxml2confdata/isolated-network.conf | 2 +- .../nat-network-dns-srv-record-minimal.conf | 2 +- .../nat-network-dns-srv-record.conf | 2 +- .../nat-network-dns-txt-record.conf | 2 +- .../networkxml2confdata/nat-network-mtu.conf | 2 +- .../nat-network-name-with-quotes.conf | 2 +- tests/networkxml2confdata/nat-network.conf | 2 +- .../networkxml2confdata/netboot-network.conf | 2 +- .../netboot-proxy-network.conf | 2 +- .../networkxml2confdata/ptr-domains-auto.conf | 2 +- 12 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6d80818e40..9fa902896b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1320,11 +1320,28 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, !(eaddr = virSocketAddrFormat(&ipdef->ranges[r].end))) goto cleanup; - virBufferAsprintf(&configbuf, "dhcp-range=%s,%s", - saddr, eaddr); - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) - virBufferAsprintf(&configbuf, ",%d", prefix); - virBufferAddLit(&configbuf, "\n"); + if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { + virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d\n", + saddr, eaddr, prefix); + } else { + /* IPv4 - dnsmasq requires a netmask rather than prefix */ + virSocketAddr netmask;

+ char *netmaskStr;

+ + if (virSocketAddrPrefixToNetmask(prefix, &netmask, AF_INET) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to translate bridge '%s' " + "prefix %d to netmask"), + def->bridge, prefix); + goto cleanup; + } + + if (!(netmaskStr = virSocketAddrFormat(&netmask))) + goto cleanup; + virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%s\n", + saddr, eaddr, netmaskStr); + VIR_FREE(netmaskStr);