[libvirt] [PATCH] network: prevent dnsmasq from listening on localhost

[Peter Krempa]

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