[libvirt] [PATCH] network: add prefix to dhcp range of dnsmasq conf file for IPv4 too

Laine Stump posted 1 patch 5 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20181017160609.434767-1-laine@laine.org
Test syntax-check passed
src/network/bridge_driver.c                                       | 7 ++-----
tests/networkxml2confdata/dhcp6-nat-network.conf                  | 2 +-
tests/networkxml2confdata/isolated-network.conf                   | 2 +-
tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf | 2 +-
tests/networkxml2confdata/nat-network-dns-srv-record.conf         | 2 +-
tests/networkxml2confdata/nat-network-dns-txt-record.conf         | 2 +-
tests/networkxml2confdata/nat-network-name-with-quotes.conf       | 2 +-
tests/networkxml2confdata/nat-network.conf                        | 2 +-
tests/networkxml2confdata/netboot-network.conf                    | 2 +-
tests/networkxml2confdata/netboot-proxy-network.conf              | 2 +-
tests/networkxml2confdata/ptr-domains-auto.conf                   | 2 +-
11 files changed, 12 insertions(+), 15 deletions(-)
[libvirt] [PATCH] network: add prefix to dhcp range of dnsmasq conf file for IPv4 too
Posted by Laine Stump 5 years, 6 months ago
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 that added dhcp support to virtual
networks did not add a prefix to the dnsmasq commandline (or later,
the dnsmasq conf file).

For *IPv6* however, dnsmasq cannot automatically determine the prefix,
so it must be explicitly provided in the conf file (as a part of the
dhcp-range option). Years after the initial IPv4 support, when IPv6
dhcp support was added, libvirt added the prefix to dhcp-range, but
only for IPv6 (following the "if it ain't broke, don't fix it"
doctrine).

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/broadcast address 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 adding the prefix to all
dhcp-range options regardless of IPv4 vs IPv6, so that's what this
patch does.

Signed-off-by: Laine Stump <laine@laine.org>
---
 src/network/bridge_driver.c                                       | 7 ++-----
 tests/networkxml2confdata/dhcp6-nat-network.conf                  | 2 +-
 tests/networkxml2confdata/isolated-network.conf                   | 2 +-
 tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf | 2 +-
 tests/networkxml2confdata/nat-network-dns-srv-record.conf         | 2 +-
 tests/networkxml2confdata/nat-network-dns-txt-record.conf         | 2 +-
 tests/networkxml2confdata/nat-network-name-with-quotes.conf       | 2 +-
 tests/networkxml2confdata/nat-network.conf                        | 2 +-
 tests/networkxml2confdata/netboot-network.conf                    | 2 +-
 tests/networkxml2confdata/netboot-proxy-network.conf              | 2 +-
 tests/networkxml2confdata/ptr-domains-auto.conf                   | 2 +-
 11 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 4bbc4f5a6d..7f5ff79fdc 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1416,11 +1416,8 @@ 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");
+            virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d\n",
+                              saddr, eaddr, prefix);
 
             VIR_FREE(saddr);
             VIR_FREE(eaddr);
diff --git a/tests/networkxml2confdata/dhcp6-nat-network.conf b/tests/networkxml2confdata/dhcp6-nat-network.conf
index d1058df3b6..e1e110fe23 100644
--- a/tests/networkxml2confdata/dhcp6-nat-network.conf
+++ b/tests/networkxml2confdata/dhcp6-nat-network.conf
@@ -8,7 +8,7 @@ strict-order
 except-interface=lo
 bind-dynamic
 interface=virbr0
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-range=2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff,64
diff --git a/tests/networkxml2confdata/isolated-network.conf b/tests/networkxml2confdata/isolated-network.conf
index ce4a59f6c1..d182f42f0a 100644
--- a/tests/networkxml2confdata/isolated-network.conf
+++ b/tests/networkxml2confdata/isolated-network.conf
@@ -10,7 +10,7 @@ bind-interfaces
 listen-address=192.168.152.1
 dhcp-option=3
 no-resolv
