Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T6294: Service dns forwarding add the ability to configure ZonetoCache #3896

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions data/templates/dns-forwarding/recursor.conf.lua.j2
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,31 @@ dofile("/usr/share/pdns-recursor/lua-config/rootkeys.lua")

-- Load lua from vyos-hostsd --
dofile("{{ config_dir }}/recursor.vyos-hostsd.conf.lua")

-- ZoneToCache --
{% if zone_cache is vyos_defined %}
{% set option_mapping = {
'refresh': 'refreshPeriod',
'retry_interval': 'retryOnErrorPeriod',
'max_zone_size': 'maxReceivedMBytes'
} %}
{% for name, conf in zone_cache.items() %}
{% set source = conf.source.items() | first %}
{% set settings = [] %}
{% for key, val in conf.options.items() %}
{% set mapped_key = option_mapping.get(key, key) %}
{% if key == 'refresh' %}
{% set val = val['interval'] %}
{% endif %}
{% if key in ['dnssec', 'zonemd'] %}
{% set _ = settings.append(mapped_key ~ ' = "' ~ val ~ '"') %}
{% else %}
{% set _ = settings.append(mapped_key ~ ' = ' ~ val) %}
{% endif %}
{% endfor %}

zoneToCache("{{ name }}", "{{ source[0] }}", "{{ source[1] }}", { {{ settings | join(', ') }} })

{% endfor %}

{% endif %}
173 changes: 173 additions & 0 deletions interface-definitions/service_dns_forwarding.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,179 @@
</leafNode>
</children>
</node>
<tagNode name="zone-cache">
<properties>
<help>Load a zone into the recursor cache</help>
<valueHelp>
<format>txt</format>
c-po marked this conversation as resolved.
Show resolved Hide resolved
<description>Domain name</description>
</valueHelp>
<constraint>
<validator name="fqdn"/>
</constraint>
</properties>
<children>
<node name="source">
<properties>
<help>Zone source</help>
</properties>
<children>
<leafNode name="axfr">
<properties>
<help>DNS server address</help>
<valueHelp>
<format>ipv4</format>
<description>IPv4 address</description>
</valueHelp>
<valueHelp>
<format>ipv6</format>
<description>IPv6 address</description>
</valueHelp>
<constraint>
<validator name="ip-address"/>
</constraint>
</properties>
</leafNode>
<leafNode name="url">
<properties>
<help>Source URL</help>
<valueHelp>
<format>url</format>
<description>Zone file URL</description>
</valueHelp>
<constraint>
<validator name="url" argument="--scheme http --scheme https"/>
</constraint>
</properties>
</leafNode>
</children>
</node>
<node name="options">
<properties>
<help>Zone caching options</help>
</properties>
<children>
<leafNode name="timeout">
<properties>
<help>Zone retrieval timeout</help>
<valueHelp>
<format>u32:1-3600</format>
<description>Request timeout in seconds</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 1-3600"/>
</constraint>
</properties>
<defaultValue>20</defaultValue>
</leafNode>
<node name="refresh">
<properties>
<help>Zone caching options</help>
</properties>
<children>
<leafNode name="on-reload">
<properties>
<help>Retrieval zone only at startup and on reload</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="interval">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a CLI perspective (both completion help and constraint) I think it would be easier to just make this seconds only

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first version used seconds, but was reworked after this comment:
#3896 (comment)

@dmbaturin what do you think about this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do seconds for ease of validation. We can later add suffixed values, once we have a validator for them. It shouldn't be a problem because a number without any suffix is interpreted as seconds anyway, so we'll only need to relax the validation to allow human-readable values like 1d12h.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

