From 5cfee2338d548035151926c5c235f3426fca0499 Mon Sep 17 00:00:00 2001 From: Ondrej Hamada Date: Tue, 27 Mar 2012 15:15:20 +0200 Subject: [PATCH] Netgroup nisdomain and hosts validation nisdomain validation: Added pattern to the 'nisdomain' parameter to validate the specified nisdomain name. According to most common use cases the same pattern as for netgroup should fit. Unit-tests added. https://fedorahosted.org/freeipa/ticket/2448 'add_external_pre_callback' function was created to allow validation of all external members. Validation is based on usage of objects primary key parameter. The 'add_external_pre_callback' fucntion has to be called directly from in the 'pre_callback' function. This change affects netgroup, hbacrule and sudorule commands. For hostname, the validator allows non-fqdn and underscore characters. validate_hostname function in ipalib.util was modified and contains additional option that allows hostname to contain underscore characters. This option is disabled by default. Unit-tests added. https://fedorahosted.org/freeipa/ticket/2447 --- API.txt | 6 +-- VERSION | 2 +- ipalib/plugins/baseldap.py | 29 ++++++++++- ipalib/plugins/hbacrule.py | 2 +- ipalib/plugins/netgroup.py | 9 ++++ ipalib/plugins/sudorule.py | 8 +-- ipalib/util.py | 10 ++-- tests/test_xmlrpc/test_hbac_plugin.py | 13 +++++ tests/test_xmlrpc/test_netgroup_plugin.py | 57 +++++++++++++++------ tests/test_xmlrpc/test_sudorule_plugin.py | 60 +++++++++++++++++++++++ 10 files changed, 165 insertions(+), 31 deletions(-) diff --git a/API.txt b/API.txt index 363ba5a5e..e9eb1e1fa 100644 --- a/API.txt +++ b/API.txt @@ -1923,7 +1923,7 @@ command: netgroup_add args: 1,9,3 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', primary_key=True, required=True) option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=True) -option: Str('nisdomainname', attribute=True, cli_name='nisdomain', multivalue=False, required=False) +option: Str('nisdomainname', attribute=True, cli_name='nisdomain', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', required=False) option: StrEnum('usercategory', attribute=True, cli_name='usercat', multivalue=False, required=False, values=(u'all',)) option: StrEnum('hostcategory', attribute=True, cli_name='hostcat', multivalue=False, required=False, values=(u'all',)) option: Str('setattr*', cli_name='setattr', exclude='webui') @@ -1960,7 +1960,7 @@ args: 1,26,4 arg: Str('criteria?', noextrawhitespace=False) option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', 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('nisdomainname', attribute=True, autofill=False, cli_name='nisdomain', multivalue=False, query=True, required=False) +option: Str('nisdomainname', attribute=True, autofill=False, cli_name='nisdomain', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', query=True, required=False) option: Str('ipauniqueid', attribute=True, autofill=False, cli_name='uuid', multivalue=False, query=True, required=False) option: StrEnum('usercategory', attribute=True, autofill=False, cli_name='usercat', multivalue=False, query=True, required=False, values=(u'all',)) option: StrEnum('hostcategory', attribute=True, autofill=False, cli_name='hostcat', multivalue=False, query=True, required=False, values=(u'all',)) @@ -1992,7 +1992,7 @@ command: netgroup_mod args: 1,11,3 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', primary_key=True, query=True, required=True) option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, required=False) -option: Str('nisdomainname', attribute=True, autofill=False, cli_name='nisdomain', multivalue=False, required=False) +option: Str('nisdomainname', attribute=True, autofill=False, cli_name='nisdomain', multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$', pattern_errmsg='may only include letters, numbers, _, -, and .', required=False) option: StrEnum('usercategory', attribute=True, autofill=False, cli_name='usercat', multivalue=False, required=False, values=(u'all',)) option: StrEnum('hostcategory', attribute=True, autofill=False, cli_name='hostcat', multivalue=False, required=False, values=(u'all',)) option: Str('setattr*', cli_name='setattr', exclude='webui') diff --git a/VERSION b/VERSION index f26e8bfa8..46d98eedc 100644 --- a/VERSION +++ b/VERSION @@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=31 +IPA_API_VERSION_MINOR=32 diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index a09e00fef..38f369a77 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -33,7 +33,7 @@ from ipalib.base import NameSpace from ipalib.cli import to_cli, from_cli from ipalib import output from ipalib.text import _ -from ipalib.util import json_serialize +from ipalib.util import json_serialize, validate_hostname from ipalib.dn import * global_output_params = ( @@ -313,6 +313,33 @@ def wait_for_value(ldap, dn, attr, value): return entry_attrs +def add_external_pre_callback(membertype, ldap, dn, keys, options): + """ + Pre callback to validate external members. + + This should be called by a command pre callback directly. + + membertype is the type of member + """ + # validate hostname with allowed underscore characters, non-fqdn + # hostnames are allowed + def validate_host(hostname): + validate_hostname(hostname, check_fqdn=False, allow_underscore=True) + + if membertype in options: + if membertype == 'host': + validator = validate_host + else: + validator = api.Object[membertype].primary_key + for value in options[membertype]: + try: + validator(value) + except errors.ValidationError as e: + raise errors.ValidationError(name=membertype, error=e.error) + except ValueError as e: + raise errors.ValidationError(name=membertype, error=e) + return dn + def add_external_post_callback(memberattr, membertype, externalattr, ldap, completed, failed, dn, entry_attrs, *keys, **options): """ Post callback to add failed members as external members. diff --git a/ipalib/plugins/hbacrule.py b/ipalib/plugins/hbacrule.py index 466648556..eb5cb696e 100644 --- a/ipalib/plugins/hbacrule.py +++ b/ipalib/plugins/hbacrule.py @@ -498,7 +498,7 @@ class hbacrule_add_sourcehost(LDAPAddMember): if 'sourcehostcategory' in entry_attrs and \ entry_attrs['sourcehostcategory'][0].lower() == 'all': raise errors.MutuallyExclusiveError(reason="source hosts cannot be added when sourcehost category='all'") - return dn + return add_external_pre_callback('host', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('sourcehost', 'host', 'externalhost', ldap, completed, failed, dn, entry_attrs, keys, options) diff --git a/ipalib/plugins/netgroup.py b/ipalib/plugins/netgroup.py index 2ba154649..06372a592 100644 --- a/ipalib/plugins/netgroup.py +++ b/ipalib/plugins/netgroup.py @@ -53,6 +53,11 @@ EXAMPLES: NETGROUP_PATTERN='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]+$' NETGROUP_PATTERN_ERRMSG='may only include letters, numbers, _, -, and .' +# according to most common use cases the netgroup pattern should fit +# also the nisdomain pattern +NISDOMAIN_PATTERN=NETGROUP_PATTERN +NISDOMAIN_PATTERN_ERRMSG=NETGROUP_PATTERN_ERRMSG + output_params = ( Str('memberuser_user?', label='Member User', @@ -118,6 +123,8 @@ class netgroup(LDAPObject): doc=_('Netgroup description'), ), Str('nisdomainname?', + pattern=NISDOMAIN_PATTERN, + pattern_errmsg=NISDOMAIN_PATTERN_ERRMSG, cli_name='nisdomain', label=_('NIS domain name'), ), @@ -255,6 +262,8 @@ class netgroup_add_member(LDAPAddMember): member_attributes = ['memberuser', 'memberhost', 'member'] has_output_params = LDAPAddMember.has_output_params + output_params + def pre_callback(self, ldap, dn, found, not_found, *keys, **options): + return add_external_pre_callback('host', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('memberhost', 'host', 'externalhost', ldap, completed, failed, dn, entry_attrs, keys, options) diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py index de7a7af37..7432bc42b 100644 --- a/ipalib/plugins/sudorule.py +++ b/ipalib/plugins/sudorule.py @@ -431,7 +431,7 @@ class sudorule_add_user(LDAPAddMember): self.obj.handle_not_found(*keys) if is_all(_entry_attrs, 'usercategory'): raise errors.MutuallyExclusiveError(reason=_("users cannot be added when user category='all'")) - return dn + return add_external_pre_callback('user', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('memberuser', 'user', 'externaluser', ldap, completed, failed, dn, entry_attrs, keys, options) @@ -464,7 +464,7 @@ class sudorule_add_host(LDAPAddMember): self.obj.handle_not_found(*keys) if is_all(_entry_attrs, 'hostcategory'): raise errors.MutuallyExclusiveError(reason=_("hosts cannot be added when host category='all'")) - return dn + return add_external_pre_callback('host', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('memberhost', 'host', 'externalhost', ldap, completed, failed, dn, entry_attrs, keys, options) @@ -517,7 +517,7 @@ class sudorule_add_runasuser(LDAPAddMember): error=unicode(_("RunAsUser does not accept '%(name)s' as a group name")) % dict(name=name)) - return dn + return add_external_pre_callback('user', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('ipasudorunas', 'user', 'ipasudorunasextuser', ldap, completed, failed, dn, entry_attrs, keys, options) @@ -565,7 +565,7 @@ class sudorule_add_runasgroup(LDAPAddMember): error=unicode(_("RunAsGroup does not accept '%(name)s' as a group name")) % dict(name=name)) - return dn + return add_external_pre_callback('group', ldap, dn, keys, options) def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options): return add_external_post_callback('ipasudorunasgroup', 'group', 'ipasudorunasextgroup', ldap, completed, failed, dn, entry_attrs, keys, options) diff --git a/ipalib/util.py b/ipalib/util.py index bbc0fa674..a79f41cc3 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -230,14 +230,14 @@ def validate_dns_label(dns_label, allow_underscore=False): '- must not be the DNS label character') \ % dict(underscore=underscore_err_msg)) -def validate_domain_name(domain_name): +def validate_domain_name(domain_name, allow_underscore=False): 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) + map(lambda label:validate_dns_label(label,allow_underscore), domain_name) if not domain_name[-1].isalpha(): # see RFC 1123 @@ -284,7 +284,7 @@ def validate_zonemgr(zonemgr): validate_domain_name(domain) -def validate_hostname(hostname, check_fqdn=True): +def validate_hostname(hostname, check_fqdn=True, allow_underscore=False): """ See RFC 952, 1123 :param hostname Checked value @@ -299,9 +299,9 @@ def validate_hostname(hostname, check_fqdn=True): if '.' not in hostname: if check_fqdn: raise ValueError(_('not fully qualified')) - validate_dns_label(hostname) + validate_dns_label(hostname,allow_underscore) else: - validate_domain_name(hostname) + validate_domain_name(hostname,allow_underscore) def validate_sshpubkey(ugettext, pubkey): try: diff --git a/tests/test_xmlrpc/test_hbac_plugin.py b/tests/test_xmlrpc/test_hbac_plugin.py index 58265dc07..bd50a585e 100644 --- a/tests/test_xmlrpc/test_hbac_plugin.py +++ b/tests/test_xmlrpc/test_hbac_plugin.py @@ -50,6 +50,8 @@ class test_hbac(XMLRPC_test): test_service = u'sshd' test_host_external = u'notfound.example.com' + test_invalid_sourcehost = u'inv+alid#srchost.nonexist.com' + def test_0_hbacrule_add(self): """ Test adding a new HBAC rule using `xmlrpc.hbacrule_add`. @@ -284,6 +286,17 @@ class test_hbac(XMLRPC_test): assert_attr_equal(entry, 'sourcehost_host', self.test_host) assert_attr_equal(entry, 'sourcehost_hostgroup', self.test_hostgroup) + def test_a_hbacrule_add_invalid_sourcehost(self): + """ + Test adding invalid source host to HBAC rule using `xmlrpc.hbacrule_add_host`. + """ + try: + api.Command['hbacrule_add_sourcehost']( + self.rule_name, host=self.test_invalid_sourcehost, hostgroup=self.test_hostgroup + ) + except errors.ValidationError: + pass + def test_a_hbacrule_add_service(self): """ Test adding service to HBAC rule using `xmlrpc.hbacrule_add_service`. diff --git a/tests/test_xmlrpc/test_netgroup_plugin.py b/tests/test_xmlrpc/test_netgroup_plugin.py index c40b01ad6..33b2c6da7 100644 --- a/tests/test_xmlrpc/test_netgroup_plugin.py +++ b/tests/test_xmlrpc/test_netgroup_plugin.py @@ -57,6 +57,9 @@ user2 = u'pexample' group1 = u'testgroup' invalidnetgroup1=u'+badnetgroup' +invalidnisdomain1=u'domain1,domain2' +invalidnisdomain2=u'+invalidnisdomain' +invalidhost=u'+invalid&host' class test_netgroup(Declarative): """ @@ -105,6 +108,20 @@ class test_netgroup(Declarative): ), + dict( + desc='Test an invalid nisdomain1 name %r' % invalidnisdomain1, + command=('netgroup_add', [netgroup1], dict(description=u'Test',nisdomainname=invalidnisdomain1)), + expected=errors.ValidationError(name='nisdomainname', error='may only include letters, numbers, _, - and .'), + ), + + + dict( + desc='Test an invalid nisdomain2 name %r' % invalidnisdomain2, + command=('netgroup_add', [netgroup1], dict(description=u'Test',nisdomainname=invalidnisdomain2)), + expected=errors.ValidationError(name='nisdomainname', error='may only include letters, numbers, _, - and .'), + ), + + dict( desc='Create %r' % netgroup1, command=('netgroup_add', [netgroup1], @@ -333,6 +350,14 @@ class test_netgroup(Declarative): ), + dict( + desc='Add invalid host %r to netgroup %r' % (invalidhost, netgroup1), + command=('netgroup_add_member', [netgroup1], dict(host=invalidhost)), + expected=errors.ValidationError(name='host', + error='only letters, numbers, _, and - are allowed. - must not be the DNS label character'), + ), + + dict( desc='Add host %r to netgroup %r' % (host1, netgroup1), command=( @@ -756,7 +781,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Test netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -777,7 +802,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Test netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -800,7 +825,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Test netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ], ), @@ -824,7 +849,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Test netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ], ), @@ -848,7 +873,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Test netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, { 'dn': fuzzy_netgroupdn, @@ -878,7 +903,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -913,7 +938,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -947,7 +972,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -980,7 +1005,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1012,7 +1037,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1043,7 +1068,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1074,7 +1099,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1105,7 +1130,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1136,7 +1161,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1167,7 +1192,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), @@ -1198,7 +1223,7 @@ class test_netgroup(Declarative): 'cn': [netgroup1], 'description': [u'Updated netgroup 1'], 'nisdomainname': [u'%s' % api.env.domain], - 'externalhost': [u'unknown'], + 'externalhost': [unknown_host], }, ), ), diff --git a/tests/test_xmlrpc/test_sudorule_plugin.py b/tests/test_xmlrpc/test_sudorule_plugin.py index 7c45256ec..6aabd2b27 100644 --- a/tests/test_xmlrpc/test_sudorule_plugin.py +++ b/tests/test_xmlrpc/test_sudorule_plugin.py @@ -53,6 +53,10 @@ class test_sudorule(XMLRPC_test): test_category = u'all' test_option = u'authenticate' + test_invalid_user = u'+invalid#user' + test_invalid_host = u'+invalid&host.nonexist.com' + test_invalid_group = u'+invalid#group' + def test_0_sudorule_add(self): """ Test adding a new Sudo rule using `xmlrpc.sudorule_add`. @@ -206,6 +210,20 @@ class test_sudorule(XMLRPC_test): entry = ret['result'] assert_attr_equal(entry, 'ipasudorunas_user', self.test_runasuser) + def test_a_sudorule_add_runasuser_invalid(self): + """ + Test adding run as invalid user to Sudo rule using + `xmlrpc.sudorule_add_runasuser`. + """ + try: + api.Command['sudorule_add_runasuser']( + self.rule_name, user=self.test_invalid_user + ) + except errors.ValidationError: + pass + else: + assert False + def test_b_sudorule_remove_runasuser(self): """ Test removing run as user to Sudo rule using @@ -239,6 +257,20 @@ class test_sudorule(XMLRPC_test): assert_attr_equal(entry, 'ipasudorunasgroup_group', self.test_runasgroup) + def test_a_sudorule_add_runasgroup_invalid(self): + """ + Test adding run as invalid user to Sudo rule using + `xmlrpc.sudorule_add_runasuser`. + """ + try: + api.Command['sudorule_add_runasgroup']( + self.rule_name, group=self.test_invalid_group + ) + except errors.ValidationError: + pass + else: + assert False + def test_b_sudorule_remove_runasgroup(self): """ Test removing run as group to Sudo rule using @@ -268,6 +300,20 @@ class test_sudorule(XMLRPC_test): entry = ret['result'] assert_attr_equal(entry, 'externaluser', self.test_external_user) + def test_a_sudorule_add_externaluser_invalid(self): + """ + Test adding an invalid external user to Sudo rule using + `xmlrpc.sudorule_add_user`. + """ + try: + api.Command['sudorule_add_user']( + self.rule_name, user=self.test_invalid_user + ) + except errors.ValidationError: + pass + else: + assert False + def test_b_sudorule_remove_externaluser(self): """ Test removing an external user from Sudo rule using @@ -424,6 +470,20 @@ class test_sudorule(XMLRPC_test): entry = ret['result'] assert_attr_equal(entry, 'externalhost', self.test_external_host) + def test_a_sudorule_add_externalhost_invalid(self): + """ + Test adding an invalid external host to Sudo rule using + `xmlrpc.sudorule_add_host`. + """ + try: + api.Command['sudorule_add_host']( + self.rule_name, host=self.test_invalid_host + ) + except errors.ValidationError: + pass + else: + assert False + def test_b_sudorule_remove_externalhost(self): """ Test removing an external host from Sudo rule using