-dhcp-range=192.168.152.2,192.168.152.254
+dhcp-range=192.168.152.2,192.168.152.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
index f35ea1d5d4..678e4a4bfd 100644
--- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
+++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
@@ -13,7 +13,7 @@ listen-address=fc00:db8:ac10:fe01::1
 listen-address=fc00:db8:ac10:fd01::1
 listen-address=10.24.10.1
 srv-host=_name._tcp
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf
index af1ed70758..4f21eb18b3 100644
--- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf
+++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf
@@ -15,7 +15,7 @@ srv-host=_name4._tcp.test4.com,test4.example.com,4444
 srv-host=_name5._udp,test5.example.com,1,55,555
 srv-host=_name6._tcp.test6.com,test6.example.com,6666,0,666
 srv-host=_name7._tcp.test7.com,test7.example.com,1,0,777
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/nat-network-dns-txt-record.conf b/tests/networkxml2confdata/nat-network-dns-txt-record.conf
index 7f560fbb5c..12e13c999e 100644
--- a/tests/networkxml2confdata/nat-network-dns-txt-record.conf
+++ b/tests/networkxml2confdata/nat-network-dns-txt-record.conf
@@ -9,7 +9,7 @@ except-interface=lo
 bind-dynamic
 interface=virbr0
 txt-record=example,example value
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/nat-network-name-with-quotes.conf b/tests/networkxml2confdata/nat-network-name-with-quotes.conf
index 36e11d17b9..63475ef511 100644
--- a/tests/networkxml2confdata/nat-network-name-with-quotes.conf
+++ b/tests/networkxml2confdata/nat-network-name-with-quotes.conf
@@ -13,7 +13,7 @@ listen-address=fc00:db8:ac10:fe01::1
 listen-address=fc00:db8:ac10:fd01::1
 listen-address=10.24.10.1
 srv-host=_name._tcp
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/nat-network.conf b/tests/networkxml2confdata/nat-network.conf
index a3c8b102d3..015d51c952 100644
--- a/tests/networkxml2confdata/nat-network.conf
+++ b/tests/networkxml2confdata/nat-network.conf
@@ -8,7 +8,7 @@ strict-order
 except-interface=lo
 bind-dynamic
 interface=virbr0
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf
index b554a5456c..987164c24c 100644
--- a/tests/networkxml2confdata/netboot-network.conf
+++ b/tests/networkxml2confdata/netboot-network.conf
@@ -10,7 +10,7 @@ expand-hosts
 except-interface=lo
 bind-interfaces
 listen-address=192.168.122.1
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 enable-tftp
diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf
index afb4033f7e..ad7e55fd09 100644
--- a/tests/networkxml2confdata/netboot-proxy-network.conf
+++ b/tests/networkxml2confdata/netboot-proxy-network.conf
@@ -10,7 +10,7 @@ expand-hosts
 except-interface=lo
 bind-interfaces
 listen-address=192.168.122.1
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-boot=pxeboot.img,,10.20.30.40
diff --git a/tests/networkxml2confdata/ptr-domains-auto.conf b/tests/networkxml2confdata/ptr-domains-auto.conf
index 7f1a393dd5..3be679ac4d 100644
--- a/tests/networkxml2confdata/ptr-domains-auto.conf
+++ b/tests/networkxml2confdata/ptr-domains-auto.conf
@@ -10,7 +10,7 @@ local=/1.0.e.f.0.1.c.a.8.b.d.0.1.0.0.2.ip6.arpa/
 except-interface=lo
 bind-dynamic
 interface=virbr0
-dhcp-range=192.168.122.2,192.168.122.254
+dhcp-range=192.168.122.2,192.168.122.254,24
 dhcp-no-override
 dhcp-authoritative
 dhcp-lease-max=253