<properties>
<help>Periodic zone retrieval interval</help>
<valueHelp>
<format>u32:0-31536000</format>
<description>Retrieval interval in seconds</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 0-31536000"/>
</constraint>
</properties>
<defaultValue>86400</defaultValue>
</leafNode>
</children>
</node>
<leafNode name="retry-interval">
<properties>
<help>Retry interval after zone retrieval errors</help>
<valueHelp>
<format>u32:1-86400</format>
<description>Retry period in seconds</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 1-86400"/>
</constraint>
</properties>
<defaultValue>60</defaultValue>
</leafNode>
<leafNode name="max-zone-size">
<properties>
<help>Maximum zone size in megabytes</help>
<valueHelp>
<format>u32:0</format>
<description>No restriction</description>
</valueHelp>
<valueHelp>
<format>u32:1-1024</format>
<description>Size in megabytes</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 0-1024"/>
</constraint>
</properties>
<defaultValue>0</defaultValue>
</leafNode>
<leafNode name="zonemd">
<properties>
<help>Message Digest for DNS Zones (RFC 8976)</help>
<completionHelp>
<list>ignore validate require</list>
</completionHelp>
<valueHelp>
<format>ignore</format>
<description>Ignore ZONEMD records</description>
</valueHelp>
<valueHelp>
<format>validate</format>
<description>Validate ZONEMD if present</description>
</valueHelp>
<valueHelp>
<format>require</format>
<description>Require valid ZONEMD record to be present</description>
</valueHelp>
<constraint>
<regex>(ignore|validate|require)</regex>
</constraint>
</properties>
<defaultValue>validate</defaultValue>
</leafNode>
<leafNode name="dnssec">
<properties>
<help>DNSSEC mode</help>
<completionHelp>
<list>ignore validate require</list>
</completionHelp>
<valueHelp>
<format>ignore</format>
<description>Do not do DNSSEC validation</description>
</valueHelp>
<valueHelp>
<format>validate</format>
<description>Reject zones with incorrect signatures but accept unsigned zones</description>
</valueHelp>
<valueHelp>
<format>require</format>
<description>Require DNSSEC validation</description>
</valueHelp>
<constraint>
<regex>(ignore|validate|require)</regex>
</constraint>
</properties>
<defaultValue>validate</defaultValue>
</leafNode>
</children>
</node>
</children>
</tagNode>
</children>
</node>
</children>
Expand Down
62 changes: 47 additions & 15 deletions python/vyos/utils/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,41 +12,72 @@
#
# You should have received a copy of the GNU Lesser General Public
# License along with this library. If not, see <http://www.gnu.org/licenses/>.
import re

# Define the number of seconds in each time unit
time_units = {
'y': 60 * 60 * 24 * 365.25, # year
'w': 60 * 60 * 24 * 7, # week
'd': 60 * 60 * 24, # day
'h': 60 * 60, # hour
'm': 60, # minute
's': 1 # second
}


def human_to_seconds(time_str):
""" Converts a human-readable interval such as 1w4d18h35m59s
to number of seconds
"""

time_patterns = {
'y': r'(\d+)\s*y',
'w': r'(\d+)\s*w',
'd': r'(\d+)\s*d',
'h': r'(\d+)\s*h',
'm': r'(\d+)\s*m',
's': r'(\d+)\s*s'
}

total_seconds = 0

for unit, pattern in time_patterns.items():
match = re.search(pattern, time_str)
if match:
value = int(match.group(1))
total_seconds += value * time_units[unit]

return int(total_seconds)


def seconds_to_human(s, separator=""):
""" Converts number of seconds passed to a human-readable
interval such as 1w4d18h35m59s
"""
s = int(s)

year = 60 * 60 * 24 * 365.25
week = 60 * 60 * 24 * 7
day = 60 * 60 * 24
hour = 60 * 60

result = []

years = s // year
years = s // time_units['y']
if years > 0:
result.append(f'{int(years)}y')
s = int(s % year)
s = int(s % time_units['y'])

weeks = s // week
weeks = s // time_units['w']
if weeks > 0:
result.append(f'{weeks}w')
s = s % week
s = s % time_units['w']

days = s // day
days = s // time_units['d']
if days > 0:
result.append(f'{days}d')
s = s % day
s = s % time_units['d']

hours = s // hour
hours = s // time_units['h']
if hours > 0:
result.append(f'{hours}h')
s = s % hour
s = s % time_units['h']

minutes = s // 60
minutes = s // time_units['m']
if minutes > 0:
result.append(f'{minutes}m')
s = s % 60
Expand All @@ -57,6 +88,7 @@

