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

@@ -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')

VERSION

@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=31
+IPA_API_VERSION_MINOR=32

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.

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)

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)
 

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)

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:

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`.

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],
                 },
             ),
         ),

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