-- 
2.14.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: add prefix to dhcp range of dnsmasq conf file for IPv4 too
Posted by Laine Stump 5 years, 6 months ago
Self-NACK. I'vebeen informed by the original reporter that for IPv4 we
have to specify a netmask rather than prefix (it worked for me with
prefix, but for him it didn't :-/)

I'll send a V2 in the morning. Too close to bedtime now to do anything
that requires attention to detail...

On 10/17/2018 12:06 PM, Laine Stump wrote:
> 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 that added dhcp support to virtual
> networks did not add a prefix to the dnsmasq commandline (or later,
> the dnsmasq conf file).
>
> For *IPv6* however, dnsmasq cannot automatically determine the prefix,
> so it must be explicitly provided in the conf file (as a part of the
> dhcp-range option). Years after the initial IPv4 support, when IPv6
> dhcp support was added, libvirt added the prefix to dhcp-range, but
> only for IPv6 (following the "if it ain't broke, don't fix it"
> doctrine).
>
> 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/broadcast address 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 adding the prefix to all
> dhcp-range options regardless of IPv4 vs IPv6, so that's what this
> patch does.
>
> Signed-off-by: Laine Stump <laine@laine.org>
> ---
>  src/network/bridge_driver.c                                       | 7 ++-----
>  tests/networkxml2confdata/dhcp6-nat-network.conf                  | 2 +-
>  tests/networkxml2confdata/isolated-network.conf                   | 2 +-
>  tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf | 2 +-
>  tests/networkxml2confdata/nat-network-dns-srv-record.conf         | 2 +-
>  tests/networkxml2confdata/nat-network-dns-txt-record.conf         | 2 +-
>  tests/networkxml2confdata/nat-network-name-with-quotes.conf       | 2 +-
>  tests/networkxml2confdata/nat-network.conf                        | 2 +-
>  tests/networkxml2confdata/netboot-network.conf                    | 2 +-
>  tests/networkxml2confdata/netboot-proxy-network.conf              | 2 +-
>  tests/networkxml2confdata/ptr-domains-auto.conf                   | 2 +-
>  11 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 4bbc4f5a6d..7f5ff79fdc 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1416,11 +1416,8 @@ 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");
> +            virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d\n",
> +                              saddr, eaddr, prefix);
>  
>              VIR_FREE(saddr);
>              VIR_FREE(eaddr);
> diff --git a/tests/networkxml2confdata/dhcp6-nat-network.conf b/tests/networkxml2confdata/dhcp6-nat-network.conf
> index d1058df3b6..e1e110fe23 100644
> --- a/tests/networkxml2confdata/dhcp6-nat-network.conf
> +++ b/tests/networkxml2confdata/dhcp6-nat-network.conf
> @@ -8,7 +8,7 @@ strict-order
>  except-interface=lo
>  bind-dynamic
>  interface=virbr0
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-range=2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff,64
> diff --git a/tests/networkxml2confdata/isolated-network.conf b/tests/networkxml2confdata/isolated-network.conf
> index ce4a59f6c1..d182f42f0a 100644
> --- a/tests/networkxml2confdata/isolated-network.conf
> +++ b/tests/networkxml2confdata/isolated-network.conf
> @@ -10,7 +10,7 @@ bind-interfaces
>  listen-address=192.168.152.1
>  dhcp-option=3
>  no-resolv
> -dhcp-range=192.168.152.2,192.168.152.254
> +dhcp-range=192.168.152.2,192.168.152.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
> index f35ea1d5d4..678e4a4bfd 100644
> --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
> +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf
> @@ -13,7 +13,7 @@ listen-address=fc00:db8:ac10:fe01::1
>  listen-address=fc00:db8:ac10:fd01::1
>  listen-address=10.24.10.1
>  srv-host=_name._tcp
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf
> index af1ed70758..4f21eb18b3 100644
> --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf
> +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf
> @@ -15,7 +15,7 @@ srv-host=_name4._tcp.test4.com,test4.example.com,4444
>  srv-host=_name5._udp,test5.example.com,1,55,555
>  srv-host=_name6._tcp.test6.com,test6.example.com,6666,0,666
>  srv-host=_name7._tcp.test7.com,test7.example.com,1,0,777
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/nat-network-dns-txt-record.conf b/tests/networkxml2confdata/nat-network-dns-txt-record.conf
> index 7f560fbb5c..12e13c999e 100644
> --- a/tests/networkxml2confdata/nat-network-dns-txt-record.conf
> +++ b/tests/networkxml2confdata/nat-network-dns-txt-record.conf
> @@ -9,7 +9,7 @@ except-interface=lo
>  bind-dynamic
>  interface=virbr0
>  txt-record=example,example value
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/nat-network-name-with-quotes.conf b/tests/networkxml2confdata/nat-network-name-with-quotes.conf
> index 36e11d17b9..63475ef511 100644
> --- a/tests/networkxml2confdata/nat-network-name-with-quotes.conf
> +++ b/tests/networkxml2confdata/nat-network-name-with-quotes.conf
> @@ -13,7 +13,7 @@ listen-address=fc00:db8:ac10:fe01::1
>  listen-address=fc00:db8:ac10:fd01::1
>  listen-address=10.24.10.1
>  srv-host=_name._tcp
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/nat-network.conf b/tests/networkxml2confdata/nat-network.conf
> index a3c8b102d3..015d51c952 100644
> --- a/tests/networkxml2confdata/nat-network.conf
> +++ b/tests/networkxml2confdata/nat-network.conf
> @@ -8,7 +8,7 @@ strict-order
>  except-interface=lo
>  bind-dynamic
>  interface=virbr0
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253
> diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf
> index b554a5456c..987164c24c 100644
> --- a/tests/networkxml2confdata/netboot-network.conf
> +++ b/tests/networkxml2confdata/netboot-network.conf
> @@ -10,7 +10,7 @@ expand-hosts
>  except-interface=lo
>  bind-interfaces
>  listen-address=192.168.122.1
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  enable-tftp
> diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf
> index afb4033f7e..ad7e55fd09 100644
> --- a/tests/networkxml2confdata/netboot-proxy-network.conf
> +++ b/tests/networkxml2confdata/netboot-proxy-network.conf
> @@ -10,7 +10,7 @@ expand-hosts
>  except-interface=lo
>  bind-interfaces
>  listen-address=192.168.122.1
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-boot=pxeboot.img,,10.20.30.40
> diff --git a/tests/networkxml2confdata/ptr-domains-auto.conf b/tests/networkxml2confdata/ptr-domains-auto.conf
> index 7f1a393dd5..3be679ac4d 100644
> --- a/tests/networkxml2confdata/ptr-domains-auto.conf
> +++ b/tests/networkxml2confdata/ptr-domains-auto.conf
> @@ -10,7 +10,7 @@ local=/1.0.e.f.0.1.c.a.8.b.d.0.1.0.0.2.ip6.arpa/
>  except-interface=lo
>  bind-dynamic
>  interface=virbr0
> -dhcp-range=192.168.122.2,192.168.122.254
> +dhcp-range=192.168.122.2,192.168.122.254,24
>  dhcp-no-override
>  dhcp-authoritative
>  dhcp-lease-max=253


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list