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

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

 



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@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
>>

--
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]