pdns_recursor: T2964: Expose query-local-address to dns config.

[lucasec]

Task reference: T2964

Description

In certain split DNS configurations, there is a need for more fine-grained control over the local address DNS forwarding uses to issue queries. The current pdns_recursor configuration allows the recursor to send queries from any available address on the interface the OS selects for the query, with no option to limit queries to a particular address or set of addresses.

This commit exposes the query-local-address option in recursor.conf to users via the service dns forwarding query-source-address config node.

If the parameter is unspecified, the default value of 0.0.0.0 (any IPv4 address) and :: (any IPv6 address) are used to match current behavior.

Users who want more control can specify one or more IPv4 and IPv6 addresses to issue queries from. Per pdns_recursor docs, the recursor will load balance queries between any available addresses in the pools. Since IPv4 and IPv6 are different pools, note that specifying only one type of address will disable issuing queries for the other address family.

Changes

• Added new property query-source-address to dns forwarding config mode definiton

Testing

You can test this PR by using the following example configuration (assuming the router has an IPv4 address of 10.5.1.1 that can be used to reach some resolver):

service {
     dns {
         forwarding {
             ...
             query-source-address 10.5.1.1
             query-source-address ::
         }
     }
}

Additional Notes

I have not located an obvious way to set the source address per-downstream resolver (as is possible in dnsmasq, see the ticket). If anyone more familiar knowns of any pdns lua wizardry, I'm all ears.

The current pdns_recursor 4.3.4 distributed with VyOS requires the source addresses to be specified in two different parameters query-local-address (for IPv4 addresses) and query-local-address6 (for IPv6 addresses). The configuration code accommodates this transparently for the user, allowing them to specify both address families in the same query-source-address node and appropriately splitting the addresses out into separate entries when rendering the config file.

Release notes indicate starting with pdns_recursor 4.4.0 both address families can be specified in the same query-local-address parameter and the query-local-address6 parameter will be deprecated and removed in 4.5.0. A comment was added so hopefully someone remembers this when we inevitably upgrade to a newer release.

[c-po]

Hi @lucasec, thank you for the contribution. Any reason we can not use source-address include and thus name the node source-address as we have in many other places?

https://github.com/vyos/vyos-1x/blob/current/interface-definitions/include/source-address-ipv4-ipv6.xml.i

[dmbaturin]

Agree, it should be source-address like everywhere else.

[c-po]

One more addition, the default values could be passed in by specifying the 0.0.0.0 :: node which now supports mumtiple values as whitespace separated list, which is clearer then habing this in the Python helper script - as this is actually part of the CLI definition and we are in the middle of cleaning this up ;).

[lucasec]

Agree on using source-address, after looking at the HTTP client precedent, I think that makes good sense.

@c-po I'm assuming you mean use the defaultValue XML property? Since we're using an include in this case, are we comfortable making the default 0.0.0.0 :: for every use of source-address-ipv4-ipv6.xml.i? (This may be a behavior change for other uses such as ssh client, as current code doesn't render any config file entry if the property is unspecified—will have to research what the client's internal default is)

[c-po]

@lucasec I just remembered that <defaultValue> can not be overwritten per <leafNode> as of https://phabricator.vyos.net/T2910 which must come first.

We should not change the default value for all users of the include file - thus please keep your default assignment in the python helper for the time beeing and simply use the #include <include/source-address-ipv4-ipv6.xml.i> as you have already found for http-client.

[lucasec]

Another blocker on using the include, the source-address-ipv4-ipv6.xml.i include does not have the <multi/> tag.

Should we just pass on the include for now until more work can be done on the templating system? I will push up a commit that renames the property to source-address still.

[c-po]

@lucasec good catch - please proceed as you lined out!

[lucasec]

As a bonus, now that we're not constrained by includes, I switched to <defaultValue>.

[lucasec]

PR should be updated and ready to merge. I tested to make sure the new defaultValue configuration is working as expected.

[dmbaturin]

Hhm, <defaultValue>0.0.0.0 ::</defaultValue>. Whitespace in a default value works correctly?

[lucasec]

It works as @c-po mentioned in his comment above—for <multi/> nodes it generates two values 0.0.0.0 and ::.

[lucasec]

Actually precedent for that is hard to find, as it's only been used in one other place so far: e4e75aa#diff-3debbbaec7fa1e1d4119681239a53a1dR29

[dmbaturin]

@lucasec I'd say this is more like an emergent behaviour... I wonder if we should allow multiple default value elements instead.

I'm happy to approve the PR as it, but it may be something to consider codifying in the future.