On Mon, 2011-10-24 at 17:08 +0200, Martin Kosek wrote: > On Mon, 2011-10-24 at 09:02 -0400, Rob Crittenden wrote: > > Martin Kosek wrote: > > > On Fri, 2011-10-21 at 11:31 -0400, Rob Crittenden wrote: > > >> Martin Kosek wrote: > > >>> On Fri, 2011-10-14 at 14:11 -0400, Rob Crittenden wrote: > > >>>> Martin Kosek wrote: > > >>>>> Do at least a basic validation of DNS zone manager mail address. > > >>>>> > > >>>>> Do not require '@' to be in the mail address as it is not used > > >>>>> in common DNS zone configuration (in bind for example) and people > > >>>>> may be used to configure it that way. '@' is always removed by the > > >>>>> installer before the DNS zone is created. > > >>>>> > > >>>>> https://fedorahosted.org/freeipa/ticket/1966 > > >>>> > > >>>> There is already a zonemgr_callback defined for this option, can the > > >>>> verify_zonemgr call be either integrated or called from that? > > >>>> > > >>>> rob > > >>>> > > >>> > > >>> Right. Please, try this one. I also added a parser error when more than > > >>> one '@' is in the checked value. > > >>> > > >>> Martin > > >> > > >> A couple of things: > > >> > > >> In the block where you are counting @ why not add an : > > >> > > >> else: > > >> raise ValueError('address is not fully qualified') > > >> > > >> rather than looking for '.' in the result? I think it will be clearer > > >> that way. I wonder if the error should contain an example as well, are > > >> people going to know what a fully-qualified means? > > >> > > >> The regex is very strict for e-mail addresses, perhaps too much so. It > > >> doesn't allow upper-case characters, periods or _, both of which are > > >> allowed in login names. A common e-mail format is first.last@domain. > > >> > > >> rob > > > > > > I reorganized the validator a little and let people enter _ in the > > > domain name. I also added a small explanation of what we mean by > > > fully-qualified. > > > > > > Since we have the zonemgr validator available, why not use it for the > > > DNS plugin too? I enhanced the plugin to use this validator too. Please, > > > see attached patch. > > > > > > Martin > > > > NACK, a client might not have the server sub-package installed so the > > import of bindinstance will fail. > > > > I think that moving the validator into dns.py as a central location > > should work though. > > > > Otherwise looks good. > > > > rob > > Thanks. I didn't realize that user can have freeipa-admintools without > freeipa-server installed. > > I placed the validator to ipalib/util.py as we cannot place it to the > dns plugin directly as all our plugins assume that we have initialized > api. > > Updated patch attached. > > Martin
And now also with API.txt change. This patch must be cursed or something. No need to bump API version, just the validator has been added. Martin
>From 0a808e40a25b8fbf7912f73834ef632de8843e02 Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Mon, 24 Oct 2011 18:35:48 +0200 Subject: [PATCH] Add --zonemgr/--admin-mail validator Do at least a basic validation of DNS zone manager mail address. Do not require '@' to be in the mail address as the SOA record stores this value without it and people may be used to configure it that way. '@' is always removed by the installer/dns plugin before the DNS zone is created. https://fedorahosted.org/freeipa/ticket/1966 --- API.txt | 6 +++--- install/tools/ipa-dns-install | 3 ++- install/tools/ipa-server-install | 13 +------------ ipalib/plugins/dns.py | 9 +++++++++ ipalib/util.py | 30 ++++++++++++++++++++++++++++++ ipaserver/install/bindinstance.py | 17 +++++++++++++++++ 6 files changed, 62 insertions(+), 16 deletions(-) diff --git a/API.txt b/API.txt index 77ff362f943ded64a0b18443e3557f2cedf0b873..2d2f0ac4f014a5d68f1293d930e7961fb759131a 100644 --- a/API.txt +++ b/API.txt @@ -854,7 +854,7 @@ args: 1,19,3 arg: Str('idnsname', attribute=True, cli_name='name', default_from=DefaultFrom(<lambda>, 'name_from_ip'), label=Gettext('Zone name', domain='ipa', localedir=None), multivalue=False, normalizer=<lambda>, primary_key=True, required=True) option: Str('name_from_ip', _validate_ipnet, attribute=True, cli_name='name_from_ip', label=Gettext('Reverse zone IP network', domain='ipa', localedir=None), multivalue=False, required=False) option: Str('idnssoamname', attribute=True, cli_name='name_server', label=Gettext('Authoritative nameserver', domain='ipa', localedir=None), multivalue=False, required=True) -option: Str('idnssoarname', attribute=True, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, required=True) +option: Str('idnssoarname', _rname_validator, attribute=True, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, required=True) option: Int('idnssoaserial', attribute=True, autofill=True, cli_name='serial', create_default=_create_zone_serial, label=Gettext('SOA serial', domain='ipa', localedir=None), minvalue=1, multivalue=False, required=False) option: Int('idnssoarefresh', attribute=True, autofill=True, cli_name='refresh', default=3600, label=Gettext('SOA refresh', domain='ipa', localedir=None), minvalue=0, multivalue=False, required=False) option: Int('idnssoaretry', attribute=True, autofill=True, cli_name='retry', default=900, label=Gettext('SOA retry', domain='ipa', localedir=None), minvalue=0, multivalue=False, required=False) @@ -899,7 +899,7 @@ arg: Str('criteria?', noextrawhitespace=False) option: Str('idnsname', attribute=True, autofill=False, cli_name='name', default_from=DefaultFrom(<lambda>, 'name_from_ip'), label=Gettext('Zone name', domain='ipa', localedir=None), multivalue=False, normalizer=<lambda>, primary_key=True, query=True, required=False) option: Str('name_from_ip', _validate_ipnet, attribute=True, autofill=False, cli_name='name_from_ip', label=Gettext('Reverse zone IP network', domain='ipa', localedir=None), multivalue=False, query=True, required=False) option: Str('idnssoamname', attribute=True, autofill=False, cli_name='name_server', label=Gettext('Authoritative nameserver', domain='ipa', localedir=None), multivalue=False, query=True, required=False) -option: Str('idnssoarname', attribute=True, autofill=False, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, query=True, required=False) +option: Str('idnssoarname', _rname_validator, attribute=True, autofill=False, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, query=True, required=False) option: Int('idnssoaserial', attribute=True, autofill=False, cli_name='serial', create_default=_create_zone_serial, label=Gettext('SOA serial', domain='ipa', localedir=None), minvalue=1, multivalue=False, query=True, required=False) option: Int('idnssoarefresh', attribute=True, autofill=False, cli_name='refresh', default=3600, label=Gettext('SOA refresh', domain='ipa', localedir=None), minvalue=0, multivalue=False, query=True, required=False) option: Int('idnssoaretry', attribute=True, autofill=False, cli_name='retry', default=900, label=Gettext('SOA retry', domain='ipa', localedir=None), minvalue=0, multivalue=False, query=True, required=False) @@ -925,7 +925,7 @@ args: 1,18,3 arg: Str('idnsname', attribute=True, cli_name='name', default_from=DefaultFrom(<lambda>, 'name_from_ip'), label=Gettext('Zone name', domain='ipa', localedir=None), multivalue=False, normalizer=<lambda>, primary_key=True, query=True, required=True) option: Str('name_from_ip', _validate_ipnet, attribute=True, autofill=False, cli_name='name_from_ip', label=Gettext('Reverse zone IP network', domain='ipa', localedir=None), multivalue=False, required=False) option: Str('idnssoamname', attribute=True, autofill=False, cli_name='name_server', label=Gettext('Authoritative nameserver', domain='ipa', localedir=None), multivalue=False, required=False) -option: Str('idnssoarname', attribute=True, autofill=False, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, required=False) +option: Str('idnssoarname', _rname_validator, attribute=True, autofill=False, cli_name='admin_email', default_from=DefaultFrom(<lambda>, 'idnsname'), label=Gettext('Administrator e-mail address', domain='ipa', localedir=None), multivalue=False, normalizer=_rname_normalizer, required=False) option: Int('idnssoaserial', attribute=True, autofill=False, cli_name='serial', create_default=_create_zone_serial, label=Gettext('SOA serial', domain='ipa', localedir=None), minvalue=1, multivalue=False, required=False) option: Int('idnssoarefresh', attribute=True, autofill=False, cli_name='refresh', default=3600, label=Gettext('SOA refresh', domain='ipa', localedir=None), minvalue=0, multivalue=False, required=False) option: Int('idnssoaretry', attribute=True, autofill=False, cli_name='retry', default=900, label=Gettext('SOA retry', domain='ipa', localedir=None), minvalue=0, multivalue=False, required=False) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index d81b6a2e804a815d5bece8426a286e3190f6dee3..7841c21dc89a02250d513ce3ebf5c5389aac98da 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -48,7 +48,8 @@ def parse_options(): parser.add_option("--reverse-zone", dest="reverse_zone", help="The reverse DNS zone to use") parser.add_option("--no-reverse", dest="no_reverse", action="store_true", default=False, help="Do not create reverse DNS zone") - parser.add_option("--zonemgr", dest="zonemgr", + parser.add_option("--zonemgr", action="callback", callback=bindinstance.zonemgr_callback, + type="string", help="DNS zone manager e-mail address. Defaults to root") parser.add_option("--zone-notif", dest="zone_notif", action="store_true", default=False, diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 76d5f2f5af656a1947da0a5d5d855a398e34ef37..d29b806da4807531f8907229eefa783f0d570f08 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -58,7 +58,6 @@ from ipaserver.plugins.ldap2 import ldap2 from ipapython import sysrestore from ipapython.ipautil import * from ipalib import api, errors, util -from ipalib.parameters import IA5Str from ipapython.config import IPAOptionParser from ipalib.dn import DN from ipalib.x509 import load_certificate_from_file, load_certificate_chain_from_file @@ -76,16 +75,6 @@ VALID_SUBJECT_ATTRS = ['cn', 'st', 'o', 'ou', 'dnqualifier', 'c', 'incorporationlocality', 'incorporationstate', 'incorporationcountry', 'businesscategory'] -def zonemgr_callback(option, opt_str, value, parser): - """ - Make sure the zonemgr is an IA5String. - """ - name = opt_str.replace('--','') - v = unicode(value, 'utf-8') - ia = IA5Str(name) - ia._convert_scalar(v) - parser.values.zonemgr = value - def subject_callback(option, opt_str, value, parser): """ Make sure the certificate subject base is a valid DN @@ -195,7 +184,7 @@ def parse_options(): dns_group.add_option("--reverse-zone", dest="reverse_zone", help="The reverse DNS zone to use") dns_group.add_option("--no-reverse", dest="no_reverse", action="store_true", default=False, help="Do not create reverse DNS zone") - dns_group.add_option("--zonemgr", action="callback", callback=zonemgr_callback, + dns_group.add_option("--zonemgr", action="callback", callback=bindinstance.zonemgr_callback, type="string", help="DNS zone manager e-mail address. Defaults to root") dns_group.add_option("--zone-notif", dest="zone_notif", diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index f6bbb3c4f1ce35e7ee5fbc793a396425610f14f0..97eb6a6d4dea2a59baef503ddfce11c1d6bdfd03 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -26,6 +26,7 @@ from ipalib import Command from ipalib import Flag, Int, List, Str, StrEnum from ipalib.plugins.baseldap import * from ipalib import _, ngettext +from ipalib.util import validate_zonemgr from ipapython import dnsclient from ipapython.ipautil import valid_ip from ldap import explode_dn @@ -136,6 +137,13 @@ _record_attributes = [str('%srecord' % t.lower()) for t in _record_types] # supported DNS classes, IN = internet, rest is almost never used _record_classes = (u'IN', u'CS', u'CH', u'HS') +def _rname_validator(ugettext, zonemgr): + try: + validate_zonemgr(zonemgr) + except ValueError, e: + return unicode(e) + return None + # normalizer for admin email def _rname_normalizer(value): value = value.replace('@', '.') @@ -323,6 +331,7 @@ class dnszone(LDAPObject): doc=_('Authoritative nameserver domain name'), ), Str('idnssoarname', + _rname_validator, cli_name='admin_email', label=_('Administrator e-mail address'), doc=_('Administrator e-mail address'), diff --git a/ipalib/util.py b/ipalib/util.py index cc887c348c74223ca2101cd31d5954200511e3fd..fa93cc75058997104b3f452b63e83b309da1260a 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -203,3 +203,33 @@ def check_writable_file(filename): fp.close() except (IOError, OSError), e: raise errors.FileError(reason=str(e)) + + +def validate_zonemgr(zonemgr): + """ See RFC 1033, 1035 """ + regex_domain = re.compile(r'^[a-z0-9][a-z0-9-]*$', re.IGNORECASE) + regex_name = re.compile(r'^[a-z0-9][a-z0-9-_]*$', re.IGNORECASE) + + if len(zonemgr) > 255: + raise ValueError(_('cannot be longer that 255 characters')) + + if zonemgr.count('@') == 1: + name, dot, domain = zonemgr.partition('@') + elif zonemgr.count('@') > 1: + raise ValueError(_('too many \'@\' characters')) + else: + # address in SOA format already (without @) + name, dot, domain = zonemgr.partition('.') + + if domain.endswith('.'): + domain = domain[:-1] + + if '.' not in domain: + raise ValueError(_('address domain is not fully qualified ' \ + '("example.com" instead of just "example")')) + + if not regex_name.match(name): + raise ValueError(_('mail account may only include letters, numbers, -, and _')) + + if not all(regex_domain.match(part) for part in domain.split(".")): + raise ValueError(_('domain name may only include letters, numbers, and -')) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index ddf5497708ab8598d9a01fa0e555dd1ced55953b..7330264fe69f0bd5aaa63ebf58a8391f85384986 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -32,6 +32,8 @@ from ipaserver.install.installutils import resolve_host from ipapython import sysrestore from ipapython import ipautil from ipalib.constants import DNS_ZONE_REFRESH +from ipalib.parameters import IA5Str +from ipalib.util import validate_zonemgr import ipalib from ipalib import api, util, errors @@ -286,6 +288,21 @@ def get_rr(zone, name, type): return [] +def zonemgr_callback(option, opt_str, value, parser): + """ + Properly validate and convert --zonemgr Option to IA5String + """ + # validate the value first + try: + validate_zonemgr(value) + except ValueError, e: + parser.error("invalid zonemgr: " + unicode(e)) + + name = opt_str.replace('--','') + v = unicode(value, 'utf-8') + ia = IA5Str(name) + ia._convert_scalar(v) + parser.values.zonemgr = value class DnsBackup(object): def __init__(self, service): -- 1.7.6.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
