[libvirt] [PATCH] network: prevent dnsmasq from listening on localhost
This patch resolves the problem reported in: https://bugzilla.redhat.com/show_bug.cgi?id=886663 The source of the problem was the fix for CVE 2011-3411: https://bugzilla.redhat.com/show_bug.cgi?id=833033 which was originally committed upstream in commit 753ff83a50263d6975f88d6605d4b5ddfcc97560. That commit improperly removed the "--except-interface lo" from dnsmasq commandlines when --bind-dynamic was used (based on comments in the latter bug). It turns out that the problem reported in the CVE could be eliminated without removing "--except-interface lo", and removing it actually caused each instance of dnsmasq to listen on localhost on port 53, which created a new problem: If another instance of dnsmasq using "bind-interfaces" (instead of "bind-dynamic") had already been started (or if another instance started later used "bind-dynamic"), this wouldn't have any immediately visible ill effects, but if you tried to start another dnsmasq instance using "bind-interfaces" *after* starting any libvirt networks, the new dnsmasq would fail to start, because there was already another process listening on port 53. (Subsequent to the CVE fix, another patch changed the network driver to put dnsmasq options in a conf file rather than directly on the dnsmasq commandline, but preserved the same options.) This patch changes the network driver to *always* add "except-interface=lo" to dnsmasq conf files, regardless of whether we use bind-dynamic or bind-interfaces. This way no libvirt dnsmasq instances are listening on localhost (and the CVE is still fixed). The actual code change is miniscule, but must be propogated through all of the test files as well. --- src/network/bridge_driver.c | 7 ++++--- tests/networkxml2confdata/dhcp6-nat-network.conf | 1 + tests/networkxml2confdata/dhcp6-network.conf | 1 + tests/networkxml2confdata/dhcp6host-routed-network.conf | 1 + tests/networkxml2confdata/isolated-network.conf | 2 +- tests/networkxml2confdata/nat-network-dns-hosts.conf | 1 + tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf | 2 +- tests/networkxml2confdata/nat-network-dns-srv-record.conf | 1 + tests/networkxml2confdata/nat-network-dns-txt-record.conf | 1 + tests/networkxml2confdata/nat-network.conf | 1 + tests/networkxml2confdata/netboot-network.conf | 2 +- tests/networkxml2confdata/netboot-proxy-network.conf | 2 +- tests/networkxml2confdata/routed-network.conf | 1 + 13 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4e1958d..a32755d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -689,6 +689,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (pidfile) virBufferAsprintf(&configbuf, "pid-file=%s\n", pidfile); + /* dnsmasq will *always* listen on localhost unless told otherwise */ + virBufferAddLit(&configbuf, "except-interface=lo\n"); + if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) { /* using --bind-dynamic with only --interface (no * --listen-address) prevents dnsmasq from responding to dns @@ -702,9 +705,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, "interface=%s\n", network->def->bridge); } else { - virBufferAddLit(&configbuf, - "bind-interfaces\n" - "except-interface=lo\n"); + virBufferAddLit(&configbuf, "bind-interfaces\n"); /* * --interface does not actually work with dnsmasq < 2.47, * due to DAD for ipv6 addresses on the interface. diff --git a/tests/networkxml2confdata/dhcp6-nat-network.conf b/tests/networkxml2confdata/dhcp6-nat-network.conf index d488900..050f3db 100644 --- a/tests/networkxml2confdata/dhcp6-nat-network.conf +++ b/tests/networkxml2confdata/dhcp6-nat-network.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr0 dhcp-range=192.168.122.2,192.168.122.254 diff --git a/tests/networkxml2confdata/dhcp6-network.conf b/tests/networkxml2confdata/dhcp6-network.conf index 5c1030c..5fde07f 100644 --- a/tests/networkxml2confdata/dhcp6-network.conf +++ b/tests/networkxml2confdata/dhcp6-network.conf @@ -9,6 +9,7 @@ domain-needed domain=mynet expand-hosts local=/mynet/ +except-interface=lo bind-dynamic interface=virbr0 dhcp-range=2001:db8:ac10:fd01::1:10,2001:db8:ac10:fd01::1:ff diff --git a/tests/networkxml2confdata/dhcp6host-routed-network.conf b/tests/networkxml2confdata/dhcp6host-routed-network.conf index cb4d0cc..f8f05c2 100644 --- a/tests/networkxml2confdata/dhcp6host-routed-network.conf +++ b/tests/networkxml2confdata/dhcp6host-routed-network.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr1 dhcp-range=192.168.122.1,static diff --git a/tests/networkxml2confdata/isolated-network.conf b/tests/networkxml2confdata/isolated-network.conf index 55a44d3..f8997bd 100644 --- a/tests/networkxml2confdata/isolated-network.conf +++ b/tests/networkxml2confdata/isolated-network.conf @@ -7,8 +7,8 @@ strict-order domain-needed local=// -bind-interfaces except-interface=lo +bind-interfaces listen-address=192.168.152.1 dhcp-option=3 no-resolv diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.conf b/tests/networkxml2confdata/nat-network-dns-hosts.conf index ae8f8c5..2577882 100644 --- a/tests/networkxml2confdata/nat-network-dns-hosts.conf +++ b/tests/networkxml2confdata/nat-network-dns-hosts.conf @@ -9,6 +9,7 @@ domain-needed domain=example.com expand-hosts local=/example.com/ +except-interface=lo bind-dynamic interface=virbr0 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index faa36e6..1e9b59c 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -7,8 +7,8 @@ strict-order domain-needed local=// -bind-interfaces except-interface=lo +bind-interfaces listen-address=192.168.122.1 listen-address=192.168.123.1 listen-address=fc00:db8:ac10:fe01::1 diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index 6079912..53d044a 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr0 srv-host=name.tcp.test-domain-name,.,1024,10,10 diff --git a/tests/networkxml2confdata/nat-network-dns-txt-record.conf b/tests/networkxml2confdata/nat-network-dns-txt-record.conf index c448bdc..921cae1 100644 --- a/tests/networkxml2confdata/nat-network-dns-txt-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-txt-record.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr0 txt-record=example,example value diff --git a/tests/networkxml2confdata/nat-network.conf b/tests/networkxml2confdata/nat-network.conf index 8f28fef..beb714b 100644 --- a/tests/networkxml2confdata/nat-network.conf +++ b/tests/networkxml2confdata/nat-network.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr0 dhcp-range=192.168.122.2,192.168.122.254 diff --git a/tests/networkxml2confdata/netboot-network.conf b/tests/networkxml2confdata/netboot-network.conf index 83dd2b3..b6f3c23 100644 --- a/tests/networkxml2confdata/netboot-network.conf +++ b/tests/networkxml2confdata/netboot-network.conf @@ -9,8 +9,8 @@ domain-needed domain=example.com expand-hosts local=/example.com/ -bind-interfaces except-interface=lo +bind-interfaces listen-address=192.168.122.1 dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override diff --git a/tests/networkxml2confdata/netboot-proxy-network.conf b/tests/networkxml2confdata/netboot-proxy-network.conf index b266d81..1e969fa 100644 --- a/tests/networkxml2confdata/netboot-proxy-network.conf +++ b/tests/networkxml2confdata/netboot-proxy-network.conf @@ -9,8 +9,8 @@ domain-needed domain=example.com expand-hosts local=/example.com/ -bind-interfaces except-interface=lo +bind-interfaces listen-address=192.168.122.1 dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override diff --git a/tests/networkxml2confdata/routed-network.conf b/tests/networkxml2confdata/routed-network.conf index dc53a4e..62ffd7a 100644 --- a/tests/networkxml2confdata/routed-network.conf +++ b/tests/networkxml2confdata/routed-network.conf @@ -7,6 +7,7 @@ strict-order domain-needed local=// +except-interface=lo bind-dynamic interface=virbr1 addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts -- 1.7.11.7
(I'm sending this v0.10.2-maint "backport" of the upstream patch to the list because the code in question was completely replaced, so it required a new patch rather than a backport. The backport for 0.9.11-maint is nearly identical to this, so I won't be sending that one) This patch resolves the problem reported in: https://bugzilla.redhat.com/show_bug.cgi?id=886663 The source of the problem was the fix for CVE 2011-3411: https://bugzilla.redhat.com/show_bug.cgi?id=833033 which was originally committed upstream in commit 753ff83a50263d6975f88d6605d4b5ddfcc97560. That commit improperly removed the "--except-interface lo" from dnsmasq commandlines when --bind-dynamic was used (based on comments in the latter bug). It turns out that the problem reported in the CVE could be eliminated without removing "--except-interface lo", and removing it actually caused each instance of dnsmasq to listen on localhost on port 53, which created a new problem: If another instance of dnsmasq using "--bind-interfaces" (instead of "--bind-dynamic") had already been started (or if another instance started later used "--bind-dynamic"), this wouldn't have any immediately visible ill effects, but if you tried to start another dnsmasq instance using "--bind-interfaces" *after* starting any libvirt networks, the new dnsmasq would fail to start, because there was already another process listening on port 53. This patch changes the network driver to *always* add "--except-interface lo" to dnsmasq conf files, regardless of whether we use bind-dynamic or bind-interfaces. This way no libvirt dnsmasq instances are listening on localhost (and the CVE is still fixed). The actual code change is miniscule, but must be propogated through all of the test files as well. (This is *not* a cherry-pick of the upstream commit, because subsequent to the CVE fix, another patch changed the network driver to put dnsmasq options in a conf file rather than directly on the dnsmasq commandline, but preserved the same options.) --- src/network/bridge_driver.c | 8 ++++---- tests/networkxml2argvdata/isolated-network.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-hosts.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-srv-record.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-txt-record.argv | 2 +- tests/networkxml2argvdata/nat-network.argv | 2 +- tests/networkxml2argvdata/netboot-network.argv | 2 +- tests/networkxml2argvdata/netboot-proxy-network.argv | 2 +- tests/networkxml2argvdata/routed-network.argv | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1fa7cd0..ff11364 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -672,6 +672,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, /* *no* conf file */ virCommandAddArg(cmd, "--conf-file="); + /* dnsmasq will *always* listen on localhost unless told otherwise */ + virCommandAddArgList(cmd, "--except-interface", "lo", NULL); + if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) { /* using --bind-dynamic with only --interface (no * --listen-address) prevents dnsmasq from responding to dns @@ -685,10 +688,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, "--interface", network->def->bridge, NULL); } else { - virCommandAddArgList(cmd, - "--bind-interfaces", - "--except-interface", "lo", - NULL); + virCommandAddArg(cmd, "--bind-interfaces"); /* * --interface does not actually work with dnsmasq < 2.47, * due to DAD for ipv6 addresses on the interface. diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv index 3d8601e..861fd74 100644 --- a/tests/networkxml2argvdata/isolated-network.argv +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-interfaces --except-interface lo \ +--except-interface lo --bind-interfaces \ --listen-address 192.168.152.1 \ --dhcp-option=3 --no-resolv \ --dhcp-range 192.168.152.2,192.168.152.254 \ diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv b/tests/networkxml2argvdata/nat-network-dns-hosts.argv index e5143ac..431e987 100644 --- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv @@ -1,5 +1,5 @@ @DNSMASQ@ --strict-order --domain=example.com \ --local=/example.com/ --domain-needed \ --conf-file= \ ---bind-dynamic --interface virbr0 \ +--except-interface lo --bind-dynamic --interface virbr0 \ --expand-hosts --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\ diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv index 031da3f..24d88ad 100644 --- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv @@ -1,7 +1,7 @@ @DNSMASQ@ \ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-interfaces --except-interface lo \ +--except-interface lo --bind-interfaces \ --listen-address 192.168.122.1 \ --listen-address 192.168.123.1 \ --listen-address fc00:db8:ac10:fe01::1 \ diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv index beff591..f417af8 100644 --- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv @@ -1,7 +1,7 @@ @DNSMASQ@ \ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-dynamic --interface virbr0 \ +--except-interface lo --bind-dynamic --interface virbr0 \ --srv-host=name.tcp.test-domain-name,.,1024,10,10 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \ diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv index fc164f6..86b7ba3 100644 --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-dynamic --interface virbr0 \ +--except-interface lo --bind-dynamic --interface virbr0 \ '--txt-record=example,example value' \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \ diff --git a/tests/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv index 6303f76..8a540d5 100644 --- a/tests/networkxml2argvdata/nat-network.argv +++ b/tests/networkxml2argvdata/nat-network.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-dynamic --interface virbr0 \ +--except-interface lo --bind-dynamic --interface virbr0 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases \ --dhcp-lease-max=253 --dhcp-no-override \ diff --git a/tests/networkxml2argvdata/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv index 9699e5c..0e17a71 100644 --- a/tests/networkxml2argvdata/netboot-network.argv +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order --domain=example.com \ --local=/example.com/ --domain-needed --conf-file= \ ---bind-interfaces --except-interface lo --listen-address 192.168.122.1 \ +--except-interface lo --bind-interfaces --listen-address 192.168.122.1 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \ --dhcp-lease-max=253 --dhcp-no-override --expand-hosts \ diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv index 9ac3018..8764ef5 100644 --- a/tests/networkxml2argvdata/netboot-proxy-network.argv +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order --domain=example.com \ --local=/example.com/ --domain-needed --conf-file= \ ---bind-interfaces --except-interface lo \ +--except-interface lo --bind-interfaces \ --listen-address 192.168.122.1 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \ diff --git a/tests/networkxml2argvdata/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv index 700c904..3221f65 100644 --- a/tests/networkxml2argvdata/routed-network.argv +++ b/tests/networkxml2argvdata/routed-network.argv @@ -1,4 +1,4 @@ @DNSMASQ@ --strict-order \ --local=// --domain-needed --conf-file= \ ---bind-dynamic --interface virbr1 \ +--except-interface lo --bind-dynamic --interface virbr1 \ --addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts\ -- 1.7.11.7
On 12/13/12 10:00, Laine Stump wrote:
(I'm sending this v0.10.2-maint "backport" of the upstream patch to the list because the code in question was completely replaced, so it required a new patch rather than a backport. The backport for 0.9.11-maint is nearly identical to this, so I won't be sending that one)
This patch resolves the problem reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=886663
The source of the problem was the fix for CVE 2011-3411:
https://bugzilla.redhat.com/show_bug.cgi?id=833033
which was originally committed upstream in commit 753ff83a50263d6975f88d6605d4b5ddfcc97560. That commit improperly removed the "--except-interface lo" from dnsmasq commandlines when --bind-dynamic was used (based on comments in the latter bug).
It turns out that the problem reported in the CVE could be eliminated without removing "--except-interface lo", and removing it actually caused each instance of dnsmasq to listen on localhost on port 53, which created a new problem:
If another instance of dnsmasq using "--bind-interfaces" (instead of "--bind-dynamic") had already been started (or if another instance started later used "--bind-dynamic"), this wouldn't have any immediately visible ill effects, but if you tried to start another dnsmasq instance using "--bind-interfaces" *after* starting any libvirt networks, the new dnsmasq would fail to start, because there was already another process listening on port 53.
This patch changes the network driver to *always* add "--except-interface lo" to dnsmasq conf files, regardless of whether we use bind-dynamic or bind-interfaces. This way no libvirt dnsmasq instances are listening on localhost (and the CVE is still fixed).
The actual code change is miniscule, but must be propogated through all of the test files as well.
(This is *not* a cherry-pick of the upstream commit, because subsequent to the CVE fix, another patch changed the network driver to put dnsmasq options in a conf file rather than directly on the dnsmasq commandline, but preserved the same options.) --- src/network/bridge_driver.c | 8 ++++---- tests/networkxml2argvdata/isolated-network.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-hosts.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-srv-record.argv | 2 +- tests/networkxml2argvdata/nat-network-dns-txt-record.argv | 2 +- tests/networkxml2argvdata/nat-network.argv | 2 +- tests/networkxml2argvdata/netboot-network.argv | 2 +- tests/networkxml2argvdata/netboot-proxy-network.argv | 2 +- tests/networkxml2argvdata/routed-network.argv | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-)
ACK to this "backport" too :). I didn't do a compile test of this patch but the changes seem reasonable and according to the manpage. Peter
On 12/13/12 08:34, Laine Stump wrote:
This patch resolves the problem reported in:
https://bugzilla.redhat.com/show_bug.cgi?id=886663
The source of the problem was the fix for CVE 2011-3411:
https://bugzilla.redhat.com/show_bug.cgi?id=833033
which was originally committed upstream in commit 753ff83a50263d6975f88d6605d4b5ddfcc97560. That commit improperly removed the "--except-interface lo" from dnsmasq commandlines when --bind-dynamic was used (based on comments in the latter bug).
It turns out that the problem reported in the CVE could be eliminated without removing "--except-interface lo", and removing it actually caused each instance of dnsmasq to listen on localhost on port 53, which created a new problem:
If another instance of dnsmasq using "bind-interfaces" (instead of "bind-dynamic") had already been started (or if another instance started later used "bind-dynamic"), this wouldn't have any immediately visible ill effects, but if you tried to start another dnsmasq instance using "bind-interfaces" *after* starting any libvirt networks, the new dnsmasq would fail to start, because there was already another process listening on port 53.
(Subsequent to the CVE fix, another patch changed the network driver to put dnsmasq options in a conf file rather than directly on the dnsmasq commandline, but preserved the same options.)
This patch changes the network driver to *always* add "except-interface=lo" to dnsmasq conf files, regardless of whether we use bind-dynamic or bind-interfaces. This way no libvirt dnsmasq instances are listening on localhost (and the CVE is still fixed).
The actual code change is miniscule, but must be propogated through all of the test files as well. --- src/network/bridge_driver.c | 7 ++++--- tests/networkxml2confdata/dhcp6-nat-network.conf | 1 + tests/networkxml2confdata/dhcp6-network.conf | 1 + tests/networkxml2confdata/dhcp6host-routed-network.conf | 1 + tests/networkxml2confdata/isolated-network.conf | 2 +- tests/networkxml2confdata/nat-network-dns-hosts.conf | 1 + tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf | 2 +- tests/networkxml2confdata/nat-network-dns-srv-record.conf | 1 + tests/networkxml2confdata/nat-network-dns-txt-record.conf | 1 + tests/networkxml2confdata/nat-network.conf | 1 + tests/networkxml2confdata/netboot-network.conf | 2 +- tests/networkxml2confdata/netboot-proxy-network.conf | 2 +- tests/networkxml2confdata/routed-network.conf | 1 + 13 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4e1958d..a32755d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -689,6 +689,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (pidfile) virBufferAsprintf(&configbuf, "pid-file=%s\n", pidfile);
+ /* dnsmasq will *always* listen on localhost unless told otherwise */ + virBufferAddLit(&configbuf, "except-interface=lo\n"); + if (dnsmasqCapsGet(caps, DNSMASQ_CAPS_BIND_DYNAMIC)) { /* using --bind-dynamic with only --interface (no * --listen-address) prevents dnsmasq from responding to dns @@ -702,9 +705,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, "interface=%s\n", network->def->bridge); } else { - virBufferAddLit(&configbuf, - "bind-interfaces\n" - "except-interface=lo\n"); + virBufferAddLit(&configbuf, "bind-interfaces\n"); /* * --interface does not actually work with dnsmasq < 2.47, * due to DAD for ipv6 addresses on the interface.
Hm, the pre-existing indentation seems to be broken in this file. ACK to this patch, the fix seems reasonable and the dnsmasq man page is telling that this option should do what we're expecting. I'll probably post a cleanup to this function later. Peter
Somehow I managed to push the changes to this file with improper indentation. This patch just re-indents, reformats the comment lines, and re-groups a couple of multi-line strings so that they fit within 80 columns. The resulting binary should be identical. I'm *not* pushing this as trivial, because the strings were re-grouped and a 2nd set of eyes verifying I didn't botch anything would probably be a good idea. (I did successfully run make syntax-check and make check). --- src/network/bridge_driver.c | 141 +++++++++++++++++++++++--------------------- 1 file changed, 75 insertions(+), 66 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a32755d..fdb9109 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -629,10 +629,10 @@ networkBuildDnsmasqHostsList(dnsmasqContext *dctx, int networkDnsmasqConfContents(virNetworkObjPtr network, - const char *pidfile, - char **configstr, - dnsmasqContext *dctx, - dnsmasqCapsPtr caps ATTRIBUTE_UNUSED) + const char *pidfile, + char **configstr, + dnsmasqContext *dctx, + dnsmasqCapsPtr caps ATTRIBUTE_UNUSED) { virBuffer configbuf = VIR_BUFFER_INITIALIZER; int r, ret = -1; @@ -664,29 +664,29 @@ networkDnsmasqConfContents(virNetworkObjPtr network, /* create dnsmasq config file appropriate for this network */ virBufferAsprintf(&configbuf, - "##WARNING: THIS IS AN AUTO-GENERATED FILE. " - "CHANGES TO IT ARE LIKELY TO BE\n" - "##OVERWRITTEN AND LOST. Changes to this " - "configuration should be made using:\n" - "## virsh net-edit %s\n" - "## or other application using the libvirt API.\n" - "##\n## dnsmasq conf file created by libvirt\n" - "strict-order\n" - "domain-needed\n", - network->def->name); - - if (network->def->domain) { + "##WARNING: THIS IS AN AUTO-GENERATED FILE. " + "CHANGES TO IT ARE LIKELY TO BE\n" + "##OVERWRITTEN AND LOST. Changes to this " + "configuration should be made using:\n" + "## virsh net-edit %s\n" + "## or other application using the libvirt API.\n" + "##\n## dnsmasq conf file created by libvirt\n" + "strict-order\n" + "domain-needed\n", + network->def->name); + + if (network->def->domain) { virBufferAsprintf(&configbuf, - "domain=%s\n" - "expand-hosts\n", - network->def->domain); - } - /* need to specify local even if no domain specified */ + "domain=%s\n" + "expand-hosts\n", + network->def->domain); + } + /* need to specify local even if no domain specified */ virBufferAsprintf(&configbuf, - "local=/%s/\n", - network->def->domain ? network->def->domain : ""); + "local=/%s/\n", + network->def->domain ? network->def->domain : ""); - if (pidfile) + if (pidfile) virBufferAsprintf(&configbuf, "pid-file=%s\n", pidfile); /* dnsmasq will *always* listen on localhost unless told otherwise */ @@ -701,9 +701,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network, * this network). This was added in response to CVE 2012-3411. */ virBufferAsprintf(&configbuf, - "bind-dynamic\n" - "interface=%s\n", - network->def->bridge); + "bind-dynamic\n" + "interface=%s\n", + network->def->bridge); } else { virBufferAddLit(&configbuf, "bind-interfaces\n"); /* @@ -721,6 +721,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (!ipaddr) goto cleanup; + /* 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) @@ -730,13 +731,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network, 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, 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); + "The version of dnsmasq on this host (%d.%d) " + "doesn't support the bind-dynamic option, " + "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); goto cleanup; } virBufferAsprintf(&configbuf, "listen-address=%s\n", ipaddr); @@ -753,7 +757,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, */ if (network->def->forward.type == VIR_NETWORK_FORWARD_NONE) { virBufferAddLit(&configbuf, "dhcp-option=3\n" - "no-resolv\n"); + "no-resolv\n"); } for (ii = 0; ii < dns->ntxts; ii++) { @@ -786,11 +790,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", dns->srvs[ii].service, dns->srvs[ii].protocol, - dns->srvs[ii].domain ? dns->srvs[ii].domain : "", - dns->srvs[ii].target ? dns->srvs[ii].target : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) { + dns->srvs[ii].domain ? dns->srvs[ii].domain : "", + dns->srvs[ii].target ? dns->srvs[ii].target : "", + recordPort ? recordPort : "", + recordPriority ? recordPriority : "", + recordWeight ? recordWeight : "") < 0) { virReportOOMError(); goto cleanup; } @@ -811,8 +815,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (ipdef->nranges || ipdef->nhosts) { if (ipv4def) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("For IPv4, multiple DHCP definitions cannot " - "be specified.")); + _("For IPv4, multiple DHCP definitions " + "cannot be specified.")); goto cleanup; } else { ipv4def = ipdef; @@ -824,17 +828,21 @@ networkDnsmasqConfContents(virNetworkObjPtr network, 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); + _("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); goto cleanup; } if (ipv6def) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("For IPv6, multiple DHCP definitions cannot " - "be specified.")); + _("For IPv6, multiple DHCP definitions " + "cannot be specified.")); goto cleanup; } else { ipv6def = ipdef; @@ -848,10 +856,10 @@ networkDnsmasqConfContents(virNetworkObjPtr network, if (ipv6def && ipv6SLAAC) { VIR_WARN("For IPv6, when DHCP is specified for one address, then " "state-full Router Advertising will occur. The additional " - "IPv6 addresses specified require manually configured guest " - "network to work properly since both state-full (DHCP) " - "and state-less (SLAAC) addressing are not supported " - "on the same network interface."); + "IPv6 addresses specified require manually configured guest " + "network to work properly since both state-full (DHCP) " + "and state-less (SLAAC) addressing are not supported " + "on the same network interface."); } ipdef = ipv4def ? ipv4def : ipv6def; @@ -867,7 +875,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, goto cleanup; } virBufferAsprintf(&configbuf, "dhcp-range=%s,%s\n", - saddr, eaddr); + saddr, eaddr); VIR_FREE(saddr); VIR_FREE(eaddr); nbleases += virSocketAddrGetRange(&ipdef->ranges[r].start, @@ -875,9 +883,10 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } /* - * For static-only DHCP, i.e. with no range but at least one host element, - * we have to add a special --dhcp-range option to enable the service in - * dnsmasq. (this is for dhcp-hosts= support) + * For static-only DHCP, i.e. with no range but at least one + * host element, we have to add a special --dhcp-range option + * to enable the service in dnsmasq. (this is for dhcp-hosts= + * support) */ if (!ipdef->nranges && ipdef->nhosts) { char *bridgeaddr = virSocketAddrFormat(&ipdef->address); @@ -909,7 +918,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, goto cleanup; } virBufferAsprintf(&configbuf, "dhcp-boot=%s%s%s\n", - ipdef->bootfile, ",,", bootserver); + ipdef->bootfile, ",,", bootserver); VIR_FREE(bootserver); } else { virBufferAsprintf(&configbuf, "dhcp-boot=%s\n", ipdef->bootfile); @@ -932,21 +941,21 @@ networkDnsmasqConfContents(virNetworkObjPtr network, /* this is done once per interface */ if (networkBuildDnsmasqHostsList(dctx, dns) < 0) - goto cleanup; + goto cleanup; /* Even if there are currently no static hosts, if we're * listening for DHCP, we should write a 0-length hosts * file to allow for runtime additions. */ if (ipv4def || ipv6def) - virBufferAsprintf(&configbuf, "dhcp-hostsfile=%s\n", - dctx->hostsfile->path); + virBufferAsprintf(&configbuf, "dhcp-hostsfile=%s\n", + dctx->hostsfile->path); - /* Likewise, always create this file and put it on the commandline, to allow for - * for runtime additions. + /* Likewise, always create this file and put it on the + * commandline, to allow for runtime additions. */ virBufferAsprintf(&configbuf, "addn-hosts=%s\n", - dctx->addnhostsfile->path); + dctx->addnhostsfile->path); /* Are we doing RA instead of radvd? */ if (DNSMASQ_RA_SUPPORT(caps)) { @@ -954,8 +963,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network, virBufferAddLit(&configbuf, "enable-ra\n"); else { for (ii = 0; - (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii)); - ii++) { + (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii)); + ii++) { if (!(ipdef->nranges || ipdef->nhosts)) { char *bridgeaddr = virSocketAddrFormat(&ipdef->address); if (!bridgeaddr) -- 1.7.11.7
On 12/13/2012 09:46 AM, Laine Stump wrote:
Somehow I managed to push the changes to this file with improper indentation. This patch just re-indents, reformats the comment lines, and re-groups a couple of multi-line strings so that they fit within 80 columns. The resulting binary should be identical.
I'm *not* pushing this as trivial, because the strings were re-grouped and a 2nd set of eyes verifying I didn't botch anything would probably be a good idea. (I did successfully run make syntax-check and make check). --- src/network/bridge_driver.c | 141 +++++++++++++++++++++++--------------------- 1 file changed, 75 insertions(+), 66 deletions(-)
You've got your second set of eyes: ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake -
Laine Stump -
Peter Krempa