[libvirt PATCH 00/17] Bump minimum dnsmasq version
This bumps the minimum dnsmasq version to the point where we do not need capability probing, reducing it to a version check (which I will be happy to remove on request). Unless I missed something, this also means we no longer need to spawn radvd manually. Note that DNSMASQ_CAPS_BINDTODEVICE was the indication of a downstream mitigation of a CVE that should no longer be needed if we have --bind-dynamic Ján Tomko (17): util: dnsmasqCapsSetFromBuffer: use error label tests: do not test dnsmasq older than 2.67 util: dnsmasq: mandate at least version 2.67 network: assume DNSMASQ_DHCPv6_SUPPORT network: assume DNSMASQ_RA_SUPPORT util: remove DNSMASQ_RA_SUPPORT network: assume DNSMASQ_CAPS_BIND_DYNAMIC network: assume DNSMASQ_CAPS_RA_PARAM util: dnsmasq: delete assumed capability flags network: remove any code dealing with radvd network: driver: remove unused radvdStateDir variable conf: remove radvdPid from virNetworkObj build: do not search for radvd binary spec: do not require radvd util: remove dnsmasqCapsGetVersion util: dnsmasq: remove caps completely network: remove unused 'driver' parameter libvirt.spec.in | 2 - meson.build | 1 - src/conf/virnetworkobj.c | 16 - src/conf/virnetworkobj.h | 7 - src/libvirt_private.syms | 4 - src/network/bridge_driver.c | 459 ++---------------- src/network/bridge_driver_platform.h | 1 - src/util/virdnsmasq.c | 69 +-- src/util/virdnsmasq.h | 24 - .../networkxml2confdata/isolated-network.conf | 5 +- .../nat-network-dns-srv-record-minimal.conf | 10 +- .../nat-network-dns-srv-record.conf | 2 + .../nat-network-dns-txt-record.conf | 2 + .../nat-network-name-with-quotes.conf | 10 +- .../networkxml2confdata/netboot-network.conf | 4 +- .../netboot-proxy-network.conf | 4 +- tests/networkxml2conftest.c | 32 +- 17 files changed, 83 insertions(+), 569 deletions(-) -- 2.31.1
Rename 'fail' to 'error' to match the prevalent usage. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index b62e353ceb..90a1ea35b6 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -620,12 +620,12 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) p = STRSKIP(buf, DNSMASQ_VERSION_STR); if (!p) - goto fail; + goto error; virSkipToDigit(&p); if (virParseVersionString(p, &caps->version, true) < 0) - goto fail; + goto error; if (strstr(buf, "--bind-dynamic")) dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC); @@ -650,7 +650,7 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) dnsmasqCapsGet(caps, DNSMASQ_CAPS_RA_PARAM) ? "" : "NOT "); return 0; - fail: + error: p = strchr(buf, '\n'); if (!p) len = strlen(buf); -- 2.31.1
Prepare to retire older versions by droping older tests. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../networkxml2confdata/isolated-network.conf | 5 +-- .../nat-network-dns-srv-record-minimal.conf | 10 +++--- .../nat-network-dns-srv-record.conf | 2 ++ .../nat-network-dns-txt-record.conf | 2 ++ .../nat-network-name-with-quotes.conf | 10 +++--- .../networkxml2confdata/netboot-network.conf | 4 +-- .../netboot-proxy-network.conf | 4 +-- tests/networkxml2conftest.c | 32 ++++++++----------- 8 files changed, 32 insertions(+), 37 deletions(-) diff --git a/tests/networkxml2confdata/isolated-network.conf b/tests/networkxml2confdata/isolated-network.conf index 693a83d9a0..ea66bb83e6 100644 --- a/tests/networkxml2confdata/isolated-network.conf +++ b/tests/networkxml2confdata/isolated-network.conf @@ -6,10 +6,11 @@ ## dnsmasq conf file created by libvirt strict-order except-interface=lo -bind-interfaces -listen-address=192.168.152.1 +bind-dynamic +interface=virbr2 dhcp-option=3 no-resolv +ra-param=*,0,0 dhcp-range=192.168.152.2,192.168.152.254,255.255.255.0 dhcp-no-override dhcp-authoritative diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index 0b2ca6f5aa..bd560ba3f4 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -6,12 +6,8 @@ ## dnsmasq conf file created by libvirt strict-order except-interface=lo -bind-interfaces -listen-address=192.168.122.1 -listen-address=192.168.123.1 -listen-address=fc00:db8:ac10:fe01::1 -listen-address=fc00:db8:ac10:fd01::1 -listen-address=10.24.10.1 +bind-dynamic +interface=virbr0 srv-host=_name._tcp dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0 dhcp-no-override @@ -19,3 +15,5 @@ dhcp-authoritative dhcp-lease-max=253 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts +dhcp-range=fc00:db8:ac10:fe01::1,ra-only +dhcp-range=fc00:db8:ac10:fd01::1,ra-only diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index a18c09aaa7..22bf3b1de9 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -21,3 +21,5 @@ dhcp-authoritative dhcp-lease-max=253 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts +dhcp-range=2001:db8:ac10:fe01::1,ra-only +dhcp-range=2001:db8:ac10:fd01::1,ra-only diff --git a/tests/networkxml2confdata/nat-network-dns-txt-record.conf b/tests/networkxml2confdata/nat-network-dns-txt-record.conf index 735c261c01..d9b981a6e5 100644 --- a/tests/networkxml2confdata/nat-network-dns-txt-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-txt-record.conf @@ -15,3 +15,5 @@ dhcp-authoritative dhcp-lease-max=253 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts +dhcp-range=2001:db8:ac10:fe01::1,ra-only +dhcp-range=2001:db8:ac10:fd01::1,ra-only diff --git a/tests/networkxml2confdata/nat-network-name-with-quotes.conf b/tests/networkxml2confdata/nat-network-name-with-quotes.conf index 1b06de3066..5c5ea7b48e 100644 --- a/tests/networkxml2confdata/nat-network-name-with-quotes.conf +++ b/tests/networkxml2confdata/nat-network-name-with-quotes.conf @@ -6,12 +6,8 @@ ## dnsmasq conf file created by libvirt strict-order except-interface=lo -bind-interfaces -listen-address=192.168.122.1 -listen-address=192.168.123.1 -listen-address=fc00:db8:ac10:fe01::1 -listen-address=fc00:db8:ac10:fd01::1 -listen-address=10.24.10.1 +bind-dynamic +interface=virbr0 srv-host=_name._tcp dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0 dhcp-no-override @@ -19,3 +15,5 @@ dhcp-authoritative dhcp-lease-max=253 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default"with"quotes".hostsfile addn-hosts=/var/lib/libvirt/dnsmasq/default"with"quotes".addnhosts +dhcp-range=fc00:db8:ac10:fe01::1,ra-only +dhcp-range=fc00:db8:ac10:fd01::1,ra-only diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf index 99272b9d68..a13239a54f 100644 --- a/tests/networkxml2confdata/netboot-network.conf +++ b/tests/networkxml2confdata/netboot-network.conf @@ -8,8 +8,8 @@ strict-order domain=example.com expand-hosts except-interface=lo -bind-interfaces -listen-address=192.168.122.1 +bind-dynamic +interface=virbr1 dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0 dhcp-no-override dhcp-authoritative diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf index fb0a20cff4..280da323e2 100644 --- a/tests/networkxml2confdata/netboot-proxy-network.conf +++ b/tests/networkxml2confdata/netboot-proxy-network.conf @@ -8,8 +8,8 @@ strict-order domain=example.com expand-hosts except-interface=lo -bind-interfaces -listen-address=192.168.122.1 +bind-dynamic +interface=virbr1 dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0 dhcp-no-override dhcp-authoritative diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 7177505918..178c74d9af 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -112,12 +112,8 @@ static int mymain(void) { int ret = 0; - dnsmasqCaps *restricted - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.48"); dnsmasqCaps *full - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.63\n--bind-dynamic"); - dnsmasqCaps *dhcpv6 - = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.64\n--bind-dynamic"); + = dnsmasqCapsNewFromBuffer("Dnsmasq version 2.67\n--bind-dynamic\n--ra-param"); #define DO_TEST(xname, xcaps) \ do { \ @@ -131,15 +127,15 @@ mymain(void) } \ } while (0) - DO_TEST("isolated-network", restricted); - DO_TEST("netboot-network", restricted); - DO_TEST("netboot-proxy-network", restricted); - DO_TEST("nat-network-dns-srv-record-minimal", restricted); - DO_TEST("nat-network-name-with-quotes", restricted); + DO_TEST("isolated-network", full); + DO_TEST("netboot-network", full); + DO_TEST("netboot-proxy-network", full); + DO_TEST("nat-network-dns-srv-record-minimal", full); + DO_TEST("nat-network-name-with-quotes", full); DO_TEST("routed-network", full); DO_TEST("routed-network-no-dns", full); DO_TEST("open-network", full); - DO_TEST("nat-network", dhcpv6); + DO_TEST("nat-network", full); DO_TEST("nat-network-dns-txt-record", full); DO_TEST("nat-network-dns-srv-record", full); DO_TEST("nat-network-dns-hosts", full); @@ -147,20 +143,18 @@ mymain(void) DO_TEST("nat-network-dns-forwarders", full); DO_TEST("nat-network-dns-forwarder-no-resolv", full); DO_TEST("nat-network-dns-local-domain", full); - DO_TEST("nat-network-mtu", dhcpv6); - DO_TEST("dhcp6-network", dhcpv6); - DO_TEST("dhcp6-nat-network", dhcpv6); - DO_TEST("dhcp6host-routed-network", dhcpv6); - DO_TEST("ptr-domains-auto", dhcpv6); - DO_TEST("dnsmasq-options", dhcpv6); + DO_TEST("nat-network-mtu", full); + DO_TEST("dhcp6-network", full); + DO_TEST("dhcp6-nat-network", full); + DO_TEST("dhcp6host-routed-network", full); + DO_TEST("ptr-domains-auto", full); + DO_TEST("dnsmasq-options", full); DO_TEST("leasetime-seconds", full); DO_TEST("leasetime-minutes", full); DO_TEST("leasetime-hours", full); DO_TEST("leasetime-infinite", full); - virObjectUnref(dhcpv6); virObjectUnref(full); - virObjectUnref(restricted); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.31.1
All the capabilities should be supported in 2.67. Make this the minimum version, since even the oldest distros we support have moved on: Debian 8: 2.72 CentOS 7: 2.76 Ubuntu 18.04: 2.79 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 90a1ea35b6..efe65174f8 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -49,6 +49,9 @@ VIR_LOG_INIT("util.dnsmasq"); #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" #define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts" +#define DNSMASQ_MIN_MAJOR 2 +#define DNSMASQ_MIN_MINOR 67 + static void dhcphostFreeContent(dnsmasqDhcpHost *host) { @@ -627,6 +630,16 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) if (virParseVersionString(p, &caps->version, true) < 0) goto error; + if (caps->version / 1000000 < DNSMASQ_MIN_MAJOR || + caps->version % 1000000 < DNSMASQ_MIN_MINOR) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("dnsmasq version >= %u.%u required but %lu.%lu found"), + DNSMASQ_MIN_MAJOR, DNSMASQ_MIN_MINOR, + caps->version / 1000000, + caps->version % 1000000); + goto error; + } + if (strstr(buf, "--bind-dynamic")) dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC); -- 2.31.1
On 12/14/21 2:09 PM, Ján Tomko wrote:
All the capabilities should be supported in 2.67. Make this the minimum version, since even the oldest distros we support have moved on:
Debian 8: 2.72 CentOS 7: 2.76 Ubuntu 18.04: 2.79
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 90a1ea35b6..efe65174f8 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -49,6 +49,9 @@ VIR_LOG_INIT("util.dnsmasq"); #define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile" #define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
+#define DNSMASQ_MIN_MAJOR 2 +#define DNSMASQ_MIN_MINOR 67 + static void dhcphostFreeContent(dnsmasqDhcpHost *host) { @@ -627,6 +630,16 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) if (virParseVersionString(p, &caps->version, true) < 0) goto error;
+ if (caps->version / 1000000 < DNSMASQ_MIN_MAJOR || + caps->version % 1000000 < DNSMASQ_MIN_MINOR) {
I think you actually want something like: if (caps->version < DNSMASQ_MIN_MAJOR * 1000000 + DNSMASQ_MIN_MINOR * 1000) (or if you wanted to avoid giving this file the knowledge of how version numbers are represented internally, you could #define DNSMASQ_MIN_VERSION "2.67", then use virParseVersionString() to parse that into an unsigned long, and then compare that result. That seems like overkill though)
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("dnsmasq version >= %u.%u required but %lu.%lu found"), + DNSMASQ_MIN_MAJOR, DNSMASQ_MIN_MINOR, + caps->version / 1000000, + caps->version % 1000000); + goto error; + } + if (strstr(buf, "--bind-dynamic")) dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC);
Remove the (now unreachable) error message and the macro. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 14 -------------- src/util/virdnsmasq.h | 6 ------ 2 files changed, 20 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 40dccf2c15..526485e3f9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1329,20 +1329,6 @@ networkDnsmasqConfContents(virNetworkObj *obj, } if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { if (ipdef->nranges || ipdef->nhosts) { - if (!DNSMASQ_DHCPv6_SUPPORT(caps)) { - unsigned long version = dnsmasqCapsGetVersion(caps); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The version of dnsmasq on this host " - "(%d.%d) doesn't adequately support " - "IPv6 dhcp range or dhcp host " - "specification. Version %d.%d or later " - "is required."), - (int)version / 1000000, - (int)(version % 1000000) / 1000, - DNSMASQ_DHCPv6_MAJOR_REQD, - DNSMASQ_DHCPv6_MINOR_REQD); - return -1; - } if (ipv6def) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("For IPv6, multiple DHCP definitions " diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index ee9839cd25..92c5d4129d 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -107,15 +107,9 @@ unsigned long dnsmasqCapsGetVersion(dnsmasqCaps *caps); char *dnsmasqDhcpHostsToString(dnsmasqDhcpHost *hosts, unsigned int nhosts); -#define DNSMASQ_DHCPv6_MAJOR_REQD 2 -#define DNSMASQ_DHCPv6_MINOR_REQD 64 #define DNSMASQ_RA_MAJOR_REQD 2 #define DNSMASQ_RA_MINOR_REQD 64 -#define DNSMASQ_DHCPv6_SUPPORT(CAPS) \ - (dnsmasqCapsGetVersion(CAPS) >= \ - (DNSMASQ_DHCPv6_MAJOR_REQD * 1000000) + \ - (DNSMASQ_DHCPv6_MINOR_REQD * 1000)) #define DNSMASQ_RA_SUPPORT(CAPS) \ (dnsmasqCapsGetVersion(CAPS) >= \ (DNSMASQ_RA_MAJOR_REQD * 1000000) + \ -- 2.31.1
Delete the code that is only run without the capability. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 134 +++++------------------------------- 1 file changed, 19 insertions(+), 115 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 526485e3f9..e57731742b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1486,21 +1486,18 @@ networkDnsmasqConfContents(virNetworkObj *obj, if (def->mtu > 0) virBufferAsprintf(&configbuf, "dhcp-option=option:mtu,%d\n", def->mtu); - /* Are we doing RA instead of radvd? */ - if (DNSMASQ_RA_SUPPORT(caps)) { - if (ipv6def) { - virBufferAddLit(&configbuf, "enable-ra\n"); - } else { - for (i = 0; - (ipdef = virNetworkDefGetIPByIndex(def, AF_INET6, i)); - i++) { - if (!(ipdef->nranges || ipdef->nhosts)) { - g_autofree char *bridgeaddr = virSocketAddrFormat(&ipdef->address); - if (!bridgeaddr) - return -1; - virBufferAsprintf(&configbuf, - "dhcp-range=%s,ra-only\n", bridgeaddr); - } + if (ipv6def) { + virBufferAddLit(&configbuf, "enable-ra\n"); + } else { + for (i = 0; + (ipdef = virNetworkDefGetIPByIndex(def, AF_INET6, i)); + i++) { + if (!(ipdef->nranges || ipdef->nhosts)) { + g_autofree char *bridgeaddr = virSocketAddrFormat(&ipdef->address); + if (!bridgeaddr) + return -1; + virBufferAsprintf(&configbuf, + "dhcp-range=%s,ra-only\n", bridgeaddr); } } } @@ -1860,84 +1857,11 @@ networkRadvdConfWrite(virNetworkDriverState *driver, static int -networkStartRadvd(virNetworkDriverState *driver, +networkStartRadvd(virNetworkDriverState *driver G_GNUC_UNUSED, virNetworkObj *obj) { - virNetworkDef *def = virNetworkObjGetDef(obj); - g_autoptr(dnsmasqCaps) dnsmasq_caps = networkGetDnsmasqCaps(driver); - pid_t radvdPid; - g_autofree char *pidfile = NULL; - g_autofree char *radvdpidbase = NULL; - g_autofree char *configfile = NULL; - g_autoptr(virCommand) cmd = NULL; - virNetworkObjSetRadvdPid(obj, -1); - /* Is dnsmasq handling RA? */ - if (DNSMASQ_RA_SUPPORT(dnsmasq_caps)) - return 0; - - if (!virNetworkDefGetIPByIndex(def, AF_INET6, 0)) { - /* no IPv6 addresses, so we don't need to run radvd */ - return 0; - } - - if (!virFileIsExecutable(RADVD)) { - virReportSystemError(errno, - _("Cannot find %s - " - "Possibly the package isn't installed"), - RADVD); - return -1; - } - - if (g_mkdir_with_parents(driver->pidDir, 0777) < 0) { - virReportSystemError(errno, - _("cannot create directory %s"), - driver->pidDir); - return -1; - } - - if (g_mkdir_with_parents(driver->radvdStateDir, 0777) < 0) { - virReportSystemError(errno, - _("cannot create directory %s"), - driver->radvdStateDir); - return -1; - } - - /* construct pidfile name */ - if (!(radvdpidbase = networkRadvdPidfileBasename(def->name))) - return -1; - - if (!(pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) - return -1; - - if (networkRadvdConfWrite(driver, obj, &configfile) < 0) - return -1; - - /* prevent radvd from daemonizing itself with "--debug 1", and use - * a dummy pidfile name - virCommand will create the pidfile we - * want to use (this is necessary because radvd's internal - * daemonization and pidfile creation causes a race, and the - * virPidFileRead() below will fail if we use them). - * Unfortunately, it isn't possible to tell radvd to not create - * its own pidfile, so we just let it do so, with a slightly - * different name. Unused, but harmless. - */ - cmd = virCommandNewArgList(RADVD, "--debug", "1", - "--config", configfile, - "--pidfile", NULL); - virCommandAddArgFormat(cmd, "%s-bin", pidfile); - - virCommandSetPidFile(cmd, pidfile); - virCommandDaemonize(cmd); - - if (virCommandRun(cmd, NULL) < 0) - return -1; - - if (virPidFileRead(driver->pidDir, radvdpidbase, &radvdPid) < 0) - return -1; - - virNetworkObjSetRadvdPid(obj, radvdPid); return 0; } @@ -1947,36 +1871,16 @@ networkRefreshRadvd(virNetworkDriverState *driver, virNetworkObj *obj) { virNetworkDef *def = virNetworkObjGetDef(obj); - g_autoptr(dnsmasqCaps) dnsmasq_caps = networkGetDnsmasqCaps(driver); g_autofree char *radvdpidbase = NULL; g_autofree char *pidfile = NULL; - pid_t radvdPid; - /* Is dnsmasq handling RA? */ - if (DNSMASQ_RA_SUPPORT(dnsmasq_caps)) { - if ((radvdpidbase = networkRadvdPidfileBasename(def->name)) && - (pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) { - /* radvd should not be running but in case it is */ - virPidFileForceCleanupPath(pidfile); - virNetworkObjSetRadvdPid(obj, -1); - } - return 0; + if ((radvdpidbase = networkRadvdPidfileBasename(def->name)) && + (pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) { + /* radvd should not be running but in case it is */ + virPidFileForceCleanupPath(pidfile); + virNetworkObjSetRadvdPid(obj, -1); } - - /* if there's no running radvd, just start it */ - radvdPid = virNetworkObjGetRadvdPid(obj); - if (radvdPid <= 0 || (kill(radvdPid, 0) < 0)) - return networkStartRadvd(driver, obj); - - if (!virNetworkDefGetIPByIndex(def, AF_INET6, 0)) { - /* no IPv6 addresses, so we don't need to run radvd */ - return 0; - } - - if (networkRadvdConfWrite(driver, obj, NULL) < 0) - return -1; - - return kill(radvdPid, SIGHUP); + return 0; } -- 2.31.1
Now that the macro is unused, delete it. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 92c5d4129d..9b8aeef226 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -106,11 +106,3 @@ const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); unsigned long dnsmasqCapsGetVersion(dnsmasqCaps *caps); char *dnsmasqDhcpHostsToString(dnsmasqDhcpHost *hosts, unsigned int nhosts); - -#define DNSMASQ_RA_MAJOR_REQD 2 -#define DNSMASQ_RA_MINOR_REQD 64 - -#define DNSMASQ_RA_SUPPORT(CAPS) \ - (dnsmasqCapsGetVersion(CAPS) >= \ - (DNSMASQ_RA_MAJOR_REQD * 1000000) + \ - (DNSMASQ_RA_MINOR_REQD * 1000)) -- 2.31.1
Introduced by dnsmasq commit: commit 54dd393f3938fc0c19088fbd319b95e37d81a2b0 CommitDate: 2012-06-20 11:23:38 +0100 Add --bind-dynamic git describe: v2.63test1 contains: v2.63test1^0 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 68 ++++++------------------------------- 1 file changed, 11 insertions(+), 57 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e57731742b..dffe4e1574 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1062,7 +1062,6 @@ networkDnsmasqConfContents(virNetworkObj *obj, size_t i; virNetworkDNSDef *dns = &def->dns; bool wantDNS = dns->enable != VIR_TRISTATE_BOOL_NO; - virNetworkIPDef *tmpipdef; virNetworkIPDef *ipdef; virNetworkIPDef *ipv4def; virNetworkIPDef *ipv6def; @@ -1173,62 +1172,17 @@ networkDnsmasqConfContents(virNetworkObj *obj, virBufferAddLit(&configbuf, "except-interface=lo0\n"); #endif - if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) { - /* using --bind-dynamic with only --interface (no - * --listen-address) prevents dnsmasq from responding to dns - * queries that arrive on some interface other than our bridge - * interface (in other words, requests originating somewhere - * other than one of the virtual guests connected directly to - * this network). This was added in response to CVE 2012-3411. - */ - virBufferAsprintf(&configbuf, - "bind-dynamic\n" - "interface=%s\n", - def->bridge); - } else { - virBufferAddLit(&configbuf, "bind-interfaces\n"); - /* - * --interface does not actually work with dnsmasq < 2.47, - * due to DAD for ipv6 addresses on the interface. - * - * virCommandAddArgList(cmd, "--interface", def->bridge, NULL); - * - * So listen on all defined IPv[46] addresses - */ - for (i = 0; - (tmpipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i)); - i++) { - g_autofree char *ipaddr = virSocketAddrFormat(&tmpipdef->address); - - if (!ipaddr) - return -1; - - /* also part of CVE 2012-3411 - if the host's version of - * dnsmasq doesn't have bind-dynamic, only allow listening on - * private/local IP addresses (see RFC1918/RFC3484/RFC4193) - */ - if (!dnsmasqCapsGet(caps, DNSMASQ_CAPS_BINDTODEVICE) && - !virSocketAddrIsPrivate(&tmpipdef->address)) { - unsigned long version = dnsmasqCapsGetVersion(caps); - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Publicly routable address %s is prohibited. " - "The version of dnsmasq on this host (%d.%d) " - "doesn't support the bind-dynamic option or " - "use SO_BINDTODEVICE on listening sockets, " - "one of which is required for safe operation " - "on a publicly routable subnet " - "(see CVE-2012-3411). You must either " - "upgrade dnsmasq, or use a private/local " - "subnet range for this network " - "(as described in RFC1918/RFC3484/RFC4193)."), - ipaddr, (int)version / 1000000, - (int)(version % 1000000) / 1000); - return -1; - } - virBufferAsprintf(&configbuf, "listen-address=%s\n", ipaddr); - } - } + /* using --bind-dynamic with only --interface (no + * --listen-address) prevents dnsmasq from responding to dns + * queries that arrive on some interface other than our bridge + * interface (in other words, requests originating somewhere + * other than one of the virtual guests connected directly to + * this network). This was added in response to CVE 2012-3411. + */ + virBufferAsprintf(&configbuf, + "bind-dynamic\n" + "interface=%s\n", + def->bridge); /* If this is an isolated network, set the default route option * (3) to be empty to avoid setting a default route that's -- 2.31.1
Introduced by: commit c4cd95df68b573b63d234ecdb675228657d65353 Author: Simon Kelley <simon@thekelleys.org.uk> CommitDate: 2013-10-10 20:58:11 +0100 Add --ra-param and remove --force-fast-ra git describe: v2.67rc3-3-gc4cd95d contains: v2.67rc4~12 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index dffe4e1574..a4535b1b49 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1197,10 +1197,8 @@ networkDnsmasqConfContents(virNetworkObj *obj, if (def->forward.type == VIR_NETWORK_FORWARD_NONE) { virBufferAddLit(&configbuf, "dhcp-option=3\n" "no-resolv\n"); - if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_RA_PARAM)) { - /* interface=* (any), interval=0 (default), lifetime=0 (seconds) */ - virBufferAddLit(&configbuf, "ra-param=*,0,0\n"); - } + /* interface=* (any), interval=0 (default), lifetime=0 (seconds) */ + virBufferAddLit(&configbuf, "ra-param=*,0,0\n"); } if (wantDNS) { -- 2.31.1
On 12/14/21 2:09 PM, Ján Tomko wrote:
Introduced by:
"Introduced by dnsmasq commit:"
commit c4cd95df68b573b63d234ecdb675228657d65353 Author: Simon Kelley <simon@thekelleys.org.uk> CommitDate: 2013-10-10 20:58:11 +0100
Add --ra-param and remove --force-fast-ra
git describe: v2.67rc3-3-gc4cd95d contains: v2.67rc4~12
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index dffe4e1574..a4535b1b49 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1197,10 +1197,8 @@ networkDnsmasqConfContents(virNetworkObj *obj, if (def->forward.type == VIR_NETWORK_FORWARD_NONE) { virBufferAddLit(&configbuf, "dhcp-option=3\n" "no-resolv\n"); - if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_RA_PARAM)) { - /* interface=* (any), interval=0 (default), lifetime=0 (seconds) */ - virBufferAddLit(&configbuf, "ra-param=*,0,0\n"); - } + /* interface=* (any), interval=0 (default), lifetime=0 (seconds) */ + virBufferAddLit(&configbuf, "ra-param=*,0,0\n"); }
if (wantDNS) {
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.c | 22 ++-------------------- src/util/virdnsmasq.h | 4 ---- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index efe65174f8..016d9d64a8 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -640,27 +640,9 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) goto error; } - if (strstr(buf, "--bind-dynamic")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC); - - /* if this string is a part of the --version output, dnsmasq - * has been patched to use SO_BINDTODEVICE when listening, - * so that it will only accept requests that arrived on the - * listening interface(s) - */ - if (strstr(buf, "--bind-interfaces with SO_BINDTODEVICE")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_BINDTODEVICE); - - if (strstr(buf, "--ra-param")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_RA_PARAM); - - VIR_INFO("dnsmasq version is %d.%d, --bind-dynamic is %spresent, " - "SO_BINDTODEVICE is %sin use, --ra-param is %spresent", + VIR_INFO("dnsmasq version is %d.%d", (int)caps->version / 1000000, - (int)(caps->version % 1000000) / 1000, - dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC) ? "" : "NOT ", - dnsmasqCapsGet(caps, DNSMASQ_CAPS_BINDTODEVICE) ? "" : "NOT ", - dnsmasqCapsGet(caps, DNSMASQ_CAPS_RA_PARAM) ? "" : "NOT "); + (int)(caps->version % 1000000) / 1000); return 0; error: diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 9b8aeef226..9aa45c3046 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -68,10 +68,6 @@ typedef struct } dnsmasqContext; typedef enum { - DNSMASQ_CAPS_BIND_DYNAMIC = 0, /* support for --bind-dynamic */ - DNSMASQ_CAPS_BINDTODEVICE = 1, /* uses SO_BINDTODEVICE for --bind-interfaces */ - DNSMASQ_CAPS_RA_PARAM = 2, /* support for --ra-param */ - DNSMASQ_CAPS_LAST, /* this must always be the last item */ } dnsmasqCapsFlags; -- 2.31.1
On 12/14/21 2:09 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virdnsmasq.c | 22 ++-------------------- src/util/virdnsmasq.h | 4 ---- 2 files changed, 2 insertions(+), 24 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index efe65174f8..016d9d64a8 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -640,27 +640,9 @@ dnsmasqCapsSetFromBuffer(dnsmasqCaps *caps, const char *buf) goto error; }
- if (strstr(buf, "--bind-dynamic")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_BIND_DYNAMIC); - - /* if this string is a part of the --version output, dnsmasq - * has been patched to use SO_BINDTODEVICE when listening, - * so that it will only accept requests that arrived on the - * listening interface(s) - */ - if (strstr(buf, "--bind-interfaces with SO_BINDTODEVICE")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_BINDTODEVICE); - - if (strstr(buf, "--ra-param")) - dnsmasqCapsSet(caps, DNSMASQ_CAPS_RA_PARAM); - - VIR_INFO("dnsmasq version is %d.%d, --bind-dynamic is %spresent, " - "SO_BINDTODEVICE is %sin use, --ra-param is %spresent", + VIR_INFO("dnsmasq version is %d.%d", (int)caps->version / 1000000, - (int)(caps->version % 1000000) / 1000, - dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC) ? "" : "NOT ", - dnsmasqCapsGet(caps, DNSMASQ_CAPS_BINDTODEVICE) ? "" : "NOT ", - dnsmasqCapsGet(caps, DNSMASQ_CAPS_RA_PARAM) ? "" : "NOT "); + (int)(caps->version % 1000000) / 1000);
One would hope that nobody is actually looking for these strings in a script anywhere :-/ (To clarify - I think it's fine to remove).
Since dnsmasq supports --ra-param for a long time, this code is now unused. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 248 +----------------------------------- 1 file changed, 6 insertions(+), 242 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a4535b1b49..39f6ed14e1 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -371,20 +371,6 @@ networkDnsmasqConfigFileName(virNetworkDriverState *driver, } -static char * -networkRadvdPidfileBasename(const char *netname) -{return g_strdup_printf("%s-radvd", netname); -} - - -static char * -networkRadvdConfigFileName(virNetworkDriverState *driver, - const char *netname) -{ - return g_strdup_printf("%s/%s-radvd.conf", driver->radvdStateDir, netname); -} - - /* do needed cleanup steps and remove the network from the list */ static int networkRemoveInactive(virNetworkDriverState *driver, @@ -392,15 +378,13 @@ networkRemoveInactive(virNetworkDriverState *driver, { g_autofree char *leasefile = NULL; g_autofree char *customleasefile = NULL; - g_autofree char *radvdconfigfile = NULL; g_autofree char *configfile = NULL; - g_autofree char *radvdpidbase = NULL; g_autofree char *statusfile = NULL; g_autofree char *macMapFile = NULL; g_autoptr(dnsmasqContext) dctx = NULL; virNetworkDef *def = virNetworkObjGetPersistentDef(obj); - /* remove the (possibly) existing dnsmasq and radvd files */ + /* remove the (possibly) existing dnsmasq files */ if (!(dctx = dnsmasqContextNew(def->name, driver->dnsmasqStateDir))) { return -1; @@ -412,12 +396,6 @@ networkRemoveInactive(virNetworkDriverState *driver, if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(driver, def->bridge))) return -1; - if (!(radvdconfigfile = networkRadvdConfigFileName(driver, def->name))) - return -1; - - if (!(radvdpidbase = networkRadvdPidfileBasename(def->name))) - return -1; - if (!(configfile = networkDnsmasqConfigFileName(driver, def->name))) return -1; @@ -436,10 +414,6 @@ networkRemoveInactive(virNetworkDriverState *driver, /* MAC map manager */ unlink(macMapFile); - /* radvd */ - unlink(radvdconfigfile); - virPidFileDelete(driver->pidDir, radvdpidbase); - /* remove status file */ unlink(statusfile); @@ -556,26 +530,15 @@ networkUpdateState(virNetworkObj *obj, virNetworkObjPortForEach(obj, networkUpdatePort, obj); - /* Try and read dnsmasq/radvd pids of active networks */ + /* Try and read dnsmasq pids of active networks */ if (virNetworkObjIsActive(obj) && def->ips && (def->nips > 0)) { - pid_t radvdPid; pid_t dnsmasqPid; - g_autofree char *radvdpidbase = NULL; ignore_value(virPidFileReadIfAlive(driver->pidDir, def->name, &dnsmasqPid, dnsmasqCapsGetBinaryPath(dnsmasq_caps))); virNetworkObjSetDnsmasqPid(obj, dnsmasqPid); - - radvdpidbase = networkRadvdPidfileBasename(def->name); - if (!radvdpidbase) - goto cleanup; - - ignore_value(virPidFileReadIfAlive(driver->pidDir, - radvdpidbase, - &radvdPid, RADVD)); - virNetworkObjSetRadvdPid(obj, radvdPid); } ret = 0; @@ -690,7 +653,6 @@ networkStateInitialize(bool privileged, network_driver->stateDir = g_strdup(RUNSTATEDIR "/libvirt/network"); network_driver->pidDir = g_strdup(RUNSTATEDIR "/libvirt/network"); network_driver->dnsmasqStateDir = g_strdup(LOCALSTATEDIR "/lib/libvirt/dnsmasq"); - network_driver->radvdStateDir = g_strdup(LOCALSTATEDIR "/lib/libvirt/radvd"); } else { configdir = virGetUserConfigDirectory(); rundir = virGetUserRuntimeDirectory(); @@ -700,7 +662,6 @@ networkStateInitialize(bool privileged, network_driver->stateDir = g_strdup_printf("%s/network/lib", rundir); network_driver->pidDir = g_strdup_printf("%s/network/run", rundir); network_driver->dnsmasqStateDir = g_strdup_printf("%s/dnsmasq/lib", rundir); - network_driver->radvdStateDir = g_strdup_printf("%s/radvd/lib", rundir); } if (g_mkdir_with_parents(network_driver->stateDir, 0777) < 0) { @@ -847,7 +808,6 @@ networkStateCleanup(void) g_free(network_driver->stateDir); g_free(network_driver->pidDir); g_free(network_driver->dnsmasqStateDir); - g_free(network_driver->radvdStateDir); virObjectUnref(network_driver->dnsmasqCaps); @@ -1697,170 +1657,6 @@ networkRestartDhcpDaemon(virNetworkDriverState *driver, } -static char radvd1[] = " AdvOtherConfigFlag off;\n\n"; -static char radvd2[] = " AdvAutonomous off;\n"; -static char radvd3[] = " AdvOnLink on;\n" - " AdvAutonomous on;\n" - " AdvRouterAddr off;\n"; - -static int -networkRadvdConfContents(virNetworkObj *obj, - char **configstr) -{ - virNetworkDef *def = virNetworkObjGetDef(obj); - g_auto(virBuffer) configbuf = VIR_BUFFER_INITIALIZER; - size_t i; - virNetworkIPDef *ipdef; - bool v6present = false, dhcp6 = false; - - *configstr = NULL; - - /* Check if DHCPv6 is needed */ - for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_INET6, i)); i++) { - v6present = true; - if (ipdef->nranges || ipdef->nhosts) { - dhcp6 = true; - break; - } - } - - /* If there are no IPv6 addresses, then we are done */ - if (!v6present) - return 0; - - /* create radvd config file appropriate for this network; - * IgnoreIfMissing allows radvd to start even when the bridge is down - */ - virBufferAsprintf(&configbuf, "interface %s\n" - "{\n" - " AdvSendAdvert on;\n" - " IgnoreIfMissing on;\n" - " AdvManagedFlag %s;\n" - "%s", - def->bridge, - dhcp6 ? "on" : "off", - dhcp6 ? "\n" : radvd1); - - /* add a section for each IPv6 address in the config */ - for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_INET6, i)); i++) { - int prefix; - g_autofree char *netaddr = NULL; - - prefix = virNetworkIPDefPrefix(ipdef); - if (prefix < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("bridge '%s' has an invalid prefix"), - def->bridge); - return -1; - } - if (!(netaddr = virSocketAddrFormat(&ipdef->address))) - return -1; - - virBufferAsprintf(&configbuf, - " prefix %s/%d\n" - " {\n%s };\n", - netaddr, prefix, - dhcp6 ? radvd2 : radvd3); - } - - virBufferAddLit(&configbuf, "};\n"); - - *configstr = virBufferContentAndReset(&configbuf); - - return 0; -} - - -/* write file and return its name (which must be freed by caller) */ -static int -networkRadvdConfWrite(virNetworkDriverState *driver, - virNetworkObj *obj, - char **configFile) -{ - virNetworkDef *def = virNetworkObjGetDef(obj); - g_autofree char *configStr = NULL; - g_autofree char *myConfigFile = NULL; - - if (!configFile) - configFile = &myConfigFile; - - *configFile = NULL; - - if (networkRadvdConfContents(obj, &configStr) < 0) - return -1; - - if (!configStr) - return 0; - - /* construct the filename */ - if (!(*configFile = networkRadvdConfigFileName(driver, def->name))) - return -1; - - /* write the file */ - if (virFileWriteStr(*configFile, configStr, 0600) < 0) { - virReportSystemError(errno, - _("couldn't write radvd config file '%s'"), - *configFile); - return -1; - } - - return 0; -} - - -static int -networkStartRadvd(virNetworkDriverState *driver G_GNUC_UNUSED, - virNetworkObj *obj) -{ - virNetworkObjSetRadvdPid(obj, -1); - - return 0; -} - - -static int -networkRefreshRadvd(virNetworkDriverState *driver, - virNetworkObj *obj) -{ - virNetworkDef *def = virNetworkObjGetDef(obj); - g_autofree char *radvdpidbase = NULL; - g_autofree char *pidfile = NULL; - - if ((radvdpidbase = networkRadvdPidfileBasename(def->name)) && - (pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) { - /* radvd should not be running but in case it is */ - virPidFileForceCleanupPath(pidfile); - virNetworkObjSetRadvdPid(obj, -1); - } - return 0; -} - - -#if 0 -/* currently unused, so it causes a build error unless we #if it out */ -static int -networkRestartRadvd(virNetworkObj *obj) -{ - virNetworkDef *def = virNetworkObjGetDef(obj); - g_autofree char *radvdpidbase = NULL; - g_autofree char *pidfile = NULL; - - /* If there is a running radvd, kill it. Essentially ignore errors from the - * following two functions, since there's really no better recovery to be - * done than to just push ahead (and that may be exactly what's needed). - */ - if ((radvdpidbase = networkRadvdPidfileBasename(def->name)) && - (pidfile = virPidFileBuildPath(driver->pidDir, radvdpidbase))) { - virPidFileForceCleanupPath(pidfile); - virNetworkObjSetRadvdPid(obj, -1); - } - - /* now start radvd if it should be started */ - return networkStartRadvd(obj); -} -#endif /* #if 0 */ - - static int networkRefreshDaemonsHelper(virNetworkObj *obj, void *opaque) @@ -1877,13 +1673,11 @@ networkRefreshDaemonsHelper(virNetworkObj *obj, case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_OPEN: /* Only the three L3 network types that are configured by - * libvirt will have a dnsmasq or radvd daemon associated + * libvirt will have a dnsmasq daemon associated * with them. Here we send a SIGHUP to an existing - * dnsmasq and/or radvd, or restart them if they've - * disappeared. + * dnsmasq, or restart it if it has disappeared. */ networkRefreshDhcpDaemon(driver, obj); - networkRefreshRadvd(driver, obj); break; case VIR_NETWORK_FORWARD_BRIDGE: @@ -1906,7 +1700,7 @@ networkRefreshDaemonsHelper(virNetworkObj *obj, } -/* SIGHUP/restart any dnsmasq or radvd daemons. +/* SIGHUP/restart any dnsmasq * This should be called when libvirtd is restarted. */ static void @@ -2263,10 +2057,6 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, dnsmasqStarted = true; - /* start radvd if there are any ipv6 addresses */ - if (v6present && networkStartRadvd(driver, obj) < 0) - goto error; - if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; @@ -2300,28 +2090,16 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, static int -networkShutdownNetworkVirtual(virNetworkDriverState *driver, +networkShutdownNetworkVirtual(virNetworkDriverState *driver G_GNUC_UNUSED, virNetworkObj *obj) { virNetworkDef *def = virNetworkObjGetDef(obj); - pid_t radvdPid; pid_t dnsmasqPid; if (def->bandwidth) virNetDevBandwidthClear(def->bridge); virNetworkObjUnrefMacMap(obj); - - radvdPid = virNetworkObjGetRadvdPid(obj); - if (radvdPid > 0) { - g_autofree char *radvdpidbase = NULL; - - kill(radvdPid, SIGTERM); - /* attempt to delete the pidfile we created */ - if ((radvdpidbase = networkRadvdPidfileBasename(def->name))) - virPidFileDelete(driver->pidDir, radvdpidbase); - } - dnsmasqPid = virNetworkObjGetDnsmasqPid(obj); if (dnsmasqPid > 0) kill(dnsmasqPid, SIGTERM); @@ -2350,12 +2128,6 @@ networkShutdownNetworkVirtual(virNetworkDriverState *driver, kill(dnsmasqPid, SIGKILL); virNetworkObjSetDnsmasqPid(obj, -1); - radvdPid = virNetworkObjGetRadvdPid(obj); - if (radvdPid > 0 && - (kill(radvdPid, 0) == 0)) - kill(radvdPid, SIGKILL); - virNetworkObjSetRadvdPid(obj, -1); - return 0; } @@ -3634,14 +3406,6 @@ networkUpdate(virNetworkPtr net, } - if (section == VIR_NETWORK_SECTION_IP) { - /* only a change in IP addresses will affect radvd, and all of radvd's - * config is stored in the conf file which will be re-read with a SIGHUP. - */ - if (networkRefreshRadvd(driver, obj) < 0) - goto cleanup; - } - /* save current network state to disk */ if ((ret = virNetworkObjSaveStatus(driver->stateDir, obj, network_driver->xmlopt)) < 0) -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver_platform.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index 884fa82831..de7cbc1195 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -46,7 +46,6 @@ struct _virNetworkDriverState { char *stateDir; char *pidDir; char *dnsmasqStateDir; - char *radvdStateDir; /* Require lock to get a reference on the object, * lockless access thereafter -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/virnetworkobj.c | 16 ---------------- src/conf/virnetworkobj.h | 7 ------- src/libvirt_private.syms | 2 -- 3 files changed, 25 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 41c7dcba5c..f18eb35ae2 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -43,7 +43,6 @@ struct _virNetworkObj { virObjectLockable parent; pid_t dnsmasqPid; - pid_t radvdPid; bool active; bool autostart; bool persistent; @@ -211,21 +210,6 @@ virNetworkObjSetDnsmasqPid(virNetworkObj *obj, } -pid_t -virNetworkObjGetRadvdPid(virNetworkObj *obj) -{ - return obj->radvdPid; -} - - -void -virNetworkObjSetRadvdPid(virNetworkObj *obj, - pid_t radvdPid) -{ - obj->radvdPid = radvdPid; -} - - virBitmap * virNetworkObjGetClassIdMap(virNetworkObj *obj) { diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h index d980e9f38d..fadd277cbd 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -66,13 +66,6 @@ void virNetworkObjSetDnsmasqPid(virNetworkObj *obj, pid_t dnsmasqPid); -pid_t -virNetworkObjGetRadvdPid(virNetworkObj *obj); - -void -virNetworkObjSetRadvdPid(virNetworkObj *obj, - pid_t radvdPid); - virBitmap * virNetworkObjGetClassIdMap(virNetworkObj *obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f5a816b002..36f826e2ed 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1248,7 +1248,6 @@ virNetworkObjGetMacMap; virNetworkObjGetNewDef; virNetworkObjGetPersistentDef; virNetworkObjGetPortStatusDir; -virNetworkObjGetRadvdPid; virNetworkObjIsActive; virNetworkObjIsAutostart; virNetworkObjIsPersistent; @@ -1276,7 +1275,6 @@ virNetworkObjSetDefTransient; virNetworkObjSetDnsmasqPid; virNetworkObjSetFloorSum; virNetworkObjSetMacMap; -virNetworkObjSetRadvdPid; virNetworkObjTaint; virNetworkObjUnrefMacMap; virNetworkObjUnsetDefTransient; -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/meson.build b/meson.build index cea8bbfa0c..0b7a2a69c1 100644 --- a/meson.build +++ b/meson.build @@ -860,7 +860,6 @@ optional_programs = [ 'modprobe', 'ovs-vsctl', 'pdwtags', - 'radvd', 'rmmod', 'scrub', 'tc', -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- libvirt.spec.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 32b4243d0a..b37c6e17f3 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -283,7 +283,6 @@ BuildRequires: libnl3-devel BuildRequires: libselinux-devel BuildRequires: dnsmasq >= 2.41 BuildRequires: iptables -BuildRequires: radvd BuildRequires: ebtables BuildRequires: module-init-tools BuildRequires: cyrus-sasl-devel @@ -464,7 +463,6 @@ Summary: Network driver plugin for the libvirtd daemon Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} Requires: dnsmasq >= 2.41 -Requires: radvd Requires: iptables %description daemon-driver-network -- 2.31.1
It has no callers anymore. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 9 --------- src/util/virdnsmasq.h | 1 - 3 files changed, 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 36f826e2ed..8ed50d1c45 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2123,7 +2123,6 @@ dnsmasqAddDhcpHost; dnsmasqAddHost; dnsmasqCapsGet; dnsmasqCapsGetBinaryPath; -dnsmasqCapsGetVersion; dnsmasqCapsNewFromBinary; dnsmasqCapsNewFromBuffer; dnsmasqContextFree; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 016d9d64a8..d086647362 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -759,15 +759,6 @@ dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps) return caps ? caps->binaryPath : DNSMASQ; } -unsigned long -dnsmasqCapsGetVersion(dnsmasqCaps *caps) -{ - if (caps) - return caps->version; - else - return 0; -} - bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag) { diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 9aa45c3046..10b512cff4 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -99,6 +99,5 @@ dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf); dnsmasqCaps *dnsmasqCapsNewFromBinary(void); bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); -unsigned long dnsmasqCapsGetVersion(dnsmasqCaps *caps); char *dnsmasqDhcpHostsToString(dnsmasqDhcpHost *hosts, unsigned int nhosts); -- 2.31.1
Now that we only check whether the dnsmasq version is new enough, there is no need for the caps field. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virdnsmasq.c | 17 ----------------- src/util/virdnsmasq.h | 5 ----- 3 files changed, 23 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8ed50d1c45..e6639f7644 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2121,7 +2121,6 @@ virIsDevMapperDevice; # util/virdnsmasq.h dnsmasqAddDhcpHost; dnsmasqAddHost; -dnsmasqCapsGet; dnsmasqCapsGetBinaryPath; dnsmasqCapsNewFromBinary; dnsmasqCapsNewFromBuffer; diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index d086647362..1edc2d711b 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -578,7 +578,6 @@ struct _dnsmasqCaps { char *binaryPath; bool noRefresh; time_t mtime; - virBitmap *flags; unsigned long version; }; @@ -589,7 +588,6 @@ dnsmasqCapsDispose(void *obj) { dnsmasqCaps *caps = obj; - virBitmapFree(caps->flags); g_free(caps->binaryPath); } @@ -603,13 +601,6 @@ static int dnsmasqCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(dnsmasqCaps); -static void -dnsmasqCapsSet(dnsmasqCaps *caps, - dnsmasqCapsFlags flag) -{ - ignore_value(virBitmapSetBit(caps->flags, flag)); -} - #define DNSMASQ_VERSION_STR "Dnsmasq version " @@ -718,7 +709,6 @@ dnsmasqCapsNewEmpty(const char *binaryPath) return NULL; if (!(caps = virObjectNew(dnsmasqCapsClass))) return NULL; - caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST); caps->binaryPath = g_strdup(binaryPath ? binaryPath : DNSMASQ); return caps; } @@ -759,13 +749,6 @@ dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps) return caps ? caps->binaryPath : DNSMASQ; } -bool -dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag) -{ - return caps && virBitmapIsBitSet(caps->flags, flag); -} - - /** dnsmasqDhcpHostsToString: * * Turns a vector of dnsmasqDhcpHost into the string that is ought to be diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 10b512cff4..c74cc887f8 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -67,10 +67,6 @@ typedef struct dnsmasqAddnHostsfile *addnhostsfile; } dnsmasqContext; -typedef enum { - DNSMASQ_CAPS_LAST, /* this must always be the last item */ -} dnsmasqCapsFlags; - typedef struct _dnsmasqCaps dnsmasqCaps; G_DEFINE_AUTOPTR_CLEANUP_FUNC(dnsmasqCaps, virObjectUnref); @@ -97,7 +93,6 @@ int dnsmasqReload(pid_t pid); dnsmasqCaps *dnsmasqCapsNewFromBuffer(const char *buf); dnsmasqCaps *dnsmasqCapsNewFromBinary(void); -bool dnsmasqCapsGet(dnsmasqCaps *caps, dnsmasqCapsFlags flag); const char *dnsmasqCapsGetBinaryPath(dnsmasqCaps *caps); char *dnsmasqDhcpHostsToString(dnsmasqDhcpHost *hosts, unsigned int nhosts); -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/network/bridge_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 39f6ed14e1..23d9ed4226 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2090,8 +2090,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, static int -networkShutdownNetworkVirtual(virNetworkDriverState *driver G_GNUC_UNUSED, - virNetworkObj *obj) +networkShutdownNetworkVirtual(virNetworkObj *obj) { virNetworkDef *def = virNetworkObjGetDef(obj); pid_t dnsmasqPid; @@ -2419,7 +2418,7 @@ networkShutdownNetwork(virNetworkDriverState *driver, case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_OPEN: - ret = networkShutdownNetworkVirtual(driver, obj); + ret = networkShutdownNetworkVirtual(obj); break; case VIR_NETWORK_FORWARD_BRIDGE: -- 2.31.1
On 12/14/21 2:09 PM, Ján Tomko wrote:
This bumps the minimum dnsmasq version to the point where we do not need capability probing, reducing it to a version check (which I will be happy to remove on request).
Unless I missed something, this also means we no longer need to spawn radvd manually.
The code doesn't lie! If removing the bits that were only true for older dnsmasq removed the lines that ran radvd, then it's true. (I recall that support for RA was added to dnsmasq fairly soon after the original ipv6 support was added, and radvd was left in libvirt only because there were so many downstreams that still had an older dnsmasq).
Note that DNSMASQ_CAPS_BINDTODEVICE was the indication of a downstream mitigation of a CVE that should no longer be needed if we have --bind-dynamic
[...]
17 files changed, 83 insertions(+), 569 deletions(-)
Nice!!! After the minor fixes I noted in 03/17 and 08/17 Reviewed-by: Laine Stump <laine@redhat.com> /me ponders what I should idly suggest be removed next...
On 12/14/21 21:06, Laine Stump wrote:
On 12/14/21 2:09 PM, Ján Tomko wrote:
This bumps the minimum dnsmasq version to the point where we do not need capability probing, reducing it to a version check (which I will be happy to remove on request).
Unless I missed something, this also means we no longer need to spawn radvd manually.
The code doesn't lie! If removing the bits that were only true for older dnsmasq removed the lines that ran radvd, then it's true. (I recall that support for RA was added to dnsmasq fairly soon after the original ipv6 support was added, and radvd was left in libvirt only because there were so many downstreams that still had an older dnsmasq).
Note that DNSMASQ_CAPS_BINDTODEVICE was the indication of a downstream mitigation of a CVE that should no longer be needed if we have --bind-dynamic
[...]
17 files changed, 83 insertions(+), 569 deletions(-)
Nice!!!
After the minor fixes I noted in 03/17 and 08/17
Reviewed-by: Laine Stump <laine@redhat.com>
/me ponders what I should idly suggest be removed next...
Parallels driver (src/vz/)? ;-) Michal
participants (3)
-
Ján Tomko -
Laine Stump -
Michal Prívozník