return separator.join(result)


def bytes_to_human(bytes, initial_exponent=0, precision=2,
int_below_exponent=0):
""" Converts a value in bytes to a human-readable size string like 640 KB
Expand Down Expand Up @@ -149,7 +181,7 @@
net = ip_network(prefix, strict=False)
euil = int('0x{0}'.format(eui64), 16)
return str(net[euil])
except: # pylint: disable=bare-except

Check failure on line 184 in python/vyos/utils/convert.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E722)

python/vyos/utils/convert.py:184:9: E722 Do not use bare `except`
return


Expand Down
39 changes: 39 additions & 0 deletions smoketest/scripts/cli/test_service_dns_forwarding.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

PDNS_REC_RUN_DIR = '/run/pdns-recursor'
CONFIG_FILE = f'{PDNS_REC_RUN_DIR}/recursor.conf'
PDNS_REC_LUA_CONF_FILE = f'{PDNS_REC_RUN_DIR}/recursor.conf.lua'
FORWARD_FILE = f'{PDNS_REC_RUN_DIR}/recursor.forward-zones.conf'
HOSTSD_FILE = f'{PDNS_REC_RUN_DIR}/recursor.vyos-hostsd.conf.lua'
PROCESS_NAME= 'pdns_recursor'
Expand Down Expand Up @@ -187,8 +188,8 @@
hosts_conf = read_file(HOSTSD_FILE)
for domain in domains:
# Test 'recursion-desired' flag for the first domain only
if domain == domains[0]: key =f'\+{domain}'

Check failure on line 191 in smoketest/scripts/cli/test_service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E701)

smoketest/scripts/cli/test_service_dns_forwarding.py:191:36: E701 Multiple statements on one line (colon)
else: key =f'{domain}'

Check failure on line 192 in smoketest/scripts/cli/test_service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E701)

smoketest/scripts/cli/test_service_dns_forwarding.py:192:17: E701 Multiple statements on one line (colon)
tmp = get_config_value(key, file=FORWARD_FILE)
canonical_entries = [(lambda h, p: f"{bracketize_ipv6(h)}:{p['port'] if 'port' in p else 53}")(h, p)
for (h, p) in nameservers.items()]
Expand Down Expand Up @@ -300,6 +301,44 @@
self.assertRegex(zone_config, fr'test\s+\d+\s+NS\s+ns1\.{test_zone}\.')
self.assertRegex(zone_config, fr'test\s+\d+\s+NS\s+ns2\.{test_zone}\.')

def test_zone_cache_url(self):
self.cli_set(base_path + ['zone-cache', 'smoketest', 'source', 'url', 'https://www.internic.net/domain/root.zone'])
self.cli_commit()

lua_config = read_file(PDNS_REC_LUA_CONF_FILE)
self.assertIn('zoneToCache("smoketest", "url", "https://www.internic.net/domain/root.zone", { dnssec = "validate", zonemd = "validate", maxReceivedMBytes = 0, retryOnErrorPeriod = 60, refreshPeriod = 86400, timeout = 20 })', lua_config)

def test_zone_cache_axfr(self):

self.cli_set(base_path + ['zone-cache', 'smoketest', 'source', 'axfr', '127.0.0.1'])
self.cli_commit()

lua_config = read_file(PDNS_REC_LUA_CONF_FILE)
self.assertIn('zoneToCache("smoketest", "axfr", "127.0.0.1", { dnssec = "validate", zonemd = "validate", maxReceivedMBytes = 0, retryOnErrorPeriod = 60, refreshPeriod = 86400, timeout = 20 })', lua_config)

def test_zone_cache_options(self):
self.cli_set(base_path + ['zone-cache', 'smoketest', 'source', 'url', 'https://www.internic.net/domain/root.zone'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'dnssec', 'ignore'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'max-zone-size', '100'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'refresh', 'interval', '10'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'retry-interval', '90'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'timeout', '50'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'options', 'zonemd', 'require'])
self.cli_commit()

lua_config = read_file(PDNS_REC_LUA_CONF_FILE)
self.assertIn('zoneToCache("smoketest", "url", "https://www.internic.net/domain/root.zone", { dnssec = "ignore", maxReceivedMBytes = 100, refreshPeriod = 10, retryOnErrorPeriod = 90, timeout = 50, zonemd = "require" })', lua_config)

def test_zone_cache_wrong_source(self):
self.cli_set(base_path + ['zone-cache', 'smoketest', 'source', 'url', 'https://www.internic.net/domain/root.zone'])
self.cli_set(base_path + ['zone-cache', 'smoketest', 'source', 'axfr', '127.0.0.1'])

with self.assertRaises(ConfigSessionError):
self.cli_commit()
# correct config to correct finish the test
self.cli_delete(base_path + ['zone-cache', 'smoketest', 'source', 'axfr'])
self.cli_commit()


if __name__ == '__main__':
unittest.main(verbosity=2)
20 changes: 20 additions & 0 deletions src/conf_mode/service_dns_forwarding.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
dns['authoritative_zone_errors'] = []
for node in dns['authoritative_domain']:
zonedata = dns['authoritative_domain'][node]
if ('disable' in zonedata) or (not 'records' in zonedata):

Check failure on line 71 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:71:48: E713 Test for membership should be `not in`
continue
zone = {
'name': node,
Expand All @@ -88,7 +88,7 @@
rdata = recorddata[rtype][subnode]

if rtype in [ 'a', 'aaaa' ]:
if not 'address' in rdata:

Check failure on line 91 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:91:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: at least one address is required')
continue

Expand All @@ -103,7 +103,7 @@
'value': address
})
elif rtype in ['cname', 'ptr']:
if not 'target' in rdata:

Check failure on line 106 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:106:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: target is required')
continue

Expand All @@ -114,7 +114,7 @@
'value': '{}.'.format(rdata['target'])
})
elif rtype == 'ns':
if not 'target' in rdata:

Check failure on line 117 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:117:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: at least one target is required')
continue

Expand All @@ -127,7 +127,7 @@
})

elif rtype == 'mx':
if not 'server' in rdata:

Check failure on line 130 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:130:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: at least one server is required')
continue

Expand All @@ -140,7 +140,7 @@
'value': '{} {}.'.format(serverdata['priority'], servername)
})
elif rtype == 'txt':
if not 'value' in rdata:

Check failure on line 143 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:143:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: at least one value is required')
continue

Expand All @@ -152,7 +152,7 @@
'value': "\"{}\"".format(value.replace("\"", "\\\""))
})
elif rtype == 'spf':
if not 'value' in rdata:

Check failure on line 155 in src/conf_mode/service_dns_forwarding.py

View workflow job for this annotation

GitHub Actions / ruff-lint / ruff-lint

Ruff (E713)

src/conf_mode/service_dns_forwarding.py:155:32: E713 Test for membership should be `not in`
dns['authoritative_zone_errors'].append(f'{subnode}.{node}: value is required')
continue

Expand Down Expand Up @@ -224,6 +224,18 @@

dns['authoritative_zones'].append(zone)

if 'zone_cache' in dns:
# convert refresh interval to sec:
for _, zone_conf in dns['zone_cache'].items():
if 'options' in zone_conf \
and 'refresh' in zone_conf['options']:

if 'on_reload' in zone_conf['options']['refresh']:
interval = 0
else:
interval = zone_conf['options']['refresh']['interval']
zone_conf['options']['refresh']['interval'] = interval

return dns

def verify(dns):
Expand Down Expand Up @@ -259,8 +271,16 @@
if not 'system_name_server' in dns:
print('Warning: No "system name-server" configured')

if 'zone_cache' in dns:
for name, conf in dns['zone_cache'].items():
if ('source' not in conf) \
or ('url' in conf['source'] and 'axfr' in conf['source']):
raise ConfigError(f'Invalid configuration for zone "{name}": '
f'Please select one source type "url" or "axfr".')

return None


def generate(dns):
# bail out early - looks like removal from running config
if not dns:
Expand Down
Loading