Limit permission and selfservice names to alphanumerics, -, _, space

The DN and ACI code doesn't always escape special characters properly.
Rather than trying to fix it, this patch takes the easy way out and
enforces that the names are safe.

https://fedorahosted.org/freeipa/ticket/2585

6 files changed, 46 insertions, 14 deletions

diff --git a/API.txt b/API.txt
index c1b2ba05..a4937681 100644
--- a/API.txt
+++ b/API.txt
@@ -2039,7 +2039,7 @@ output: Output('result', <type 'bool'>, None)
 output: Output('value', <type 'unicode'>, None)
 command: permission_add
 args: 1,12,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, required=True)
 option: Str('permissions', attribute=True, cli_name='permissions', csv=True, multivalue=True, required=True)
 option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=False, required=False)
 option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
@@ -2057,7 +2057,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: permission_add_member
 args: 1,4,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', 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')
@@ -2067,7 +2067,7 @@ output: Output('failed', <type 'dict'>, None)
 output: Output('completed', <type 'int'>, None)
 command: permission_del
 args: 1,1,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=True)
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('result', <type 'dict'>, None)
@@ -2075,7 +2075,7 @@ output: Output('value', <type 'unicode'>, None)
 command: permission_find
 args: 1,14,4
 arg: Str('criteria?', noextrawhitespace=False)
-option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False)
+option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=False)
 option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, query=True, required=False)
 option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False)
 option: StrEnum('type', attribute=True, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
@@ -2095,7 +2095,7 @@ output: Output('count', <type 'int'>, None)
 output: Output('truncated', <type 'bool'>, None)
 command: permission_mod
 args: 1,15,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=True)
 option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, required=False)
 option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=False, required=False)
 option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
@@ -2110,13 +2110,13 @@ option: Flag('rights', autofill=True, default=False)
 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')
-option: Str('rename', cli_name='rename', multivalue=False, primary_key=True, required=False)
+option: Str('rename', cli_name='rename', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, required=False)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('value', <type 'unicode'>, None)
 command: permission_remove_member
 args: 1,4,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', 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')
@@ -2126,7 +2126,7 @@ output: Output('failed', <type 'dict'>, None)
 output: Output('completed', <type 'int'>, None)
 command: permission_show
 args: 1,4,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=True)
 option: Flag('rights', autofill=True, default=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
@@ -2437,7 +2437,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: selfservice_add
 args: 1,5,3
-arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
+arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, required=True)
 option: Str('permissions', attribute=True, cli_name='permissions', csv=True, multivalue=True, required=False)
 option: Str('attrs', attribute=True, cli_name='attrs', csv=True, multivalue=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2448,14 +2448,14 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: selfservice_del
 args: 1,0,3
-arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=True)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('result', <type 'bool'>, None)
 output: Output('value', <type 'unicode'>, None)
 command: selfservice_find
 args: 1,7,4
 arg: Str('criteria?')
-option: Str('aciname', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False)
+option: Str('aciname', attribute=True, autofill=False, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=False)
 option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, query=True, required=False)
 option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
@@ -2468,7 +2468,7 @@ output: Output('count', <type 'int'>, None)
 output: Output('truncated', <type 'bool'>, None)
 command: selfservice_mod
 args: 1,5,3
-arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', primary_key=True, query=True, required=True)
 option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, required=False)
 option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, required=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2479,7 +2479,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: selfservice_show
 args: 1,3,3
-arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', pattern_errmsg='May only contain letters, numbers, -, _, and space', 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')
diff --git a/VERSION b/VERSION
index 77572ea6..a8327a05 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=33
+IPA_API_VERSION_MINOR=34
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index ce2536d9..9b669d9f 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -18,6 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import copy
+
 from ipalib.plugins.baseldap import *
 from ipalib import api, _, ngettext
 from ipalib import Flag, Str, StrEnum
@@ -92,6 +93,7 @@ output_params = (
 
 dn_ipaconfig = str(DN('cn=ipaconfig,cn=etc,%s' % api.env.basedn))
 
+
 def check_attrs(attrs, type):
     # Trying to delete attributes - no need for validation
     if attrs is None:
@@ -154,6 +156,8 @@ class permission(LDAPObject):
             cli_name='name',
             label=_('Permission name'),
             primary_key=True,
+            pattern='^[-_ a-zA-Z0-9]+$',
+            pattern_errmsg="May only contain letters, numbers, -, _, and space",
         ),
         Str('permissions+',
             cli_name='permissions',
diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py
index 6f843d46..a60475b7 100644
--- a/ipalib/plugins/selfservice.py
+++ b/ipalib/plugins/selfservice.py
@@ -18,6 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import copy
+
 from ipalib import api, _, ngettext
 from ipalib import Flag, Str
 from ipalib.request import context
@@ -60,6 +61,7 @@ output_params = (
     ),
 )
 
+
 class selfservice(Object):
     """
     Selfservice object.
@@ -77,6 +79,8 @@ class selfservice(Object):
             label=_('Self-service name'),
             doc=_('Self-service name'),
             primary_key=True,
+            pattern='^[-_ a-zA-Z0-9]+$',
+            pattern_errmsg="May only contain letters, numbers, -, _, and space",
         ),
         Str('permissions*',
             cli_name='permissions',
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index 51546732..ab285886 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -45,6 +45,8 @@ privilege1 = u'testpriv1'
 privilege1_dn = DN(('cn',privilege1),
                    api.env.container_privilege,api.env.basedn)
 
+invalid_permission1 = u'bad;perm'
+
 
 class test_permission(Declarative):
 
@@ -712,5 +714,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        dict(
+            desc='Try to create invalid %r' % invalid_permission1,
+            command=('permission_add', [invalid_permission1], dict(
+                     type=u'user',
+                     permissions=u'write',
+                )),
+            expected=errors.ValidationError(name='name',
+                error='May only contain letters, numbers, -, _, and space'),
+        ),
 
     ]
diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py
index d457627c..3546701d 100644
--- a/tests/test_xmlrpc/test_selfservice_plugin.py
+++ b/tests/test_xmlrpc/test_selfservice_plugin.py
@@ -26,6 +26,7 @@ from tests.test_xmlrpc import objectclasses
 from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 
 selfservice1 = u'testself'
+invalid_selfservice1 = u'bad+name'
 
 class test_selfservice(Declarative):
 
@@ -270,4 +271,16 @@ class test_selfservice(Declarative):
             )
         ),
 
+        dict(
+            desc='Create invalid %r' % invalid_selfservice1,
+            command=(
+                'selfservice_add', [invalid_selfservice1], dict(
+                    attrs=[u'street', u'c', u'l', u'st', u'postalcode'],
+                    permissions=u'write',
+                )
+            ),
+            expected=errors.ValidationError(name='name',
+                error='May only contain letters, numbers, -, _, and space'),
+        ),
+
     ]