From 3dac7a772d3a06741473c175f2d77f5653ec296b Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Feb 29 2012 17:58:43 +0000 Subject: Improve hostname and domain name validation DNS plugin did not check DNS zone and DNS record validity and user was thus able to create domains like "foo bar" or other invalid DNS labels which would really confuse both user and bind-dyndb-ldap plugin. This patch at first consolidates hostname/domain name validators so that they use common functions and we don't have regular expressions and other checks defined in several places. These new cleaned validators are then used for zone/record name validation. https://fedorahosted.org/freeipa/ticket/2384 --- diff --git a/API.txt b/API.txt index 73d115c..b5f5774 100644 --- a/API.txt +++ b/API.txt @@ -1626,7 +1626,7 @@ output: Output('error', (, , ), None output: Output('value', , None) command: host_add args: 1,16,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, required=True) option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False) option: Str('l', attribute=True, cli_name='locality', multivalue=False, required=False) option: Str('nshostlocation', attribute=True, cli_name='location', multivalue=False, required=False) @@ -1648,7 +1648,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA output: Output('value', , None) command: host_add_managedby args: 1,4,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('version?', exclude='webui') @@ -1658,21 +1658,21 @@ output: Output('failed', , None) output: Output('completed', , None) command: host_del args: 1,1,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=True, primary_key=True, query=True, required=True) option: Flag('updatedns?', autofill=True, default=False) output: Output('summary', (, ), None) output: Output('result', , None) output: Output('value', , None) command: host_disable args: 1,0,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) output: Output('summary', (, ), None) output: Output('result', , None) output: Output('value', , None) command: host_find args: 1,31,4 arg: Str('criteria?', noextrawhitespace=False) -option: Str('fqdn', attribute=True, autofill=False, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=False) +option: Str('fqdn', attribute=True, autofill=False, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=False) option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False) option: Str('l', attribute=True, autofill=False, cli_name='locality', multivalue=False, query=True, required=False) option: Str('nshostlocation', attribute=True, autofill=False, cli_name='location', multivalue=False, query=True, required=False) @@ -1709,7 +1709,7 @@ output: Output('count', , None) output: Output('truncated', , None) command: host_mod args: 1,19,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, required=False) option: Str('l', attribute=True, autofill=False, cli_name='locality', multivalue=False, required=False) option: Str('nshostlocation', attribute=True, autofill=False, cli_name='location', multivalue=False, required=False) @@ -1734,7 +1734,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA output: Output('value', , None) command: host_remove_managedby args: 1,4,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('version?', exclude='webui') @@ -1744,7 +1744,7 @@ output: Output('failed', , None) output: Output('completed', , None) command: host_show args: 1,5,3 -arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True) +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) option: Flag('rights', autofill=True, default=False) option: Str('out?') option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index af23e03..44fced6 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -28,7 +28,8 @@ from ipalib import Command from ipalib.parameters import Flag, Bool, Int, Decimal, Str, StrEnum, Any from ipalib.plugins.baseldap import * from ipalib import _, ngettext -from ipalib.util import validate_zonemgr, normalize_zonemgr, validate_hostname +from ipalib.util import (validate_zonemgr, normalize_zonemgr, + validate_hostname, validate_dns_label, validate_domain_name) from ipapython import dnsclient from ipapython.ipautil import valid_ip, CheckedIPAddress from ldap import explode_dn @@ -299,7 +300,7 @@ def _normalize_bind_aci(bind_acis): acis += u';' return acis -def _domain_name_validator(ugettext, value): +def _bind_hostname_validator(ugettext, value): try: # Allow domain name which is not fully qualified. These are supported # in bind and then translated as .. @@ -310,6 +311,22 @@ def _domain_name_validator(ugettext, value): return None +def _dns_record_name_validator(ugettext, value): + if value == _dns_zone_record: + return + + try: + map(lambda label:validate_dns_label(label, allow_underscore=True), \ + value.split(u'.')) + except ValueError, e: + return unicode(e) + +def _domain_name_validator(ugettext, value): + try: + validate_domain_name(value) + except ValueError, e: + return unicode(e) + def _hostname_validator(ugettext, value): try: validate_hostname(value) @@ -777,7 +794,7 @@ class AFSDBRecord(DNSRecord): maxvalue=65535, ), Str('hostname', - _domain_name_validator, + _bind_hostname_validator, label=_('Hostname'), ), ) @@ -816,7 +833,7 @@ class CNAMERecord(DNSRecord): rfc = 1035 parts = ( Str('hostname', - _domain_name_validator, + _bind_hostname_validator, label=_('Hostname'), doc=_('A hostname which this alias hostname points to'), ), @@ -837,7 +854,7 @@ class DNAMERecord(DNSRecord): rfc = 2672 parts = ( Str('target', - _domain_name_validator, + _bind_hostname_validator, label=_('Target'), ), ) @@ -916,7 +933,7 @@ class KXRecord(DNSRecord): maxvalue=65535, ), Str('exchanger', - _domain_name_validator, + _bind_hostname_validator, label=_('Exchanger'), doc=_('A host willing to act as a key exchanger'), ), @@ -1057,7 +1074,7 @@ class MXRecord(DNSRecord): maxvalue=65535, ), Str('exchanger', - _domain_name_validator, + _bind_hostname_validator, label=_('Exchanger'), doc=_('A host willing to act as a mail exchanger'), ), @@ -1069,7 +1086,7 @@ class NSRecord(DNSRecord): parts = ( Str('hostname', - _domain_name_validator, + _bind_hostname_validator, label=_('Hostname'), ), ) @@ -1083,7 +1100,7 @@ class NSECRecord(DNSRecord): parts = ( Str('next', - _domain_name_validator, + _bind_hostname_validator, label=_('Next Domain Name'), ), StrEnum('types+', @@ -1181,7 +1198,7 @@ def _srv_target_validator(ugettext, value): if value == u'.': # service not available return - return _domain_name_validator(ugettext, value) + return _bind_hostname_validator(ugettext, value) class SRVRecord(DNSRecord): rrtype = 'SRV' @@ -1426,6 +1443,7 @@ class dnszone(LDAPObject): takes_params = ( Str('idnsname', + _domain_name_validator, cli_name='name', label=_('Zone name'), doc=_('Zone name (FQDN)'), @@ -1742,6 +1760,7 @@ class dnsrecord(LDAPObject): takes_params = ( Str('idnsname', + _dns_record_name_validator, cli_name='name', label=_('Record name'), doc=_('Record name'), diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index df9ad73..0ff5237 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -31,7 +31,9 @@ from ipalib.plugins.baseldap import * from ipalib.plugins.service import split_principal from ipalib.plugins.service import validate_certificate from ipalib.plugins.service import set_certificate_attrs -from ipalib.plugins.dns import dns_container_exists, _record_types, add_records_for_host_validation, add_records_for_host +from ipalib.plugins.dns import (dns_container_exists, _record_types, + add_records_for_host_validation, add_records_for_host, + _hostname_validator, get_reverse_zone) from ipalib.plugins.dns import get_reverse_zone from ipalib import _, ngettext from ipalib import x509 @@ -97,14 +99,6 @@ EXAMPLES: ipa host-add-managedby --hosts=test2 test """) -def validate_host(ugettext, fqdn): - """ - Require at least one dot in the hostname (to support localhost.localdomain) - """ - if fqdn.find('.') == -1: - return _('Fully-qualified hostname required') - return None - def remove_fwd_ptr(ipaddr, host, domain, recordtype): api.log.debug('deleting ipaddr %s' % ipaddr) try: @@ -225,10 +219,7 @@ class host(LDAPObject): label_singular = _('Host') takes_params = ( - Str('fqdn', validate_host, - pattern='^[a-zA-Z0-9][a-zA-Z0-9-\.]{0,254}$', - pattern_errmsg='may only include letters, numbers, and -', - maxlength=255, + Str('fqdn', _hostname_validator, cli_name='hostname', label=_('Host name'), primary_key=True, @@ -481,7 +472,7 @@ class host_del(LDAPDelete): def pre_callback(self, ldap, dn, *keys, **options): # If we aren't given a fqdn, find it - if validate_host(None, keys[-1]) is not None: + if _hostname_validator(None, keys[-1]) is not None: hostentry = api.Command['host_show'](keys[-1])['result'] fqdn = hostentry['fqdn'][0] else: @@ -856,7 +847,7 @@ class host_disable(LDAPQuery): ldap = self.obj.backend # If we aren't given a fqdn, find it - if validate_host(None, keys[-1]) is not None: + if _hostname_validator(None, keys[-1]) is not None: hostentry = api.Command['host_show'](keys[-1])['result'] fqdn = hostentry['fqdn'][0] else: diff --git a/ipalib/util.py b/ipalib/util.py index 395bf0c..bbc0fa6 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -213,6 +213,36 @@ def normalize_zonemgr(zonemgr): return zonemgr +def validate_dns_label(dns_label, allow_underscore=False): + label_chars = r'a-z0-9' + underscore_err_msg = '' + if allow_underscore: + label_chars += "_" + underscore_err_msg = u' _,' + label_regex = r'^[%(chars)s]([%(chars)s-]?[%(chars)s])*$' % dict(chars=label_chars) + regex = re.compile(label_regex, re.IGNORECASE) + + if len(dns_label) > 63: + raise ValueError(_('DNS label cannot be longer that 63 characters')) + + if not regex.match(dns_label): + raise ValueError(_('only letters, numbers,%(underscore)s and - are allowed. ' \ + '- must not be the DNS label character') \ + % dict(underscore=underscore_err_msg)) + +def validate_domain_name(domain_name): + if domain_name.endswith('.'): + domain_name = domain_name[:-1] + + domain_name = domain_name.split(".") + + # apply DNS name validator to every name part + map(lambda label:validate_dns_label(label), domain_name) + + if not domain_name[-1].isalpha(): + # see RFC 1123 + raise ValueError(_('top level domain label must be alphabetic')) + def validate_zonemgr(zonemgr): """ See RFC 1033, 1035 """ regex_domain = re.compile(r'^[a-z0-9]([a-z0-9-]?[a-z0-9])*$', re.IGNORECASE) @@ -252,8 +282,7 @@ def validate_zonemgr(zonemgr): if not regex_local_part.match(local_part): raise ValueError(local_part_errmsg) - if not all(regex_domain.match(part) for part in domain.split(".")): - raise ValueError(_('domain name may only include letters, numbers, and -')) + validate_domain_name(domain) def validate_hostname(hostname, check_fqdn=True): """ See RFC 952, 1123 @@ -261,20 +290,18 @@ def validate_hostname(hostname, check_fqdn=True): :param hostname Checked value :param check_fqdn Check if hostname is fully qualified """ - regex_name = re.compile(r'^[a-z0-9]([a-z0-9-]?[a-z0-9])*$', re.IGNORECASE) - if len(hostname) > 255: raise ValueError(_('cannot be longer that 255 characters')) if hostname.endswith('.'): hostname = hostname[:-1] - if check_fqdn and '.' not in hostname: - raise ValueError(_('not fully qualified')) - - if not all(regex_name.match(part) for part in hostname.split(".")): - raise ValueError(_('only letters, numbers, and - are allowed. ' \ - '- must not be the last name character')) + if '.' not in hostname: + if check_fqdn: + raise ValueError(_('not fully qualified')) + validate_dns_label(hostname) + else: + validate_domain_name(hostname) def validate_sshpubkey(ugettext, pubkey): try: diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py index 7b1a453..e3958d2 100644 --- a/tests/test_xmlrpc/test_dns_plugin.py +++ b/tests/test_xmlrpc/test_dns_plugin.py @@ -93,6 +93,19 @@ class test_dns(Declarative): dict( + desc='Try to create zone with invalid name', + command=( + 'dnszone_add', [u'invalid zone'], { + 'idnssoamname': dnszone1_mname, + 'idnssoarname': dnszone1_rname, + 'ip_address' : u'1.2.3.4', + } + ), + expected=errors.ValidationError(name='idnsname', error=''), + ), + + + dict( desc='Create zone %r' % dnszone1, command=( 'dnszone_add', [dnszone1], { @@ -444,6 +457,13 @@ class test_dns(Declarative): dict( + desc='Try to create record with invalid name in zone %r' % dnszone1, + command=('dnsrecord_add', [dnszone1, u'invalid record'], {'arecord': u'127.0.0.1'}), + expected=errors.ValidationError(name='idnsname', error=''), + ), + + + dict( desc='Create record %r in zone %r' % (dnszone1, dnsres1), command=('dnsrecord_add', [dnszone1, dnsres1], {'arecord': u'127.0.0.1'}), expected={