[libvirt] [PATCH] network: Let domains be restricted to local DNS
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream. This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop. Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org> --- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre> @@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) /* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + } if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef { char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } if (network->def->domain) { + if (network->def->domain_local) { + virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 2.1.0
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about: 1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file? 2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local). If you want to use the local= parameter, you would have to parse the domain value as it can be multiple domains separated by a comma and for domains that are actually IP addresses, you'd need to add the reverse of that (e.g. 122.168.192.in-addr.arpa).
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org> --- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) { + virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about:
1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file?
No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed in /etc/resolv.conf for *anything*. In this case, all that is wanted is to resolve requests _within the configured domain_ only locally, and not forward those requests upstream. Requests for any other domain _should_ be forwarded to whatever server is listed in /etc/resolv.conf. Based on a discussion with Josh, I'm certain that this is the behavior he wants (and it makes sense to have it available).
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local).
Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
If you want to use the local= parameter, you would have to parse the domain value as it can be multiple domains separated by a comma and for domains that are actually IP addresses, you'd need to add the reverse of that (e.g. 122.168.192.in-addr.arpa).
I'm not following this. Although the parsing code doesn't validate it, the domain name in libvirt's XML is only allowed to be a single "dnsName" according to the schema, and that is also what is documented in formatnetwork.html. And I can't think of any useful reason for specifying an IP address as the name (even though neither libvirt nor dnsmasq produces an error when you do that, it is simply a broken concept from the beginning).
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org> --- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) { + virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote:
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about:
1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file?
No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed in /etc/resolv.conf for *anything*. In this case, all that is wanted is to resolve requests _within the configured domain_ only locally, and not forward those requests upstream. Requests for any other domain _should_ be forwarded to whatever server is listed in /etc/resolv.conf. Based on a discussion with Josh, I'm certain that this is the behavior he wants (and it makes sense to have it available).
OK, I just misunderstood this part, sorry for that.
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local).
Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
Looking at the documentation again, I understand it a bit more yet again. Let's assume people use normal domain names. Libvirt can use the IP address and netmask from the 'ip' element (if known) and construct the domain parameter for the config file: domain=virt,192.168.13.0/24,local If there's no IP address known, then it could be constructed as follows: domain=virt local=/virt/ What do you say? Currently this is possible to achieve with the following in the XML: <domain name='virt,192.168.13.0/24,local'/> But that's just plain ugly ;-)
If you want to use the local= parameter, you would have to parse the domain value as it can be multiple domains separated by a comma and for domains that are actually IP addresses, you'd need to add the reverse of that (e.g. 122.168.192.in-addr.arpa).
I'm not following this. Although the parsing code doesn't validate it, the domain name in libvirt's XML is only allowed to be a single "dnsName" according to the schema, and that is also what is documented in formatnetwork.html. And I can't think of any useful reason for specifying an IP address as the name (even though neither libvirt nor dnsmasq produces an error when you do that, it is simply a broken concept from the beginning).
I misunderstood the dnsmasq manual. I didn't know that the commas only separated particular parameters. And dnsmasq actually validates it, for example using ,local with /23 network outputs an error when starting the network or if there are more than 2 commas in the parameter. Martin
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org> --- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) { + virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 12/01/2014 05:18 AM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote:
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about:
1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file?
No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed in /etc/resolv.conf for *anything*. In this case, all that is wanted is to resolve requests _within the configured domain_ only locally, and not forward those requests upstream. Requests for any other domain _should_ be forwarded to whatever server is listed in /etc/resolv.conf. Based on a discussion with Josh, I'm certain that this is the behavior he wants (and it makes sense to have it available).
OK, I just misunderstood this part, sorry for that.
Right, I want to keep its own domain local, but go out otherwise.
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local).
Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
Looking at the documentation again, I understand it a bit more yet again. Let's assume people use normal domain names. Libvirt can use the IP address and netmask from the 'ip' element (if known) and construct the domain parameter for the config file:
domain=virt,192.168.13.0/24,local
If there's no IP address known, then it could be constructed as follows:
domain=virt local=/virt/
What do you say?
Currently this is possible to achieve with the following in the XML:
<domain name='virt,192.168.13.0/24,local'/>
But that's just plain ugly ;-)
Interesting. If this works, and libvirt promises not to take it away from me later, then it would be enough to satisfy my itch. :) However, it doesn't actually seem to do the right thing. The option does survive from XML to the dnsmasq conf, but it doesn't seem to actually behave as local. When I manually tweak the conf and restart libvirt's dnsmasq myself, it seems that having separate "domain=foo" and "local=/foo/" terminates properly, but having a oneliner "domain=foo,192.168.122.0/24,local" is still forwarding to the host. Which then forwards back to libvirt, and I get my loop. :( This is dnsmasq-2.72-3.fc21.x86_64. Maybe dnsmasq only only treats it local when coming from that subnet? But it's documented as equivalent to local=/foo/, so maybe dnsmasq has a bug here. Did you try it?
If you want to use the local= parameter, you would have to parse the domain value as it can be multiple domains separated by a comma and for domains that are actually IP addresses, you'd need to add the reverse of that (e.g. 122.168.192.in-addr.arpa).
I'm not following this. Although the parsing code doesn't validate it, the domain name in libvirt's XML is only allowed to be a single "dnsName" according to the schema, and that is also what is documented in formatnetwork.html. And I can't think of any useful reason for specifying an IP address as the name (even though neither libvirt nor dnsmasq produces an error when you do that, it is simply a broken concept from the beginning).
I misunderstood the dnsmasq manual. I didn't know that the commas only separated particular parameters. And dnsmasq actually validates it, for example using ,local with /23 network outputs an error when starting the network or if there are more than 2 commas in the parameter.
Martin
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org> --- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */ unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) { + virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Update: I think it is indeed a dnsmasq bug -- details below... [trimming some context and cross-posting to dnsmasq-discuss] On 12/01/2014 10:50 AM, Josh Stone wrote:
On 12/01/2014 05:18 AM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote:
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local).
Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
Looking at the documentation again, I understand it a bit more yet again. Let's assume people use normal domain names. Libvirt can use the IP address and netmask from the 'ip' element (if known) and construct the domain parameter for the config file:
domain=virt,192.168.13.0/24,local
If there's no IP address known, then it could be constructed as follows:
domain=virt local=/virt/
What do you say?
Currently this is possible to achieve with the following in the XML:
<domain name='virt,192.168.13.0/24,local'/>
But that's just plain ugly ;-)
Interesting. If this works, and libvirt promises not to take it away from me later, then it would be enough to satisfy my itch. :)
However, it doesn't actually seem to do the right thing. The option does survive from XML to the dnsmasq conf, but it doesn't seem to actually behave as local. When I manually tweak the conf and restart libvirt's dnsmasq myself, it seems that having separate "domain=foo" and "local=/foo/" terminates properly, but having a oneliner "domain=foo,192.168.122.0/24,local" is still forwarding to the host. Which then forwards back to libvirt, and I get my loop. :(
This is dnsmasq-2.72-3.fc21.x86_64. Maybe dnsmasq only only treats it local when coming from that subnet? But it's documented as equivalent to local=/foo/, so maybe dnsmasq has a bug here. Did you try it?
I found that dnsmasq option.c has these lines: /* generate the equivalent of local=/<domain>/ local=/xxx.yyy.zzz.in-addr.arpa/ */ struct server *serv = add_rev4(new->start, msize); serv->flags |= SERV_NO_ADDR; But that only appears to create the second part for rDNS, where the first part is more important for my case. That changed in commit de73a497cad21, which went into v2.69, and before that it did have an additional snippet to create the local=/domain/ part too. So, dnsmasq folks, do you agree that this is a bug? Thanks, Josh
On 12/01/2014 01:50 PM, Josh Stone wrote:
On 12/01/2014 05:18 AM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote:
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about:
1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file? No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed in /etc/resolv.conf for *anything*. In this case, all that is wanted is to resolve requests _within the configured domain_ only locally, and not forward those requests upstream. Requests for any other domain _should_ be forwarded to whatever server is listed in /etc/resolv.conf. Based on a discussion with Josh, I'm certain that this is the behavior he wants (and it makes sense to have it available).
OK, I just misunderstood this part, sorry for that. Right, I want to keep its own domain local, but go out otherwise.
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local). Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
Looking at the documentation again, I understand it a bit more yet again. Let's assume people use normal domain names. Libvirt can use the IP address and netmask from the 'ip' element (if known) and construct the domain parameter for the config file:
domain=virt,192.168.13.0/24,local
If there's no IP address known, then it could be constructed as follows:
domain=virt local=/virt/
What do you say?
Yeah, that's what I was suggesting as a possibility above (but wondering if it was worth the ambiguity), but according to Josh's testing, there is at least one version of dnsmasq that implements "domain=blah,x.x.x.x/y,local" incorrectly, so I guess we can't use it, at least not by itself.
Currently this is possible to achieve with the following in the XML:
<domain name='virt,192.168.13.0/24,local'/>
But that's just plain ugly ;-)
Interesting. If this works, and libvirt promises not to take it away from me later, then it would be enough to satisfy my itch. :)
I'm actually glad that it didn't work, as even the presence of this email could create a precedent that we don't want to exist. As a rule, libvirt does not like multiple attributes in a single string; if multiple attributes are needed, we try to represent them separately in the XML. In other words, don't count on the above XML working. As a matter of fact, assume that it will be eliminated.
However, it doesn't actually seem to do the right thing. The option does survive from XML to the dnsmasq conf, but it doesn't seem to actually behave as local. When I manually tweak the conf and restart libvirt's dnsmasq myself, it seems that having separate "domain=foo" and "local=/foo/" terminates properly, but having a oneliner "domain=foo,192.168.122.0/24,local" is still forwarding to the host. Which then forwards back to libvirt, and I get my loop. :(
This is dnsmasq-2.72-3.fc21.x86_64. Maybe dnsmasq only only treats it local when coming from that subnet? But it's documented as equivalent to local=/foo/, so maybe dnsmasq has a bug here. Did you try it?
From the other mail it sounds like you discovered a bug in dnsmasq. But
libvirt will anyway want to work even with buggy versions of dnsmasq, so perhaps localOnly should continue to add local=/foo/, but also add ",${ip}/$prefix,local" to the domain option *if* the prefix is 8, 16, or 24. I would say this can/should be done in a separate patch (which would update the test cases and the docs accordingly). Back to the code of your patch - the only issue I see with it is what John caught (missing the addition to virNetworkDefFormatBuf(), and using virTriStateBool(To|From)String rather than comparing to "yes"). Other than that, it would have been nice if this config could be in the <dns> element, but <domain> is at the top level of network because it is used by both dns and dhcp, and this new attribute is most closely associated with the domain name, so that's not really possible.
On 12/02/2014 08:06 AM, Laine Stump wrote:
On 12/01/2014 01:50 PM, Josh Stone wrote:
On 12/01/2014 05:18 AM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 07:49:32AM -0500, Laine Stump wrote:
On 11/30/2014 04:06 PM, Martin Kletzander wrote:
On Thu, Nov 20, 2014 at 06:46:41PM -0800, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
I've found 2 things in this patch I'm not sure about:
1) According to the documentation about --local= parameter, it says: "Setting this flag does not suppress reading of /etc/resolv.conf, use -R to do that." Shouldn't "no-resolv" be added as the option you want in the config file? No, "-R" (aka "no-resolv") tells dnsmasq to not use the servers listed in /etc/resolv.conf for *anything*. In this case, all that is wanted is to resolve requests _within the configured domain_ only locally, and not forward those requests upstream. Requests for any other domain _should_ be forwarded to whatever server is listed in /etc/resolv.conf. Based on a discussion with Josh, I'm certain that this is the behavior he wants (and it makes sense to have it available).
OK, I just misunderstood this part, sorry for that. Right, I want to keep its own domain local, but go out otherwise.
2) If your answer to the previous question is "NO", which might very well be the case, for example if you don't want to forward that one particular domain only (and I misunderstood the commit message), I think you should just use the ",local" subparameter of the domain parameter (domain=example.com,local). Interesting idea - this requires specifying the IP/prefix of the network in the option along with domain (which I suppose isn't a problem), and has the same effect as adding local=/domain.com/ as well as the reverse resolution, which is a nice plus. However, the prefix for the network must be 8, 16, or 24 for this to work (because of the way that *.in-addr.arpa records are defined), so we can't do this all the time. The question is then - do we write in a special case to do it this way when the prefix is one of the right lengths, or just not do it at all? Having it "magically happen" for some configs and not others could lead to confusion, but having reverse resolution when possible would be very useful (for example, some sshd setups have a long delay if the reverse-resolve of the client's IP doesn't return something other than "not found"; a pointless piece of security theater, but still fairly common).
Looking at the documentation again, I understand it a bit more yet again. Let's assume people use normal domain names. Libvirt can use the IP address and netmask from the 'ip' element (if known) and construct the domain parameter for the config file:
domain=virt,192.168.13.0/24,local
If there's no IP address known, then it could be constructed as follows:
domain=virt local=/virt/
What do you say?
Yeah, that's what I was suggesting as a possibility above (but wondering if it was worth the ambiguity), but according to Josh's testing, there is at least one version of dnsmasq that implements "domain=blah,x.x.x.x/y,local" incorrectly, so I guess we can't use it, at least not by itself.
Currently this is possible to achieve with the following in the XML:
<domain name='virt,192.168.13.0/24,local'/>
But that's just plain ugly ;-)
Interesting. If this works, and libvirt promises not to take it away from me later, then it would be enough to satisfy my itch. :)
I'm actually glad that it didn't work, as even the presence of this email could create a precedent that we don't want to exist.
As a rule, libvirt does not like multiple attributes in a single string; if multiple attributes are needed, we try to represent them separately in the XML.
In other words, don't count on the above XML working. As a matter of fact, assume that it will be eliminated.
Fair enough, I'll ignore this possibility.
However, it doesn't actually seem to do the right thing. The option does survive from XML to the dnsmasq conf, but it doesn't seem to actually behave as local. When I manually tweak the conf and restart libvirt's dnsmasq myself, it seems that having separate "domain=foo" and "local=/foo/" terminates properly, but having a oneliner "domain=foo,192.168.122.0/24,local" is still forwarding to the host. Which then forwards back to libvirt, and I get my loop. :(
This is dnsmasq-2.72-3.fc21.x86_64. Maybe dnsmasq only only treats it local when coming from that subnet? But it's documented as equivalent to local=/foo/, so maybe dnsmasq has a bug here. Did you try it?
From the other mail it sounds like you discovered a bug in dnsmasq. But libvirt will anyway want to work even with buggy versions of dnsmasq, so perhaps localOnly should continue to add local=/foo/, but also add ",${ip}/$prefix,local" to the domain option *if* the prefix is 8, 16, or 24. I would say this can/should be done in a separate patch (which would update the test cases and the docs accordingly).
Once you have to have the local=/domain/ option anyway, the only other thing you really get is rDNS. Maybe <ip> or its <dhcp> should have their own localOnly attribute to restrict the in-addr.arpa separately?
Back to the code of your patch - the only issue I see with it is what John caught (missing the addition to virNetworkDefFormatBuf(), and using virTriStateBool(To|From)String rather than comparing to "yes"). Other than that, it would have been nice if this config could be in the <dns> element, but <domain> is at the top level of network because it is used by both dns and dhcp, and this new attribute is most closely associated with the domain name, so that's not really possible.
I'll send an update for tristate comparisons and formatting. Thanks, Josh
On 11/20/2014 09:46 PM, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org>
Depending on how you proceed with Martin's comments...
--- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
This should use virTristateBoolTypeFromString like other yes/no processing. Also how do you save in the XML what you've read in (hint: Format* function)? Perhaps look at 'forwardPlainNames' for an example of Tristate and find all the places it touches. Something is a Tristate when it's optional, can be yes or no, and has a default...
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */
This would then be: int domain_local; /* enum virTristateBool */
unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) {
This would then check using == VIR_TRISTATE_BOOL_YES
+ virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6);
On 12/01/2014 03:40 AM, John Ferlan wrote:
On 11/20/2014 09:46 PM, Josh Stone wrote:
This adds a new "localOnly" attribute on the domain element of the network xml. With this set to "yes", DNS requests under that domain will only be resolved by libvirt's dnsmasq, never forwarded upstream.
This was how it worked before commit f69a6b987d616, and I found that functionality useful. For example, I have my host's NetworkManager dnsmasq configured to forward that domain to libvirt's dnsmasq, so I can easily resolve guest names from outside. But if libvirt's dnsmasq doesn't know a name and forwards it to the host, I'd get an endless forwarding loop. Now I can set localOnly="yes" to prevent the loop.
Signed-off-by: Josh Stone <jistone@redhat.com> Cc: Laine Stump <laine@laine.org>
Depending on how you proceed with Martin's comments...
--- docs/formatnetwork.html.in | 12 +++++++++++- docs/schemas/network.rng | 3 +++ src/conf/network_conf.c | 5 +++++ src/conf/network_conf.h | 1 + src/network/bridge_driver.c | 5 +++++ .../networkxml2confdata/nat-network-dns-local-domain.conf | 14 ++++++++++++++ tests/networkxml2confdata/nat-network-dns-local-domain.xml | 9 +++++++++ tests/networkxml2conftest.c | 1 + 8 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.conf create mode 100644 tests/networkxml2confdata/nat-network-dns-local-domain.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index dc438aee8622..defcdba00930 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -82,7 +82,7 @@ <pre> ... <bridge name="virbr0" stp="on" delay="5"/> - <domain name="example.com"/> + <domain name="example.com" localOnly="no"/> <forward mode="nat" dev="eth0"/> ...</pre>
@@ -113,6 +113,16 @@ a <code><forward></code> mode of "nat" or "route" (or an isolated network with no <code><forward></code> element). <span class="since">Since 0.4.5</span> + + <p> + If the optional <code>localOnly</code> attribute on the + <code>domain</code> element is "yes", then DNS requests under + this domain will only be resolved by the virtual network's own + DNS server - they will not be forwarded to the host's upstream + DNS server. If <code>localOnly</code> is "no", and by + default, unresolved requests <b>will</b> be forwarded. + <span class="since">Since 1.2.11</span> + </p> </dd> <dt><code>forward</code></dt> <dd>Inclusion of the <code>forward</code> element indicates that diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4546f8037580..a1da28092375 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -225,6 +225,9 @@ <optional> <element name="domain"> <attribute name="name"><ref name="dnsName"/></attribute> + <optional> + <attribute name="localOnly"><ref name="virYesNo"/></attribute> + </optional> </element> </optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067334e87cb0..61451c39805f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2083,6 +2083,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + tmp = virXPathString("string(./domain[1]/@localOnly)", ctxt); + if (tmp) { + def->domain_local = STRCASEEQ(tmp, "yes"); + VIR_FREE(tmp); + }
This should use virTristateBoolTypeFromString like other yes/no processing.
Ok, I just didn't see any really consistent way to parse these. STRCASEEQ was just simple and also used elsewhere, for @default and @managed, but maybe those aren't tristate.
Also how do you save in the XML what you've read in (hint: Format* function)? Perhaps look at 'forwardPlainNames' for an example of Tristate and find all the places it touches. Something is a Tristate when it's optional, can be yes or no, and has a default...
Ah, I missed Format, thanks. I'll update with forwardPlainNames as a template, if the other subthread concludes localOnly is still useful.
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && (def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 660cd2d10cd1..6308a7dcfbf7 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -232,6 +232,7 @@ struct _virNetworkDef {
char *bridge; /* Name of bridge device */ char *domain; + bool domain_local; /* Choose not to forward dns for this domain */
This would then be:
int domain_local; /* enum virTristateBool */
unsigned long delay; /* Bridge forward delay (ms) */ bool stp; /* Spanning tree protocol */ virMacAddr mac; /* mac address of bridge device */ diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6cb421c52850..dfa375d3aa72 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -912,6 +912,11 @@ networkDnsmasqConfContents(virNetworkObjPtr network, }
if (network->def->domain) { + if (network->def->domain_local) {
This would then check using == VIR_TRISTATE_BOOL_YES
+ virBufferAsprintf(&configbuf, + "local=/%s/\n", + network->def->domain); + } virBufferAsprintf(&configbuf, "domain=%s\n" "expand-hosts\n", diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.conf b/tests/networkxml2confdata/nat-network-dns-local-domain.conf new file mode 100644 index 000000000000..5f41b9186cbc --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.conf @@ -0,0 +1,14 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +local=/example.com/ +domain=example.com +expand-hosts +except-interface=lo +bind-dynamic +interface=virbr0 +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts diff --git a/tests/networkxml2confdata/nat-network-dns-local-domain.xml b/tests/networkxml2confdata/nat-network-dns-local-domain.xml new file mode 100644 index 000000000000..a92d71f1f2f6 --- /dev/null +++ b/tests/networkxml2confdata/nat-network-dns-local-domain.xml @@ -0,0 +1,9 @@ +<network> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9c</uuid> + <forward dev='eth0' mode='nat'/> + <bridge name='virbr0' stp='on' delay='0' /> + <domain name='example.com' localOnly='yes'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + </ip> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 4f1d9345ffe4..d2aa8c62cfcd 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -146,6 +146,7 @@ mymain(void) DO_TEST("nat-network-dns-hosts", full); DO_TEST("nat-network-dns-forward-plain", full); DO_TEST("nat-network-dns-forwarders", full); + DO_TEST("nat-network-dns-local-domain", full); DO_TEST("dhcp6-network", dhcpv6); DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6);
participants (4)
-
John Ferlan -
Josh Stone -
Laine Stump -
Martin Kletzander