diff options
| -rw-r--r-- | interface-definitions/dns-dynamic.xml.in | 4 | ||||
| -rwxr-xr-x | smoketest/scripts/cli/test_service_dns_dynamic.py | 39 | ||||
| -rwxr-xr-x | src/conf_mode/dns_dynamic.py | 36 | ||||
| -rwxr-xr-x | src/migration-scripts/dns-dynamic/0-to-1 | 40 | ||||
| -rwxr-xr-x | src/migration-scripts/dns-dynamic/2-to-3 | 45 |
5 files changed, 133 insertions, 31 deletions
diff --git a/interface-definitions/dns-dynamic.xml.in b/interface-definitions/dns-dynamic.xml.in index f089f0e52..388e7c5d2 100644 --- a/interface-definitions/dns-dynamic.xml.in +++ b/interface-definitions/dns-dynamic.xml.in @@ -19,6 +19,10 @@ <format>txt</format> <description>Dynamic DNS service name</description> </valueHelp> + <constraint> + #include <include/constraint/alpha-numeric-hyphen-underscore.xml.i> + </constraint> + <constraintErrorMessage>Dynamic DNS service name must be alphanumeric and can contain hyphens and underscores</constraintErrorMessage> </properties> <children> #include <include/generic-description.xml.i> diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py index 3c7303f32..ae46b18ba 100755 --- a/smoketest/scripts/cli/test_service_dns_dynamic.py +++ b/smoketest/scripts/cli/test_service_dns_dynamic.py @@ -17,8 +17,6 @@ import os import unittest import tempfile -import random -import string from base_vyostest_shim import VyOSUnitTestSHIM @@ -67,14 +65,12 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): self.cli_set(name_path + [svc, 'address', interface]) self.cli_set(name_path + [svc, 'host-name', hostname]) self.cli_set(name_path + [svc, 'password', password]) - self.cli_set(name_path + [svc, 'zone', zone]) - self.cli_set(name_path + [svc, 'ttl', ttl]) for opt, value in details.items(): self.cli_set(name_path + [svc, opt, value]) - # 'zone' option is supported and required by 'cloudfare', but not 'freedns' and 'zoneedit' + # 'zone' option is supported by 'cloudfare' and 'zoneedit1', but not 'freedns' self.cli_set(name_path + [svc, 'zone', zone]) - if details['protocol'] == 'cloudflare': + if details['protocol'] in ['cloudflare', 'zoneedit1']: pass else: # exception is raised for unsupported ones @@ -292,7 +288,36 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): self.assertIn(f'password=\'{password}\'', ddclient_conf) self.assertIn(f'{hostname}', ddclient_conf) - def test_07_dyndns_vrf(self): + def test_07_dyndns_dynamic_interface(self): + # Check if DDNS service can be configured and runs + svc_path = name_path + ['namecheap'] + proto = 'namecheap' + dyn_interface = 'pppoe587' + + self.cli_set(svc_path + ['address', dyn_interface]) + self.cli_set(svc_path + ['protocol', proto]) + self.cli_set(svc_path + ['server', server]) + self.cli_set(svc_path + ['username', username]) + self.cli_set(svc_path + ['password', password]) + self.cli_set(svc_path + ['host-name', hostname]) + + # Dynamic interface will raise a warning but still go through + # XXX: We should have idiomatic class "ConfigSessionWarning" wrapping + # "Warning" similar to "ConfigSessionError". + # with self.assertWarns(Warning): + # self.cli_commit() + self.cli_commit() + + # Check the generating config parameters + ddclient_conf = cmd(f'sudo cat {DDCLIENT_CONF}') + self.assertIn(f'ifv4={dyn_interface}', ddclient_conf) + self.assertIn(f'protocol={proto}', ddclient_conf) + self.assertIn(f'server={server}', ddclient_conf) + self.assertIn(f'login={username}', ddclient_conf) + self.assertIn(f'password=\'{password}\'', ddclient_conf) + self.assertIn(f'{hostname}', ddclient_conf) + + def test_08_dyndns_vrf(self): # Table number randomized, but should be within range 100-65535 vrf_table = '58710' vrf_name = f'vyos-test-{vrf_table}' diff --git a/src/conf_mode/dns_dynamic.py b/src/conf_mode/dns_dynamic.py index c4dcb76ed..99fa8feee 100755 --- a/src/conf_mode/dns_dynamic.py +++ b/src/conf_mode/dns_dynamic.py @@ -15,7 +15,7 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. import os - +import re from sys import exit from vyos.base import Warning @@ -30,6 +30,9 @@ airbag.enable() config_file = r'/run/ddclient/ddclient.conf' systemd_override = r'/run/systemd/system/ddclient.service.d/override.conf' +# Dynamic interfaces that might not exist when the configuration is loaded +dynamic_interfaces = ('pppoe', 'sstpc') + # Protocols that require zone zone_necessary = ['cloudflare', 'digitalocean', 'godaddy', 'hetzner', 'gandi', 'nfsn', 'nsupdate'] @@ -86,17 +89,29 @@ def verify(dyndns): if field not in config: raise ConfigError(f'"{field.replace("_", "-")}" {error_msg_req}') - # If dyndns address is an interface, ensure that it exists + # If dyndns address is an interface, ensure + # that the interface exists (or just warn if dynamic interface) # and that web-options are not set if config['address'] != 'web': # exclude check interface for dynamic interfaces - interface_filter = ('pppoe', 'sstpc') - if config['address'].startswith(interface_filter): - Warning(f'interface {config["address"]} does not exist!') + if config['address'].startswith(dynamic_interfaces): + Warning(f'Interface "{config["address"]}" does not exist yet and cannot ' + f'be used for Dynamic DNS service "{service}" until it is up!') else: verify_interface_exists(config['address']) if 'web_options' in config: - raise ConfigError(f'"web-options" is applicable only when using HTTP(S) web request to obtain the IP address') + raise ConfigError(f'"web-options" is applicable only when using HTTP(S) ' + f'web request to obtain the IP address') + + # Warn if using checkip.dyndns.org, as it does not support HTTPS + # See: https://github.com/ddclient/ddclient/issues/597 + if 'web_options' in config: + if 'url' not in config['web_options']: + raise ConfigError(f'"url" in "web-options" {error_msg_req} ' + f'with protocol "{config["protocol"]}"') + elif re.search("^(https?://)?checkip\.dyndns\.org", config['web_options']['url']): + Warning(f'"checkip.dyndns.org" does not support HTTPS requests for IP address ' + f'lookup. Please use a different IP address lookup service.') # RFC2136 uses 'key' instead of 'password' if config['protocol'] != 'nsupdate' and 'password' not in config: @@ -124,13 +139,16 @@ def verify(dyndns): if config['ip_version'] == 'both': if config['protocol'] not in dualstack_supported: - raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} with protocol "{config["protocol"]}"') + raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} ' + f'with protocol "{config["protocol"]}"') # dyndns2 protocol in ddclient honors dual stack only for dyn.com (dyndns.org) if config['protocol'] == 'dyndns2' and 'server' in config and config['server'] not in dyndns_dualstack_servers: - raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} for "{config["server"]}" with protocol "{config["protocol"]}"') + raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} ' + f'for "{config["server"]}" with protocol "{config["protocol"]}"') if {'wait_time', 'expiry_time'} <= config.keys() and int(config['expiry_time']) < int(config['wait_time']): - raise ConfigError(f'"expiry-time" must be greater than "wait-time" for Dynamic DNS service "{service}"') + raise ConfigError(f'"expiry-time" must be greater than "wait-time" for ' + f'Dynamic DNS service "{service}"') return None diff --git a/src/migration-scripts/dns-dynamic/0-to-1 b/src/migration-scripts/dns-dynamic/0-to-1 index d80e8d44a..b7674a9c8 100755 --- a/src/migration-scripts/dns-dynamic/0-to-1 +++ b/src/migration-scripts/dns-dynamic/0-to-1 @@ -25,8 +25,10 @@ # to "service dns dynamic address <address> service <config> username ..." # - apply global 'ipv6-enable' to per <config> 'ip-version: ipv6' # - apply service protocol mapping upfront, they are not 'auto-detected' anymore +# - migrate web-options url to stricter format import sys +import re from vyos.configtree import ConfigTree service_protocol_mapping = { @@ -81,20 +83,42 @@ for address in config.list_nodes(new_base_path): config.rename(new_base_path + [address, 'service', svc_cfg, 'login'], 'username') # Apply global 'ipv6-enable' to per <config> 'ip-version: ipv6' if config.exists(new_base_path + [address, 'ipv6-enable']): - config.set(new_base_path + [address, 'service', svc_cfg, 'ip-version'], - value='ipv6', replace=False) + config.set(new_base_path + [address, 'service', svc_cfg, 'ip-version'], 'ipv6') config.delete(new_base_path + [address, 'ipv6-enable']) # Apply service protocol mapping upfront, they are not 'auto-detected' anymore if svc_cfg in service_protocol_mapping: config.set(new_base_path + [address, 'service', svc_cfg, 'protocol'], - value=service_protocol_mapping.get(svc_cfg), replace=False) + service_protocol_mapping.get(svc_cfg)) - # Migrate "service dns dynamic interface <interface> use-web" - # to "service dns dynamic address <address> web-options" - # Also, rename <address> to 'web' literal for backward compatibility + # If use-web is set, then: + # Move "service dns dynamic address <address> <service|rfc2136> <service> ..." + # to "service dns dynamic address web <service|rfc2136> <service>-<address> ..." + # Move "service dns dynamic address web use-web ..." + # to "service dns dynamic address web web-options ..." + # Note: The config is named <service>-<address> to avoid name conflict with old entries if config.exists(new_base_path + [address, 'use-web']): - config.rename(new_base_path + [address], 'web') - config.rename(new_base_path + ['web', 'use-web'], 'web-options') + for svc_type in ['rfc2136', 'service']: + if config.exists(new_base_path + [address, svc_type]): + config.set(new_base_path + ['web', svc_type]) + config.set_tag(new_base_path + ['web', svc_type]) + for svc_cfg in config.list_nodes(new_base_path + [address, svc_type]): + config.copy(new_base_path + [address, svc_type, svc_cfg], + new_base_path + ['web', svc_type, f'{svc_cfg}-{address}']) + + # Multiple web-options were not supported, so copy only the first one + # Also, migrate web-options url to stricter format and transition + # checkip.dyndns.org to https://domains.google.com/checkip for better + # TLS support (see: https://github.com/ddclient/ddclient/issues/597) + if not config.exists(new_base_path + ['web', 'web-options']): + config.copy(new_base_path + [address, 'use-web'], new_base_path + ['web', 'web-options']) + if config.exists(new_base_path + ['web', 'web-options', 'url']): + url = config.return_value(new_base_path + ['web', 'web-options', 'url']) + if re.search("^(https?://)?checkip\.dyndns\.org", url): + config.set(new_base_path + ['web', 'web-options', 'url'], 'https://domains.google.com/checkip') + if not url.startswith(('http://', 'https://')): + config.set(new_base_path + ['web', 'web-options', 'url'], f'https://{url}') + + config.delete(new_base_path + [address]) try: with open(file_name, 'w') as f: diff --git a/src/migration-scripts/dns-dynamic/2-to-3 b/src/migration-scripts/dns-dynamic/2-to-3 index 187c2a895..4e0aa37d5 100755 --- a/src/migration-scripts/dns-dynamic/2-to-3 +++ b/src/migration-scripts/dns-dynamic/2-to-3 @@ -21,10 +21,27 @@ # to "service dns dynamic name <service> address <interface> protocol 'nsupdate'" # - migrate "service dns dynamic address <interface> service <service> ..." # to "service dns dynamic name <service> address <interface> ..." +# - normalize the all service names to conform with name constraints import sys +import re +from unicodedata import normalize from vyos.configtree import ConfigTree +def normalize_name(name): + """Normalize service names to conform with name constraints. + + This is necessary as part of migration because there were no constraints in + the old name format. + """ + # Normalize unicode characters to ASCII (NFKD) + # Replace all separators with hypens, strip leading and trailing hyphens + name = normalize('NFKD', name).encode('ascii', 'ignore').decode() + name = re.sub(r'(\s|_|\W)+', '-', name).strip('-') + + return name + + if len(sys.argv) < 2: print("Must specify file name!") sys.exit(1) @@ -64,22 +81,36 @@ for address in config.list_nodes(address_path): for svc_type in ['service', 'rfc2136']: if config.exists(address_path_tag + [svc_type]): - # Move RFC2136 as service configuration, rename to avoid name conflict and set protocol to 'nsupdate' + # Set protocol to 'nsupdate' for RFC2136 configuration if svc_type == 'rfc2136': - for rfc_cfg_old in config.list_nodes(address_path_tag + ['rfc2136']): - rfc_cfg_new = f'{rfc_cfg_old}-rfc2136' - config.rename(address_path_tag + ['rfc2136', rfc_cfg_old], rfc_cfg_new) - config.set(address_path_tag + ['rfc2136', rfc_cfg_new, 'protocol'], 'nsupdate') + for rfc_cfg in config.list_nodes(address_path_tag + ['rfc2136']): + config.set(address_path_tag + ['rfc2136', rfc_cfg, 'protocol'], 'nsupdate') # Add address as config value in each service before moving the service path - # And then copy the services from 'address <interface> service <service>' to 'name <service>' + # And then copy the services from 'address <interface> service <service>' + # to 'name (service|rfc2136)-<service>-<address>' + # Note: The new service is named (service|rfc2136)-<service>-<address> + # to avoid name conflict with old entries for svc_cfg in config.list_nodes(address_path_tag + [svc_type]): config.set(address_path_tag + [svc_type, svc_cfg, 'address'], address) - config.copy(address_path_tag + [svc_type, svc_cfg], name_path + [svc_cfg]) + config.copy(address_path_tag + [svc_type, svc_cfg], + name_path + ['-'.join([svc_type, svc_cfg, address])]) # Finally cleanup the old address path config.delete(address_path) +# Normalize the all service names to conform with name constraints +index = 1 +for name in config.list_nodes(name_path): + new_name = normalize_name(name) + if new_name != name: + # Append index if there is still a name conflicts after normalization + # For example, "foo-?(" and "foo-!)" both normalize to "foo-" + if config.exists(name_path + [new_name]): + new_name = f'{new_name}-{index}' + index += 1 + config.rename(name_path + [name], new_name) + try: with open(file_name, 'w') as f: f.write(config.to_string()) |