Re: [PATCH] network: Let domains be restricted to local DNS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
Cc: Laine Stump <laine@xxxxxxxxx>
---
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>
        ...
        &lt;bridge name="virbr0" stp="on" delay="5"/&gt;
-        &lt;domain name="example.com"/&gt;
+        &lt;domain name="example.com" localOnly="no"/&gt;
        &lt;forward mode="nat" dev="eth0"/&gt;
        ...</pre>

@@ -113,6 +113,16 @@
        a <code>&lt;forward&gt;</code> mode of "nat" or "route" (or an
        isolated network with no <code>&lt;forward&gt;</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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]