dns/bind: forwarders with port

[raldone01]

Fixes #1715
First time writing a jinja script.
Let me know if anything is incorrect.

[fichtner]

Hi, what are we trying to fix? Validation/model not taking the right data? Ideally I’d like to avoid large data manipulation in the template.

Cheers,
Franco

[raldone01]

I want to run unbound on port 53530 and bind on port 53.
I would then add 127.0.0.1:53530 to the forwarders.
Currently I can't add the port to the forwarders.

Is it possible to handle the data manipulation somewhere else?

[fichtner]

Yes, ping me on Monday please.

[raldone01]

I just read the Plugin Hello World.

I think the validation/transformation logic should go into:

GeneralController.php

namespace OPNsense\Bind\Api;

use OPNsense\Base\ApiMutableModelControllerBase;

class GeneralController extends ApiMutableModelControllerBase
{
    protected static $internalModelClass = '\OPNsense\Bind\General';
    protected static $internalModelName = 'general';
}

I don't know how to go about this though.

Also what is the minimal setup required to deploy/test plugins?

[fichtner]

The change belongs to the model xml file. You can look up how the forward address is defined there.

[fichtner]

PS: the Listen addresses are already addr/port pairs but that construct is not great. It’s probably work to make this future proof but I had this on my wish list for years anyway.

[raldone01]

So you would also like the listen addresses be addr:port and have the port field removed?
Currently you can only have one shared port for all listen ips.
Also it should not be very difficult to just have one listen field shared for ipv4 and ipv6 addresses.

I could just copy and paste my jinja parser two more times (slightly adapted) if you want 🙃. Or I could create a jinja macro?

If it is possible to transform the input from the webui in php I could port the jinja script over and maybe even add some proper validation errors. To do that I would need more guidance though.

Note: It's fine to continue this on monday.

[raldone01]

Or would you like to add a new input type "IpPortField" which can handle ipv4,ipv6 and optional ports?
It would be great if it could expose the following to jinja:

general.xml
<form>
    <id>general.forwarders</id>
    <label>DNS Forwarders</label>
    <style>tokenize</style>
    <type>select_multiple</type>
    <allownew>true</allownew>
    <help>Set one or more hosts to send your DNS queries if the request is unknown.</help>
</form>
General.xml
<forwarders type="IpPortField">
    <Required>N</Required>
    <Ipv4>Y</Ipv4>
    <Ipv6>Y</Ipv6>
    <DefaultPort>53</DefaultPort>
</forwarders>
named.conf
{% if helpers.exists('OPNsense.bind.general.forwarders')%}
    {%- set formatted_forwarders = [] -%}
    {%- for forwarder in OPNsense.bind.general.forwarders -%}
        # forwarder could also have forwarder.ip_type = "ipv4"|"ipv6"
        {% set formatted = forwarder.ip ~ ' port ' ~ forwarder.port ~ ';' -%}
        {%- set tmp = formatted_forwarders.append(formatted) -%}
    {%- endfor -%}
    forwarders    { {{ formatted_forwarders|join(' ') }} };
{% endif -%}

[raldone01]

@fichtner Ping

[raldone01]

It could also be done like query forwarding in unbound.
This supports enabling/disabling and ip, port pairs.

[fichtner]

@raldone01 we discussed this internally today. If you want to extend the field that's ok and we suggest the simple approach taken by dnscryptproxy: https://github.com/opnsense/plugins/blob/master/dns/dnscrypt-proxy/src/opnsense/mvc/app/models/OPNsense/Dnscryptproxy/General.xml#L10L13

the validation is nonexistent but it does the job. later maybe we will add a special field type for it with the validation included but it's not high on the priority list as the core system tries to avoid dealing with "loose" IP/port configuration not otherwise related to the rest of the system. For plugins that approach can be ok.

I wouldn't recommend overcomplicating forwarders more than this. In Unbound it's done for historic and technical reasons (DoT requires more input per server) but here in bind I think it works just fine with the easy appraoch.

[raldone01]

So the jinja template is fine?
Should I just change the model type in General.xml to CSVListField?

[fichtner]

You can try it. I'm not 100% on the syntax bind requires for multiple values but it's easy enough to try.

[raldone01]

Is it fine to change files in the plugin of my live opensense?
How does the plugin update process handle modified local files?

[fichtner]

update will overwrite manual changes, but for testing that is the easiest way to do it indeed

[fichtner]

@raldone01 how are things going on your end?

[raldone01]

I still want to do it. My time messing with opnsense is limited to days where no one else is home.
I hope I will get to it on friday. (Had to sort out some urgent ipv6 routing troubles last week.)

[fichtner]

No rush, just wanted to poke you about it while looking at open PRs.

[raldone01]

Still fiddling with the basic dns setup. 🙂
Once ubound works properly I will get back to this.