https://fedorahosted.org/freeipa/ticket/2585: ipa permission-add throws internal server error when name contains '<', '>' or other special characters.

The problem is, of course, proper escaping; not only in DNs but also in ACIs. Right now we don't really do either.


This patch is just a simple workaround: disallow anything except known-good characters. It's just names, so no functionality is lost.

All tickets for April are now taken, so unless a new one comes my way, I'll take a dive into the code and fix it properly. This could take some time and would mean somewhat larger changes.

--
PetrĀ³
From 20a04656131dfeb36ee8b8f8313e6d712411c1e0 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 6 Apr 2012 04:56:46 -0400
Subject: [PATCH] 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
---
 ipalib/plugins/permission.py                 |   10 +++++++++-
 ipalib/plugins/selfservice.py                |   10 +++++++++-
 tests/test_xmlrpc/test_permission_plugin.py  |   11 +++++++++++
 tests/test_xmlrpc/test_selfservice_plugin.py |   13 +++++++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index ce2536d9921ede73d2c26468f5d99609552e1881..01f081c3829c9cd314fcd41c7578bf9f74ed82c3 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -18,6 +18,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import copy
+import re
+
 from ipalib.plugins.baseldap import *
 from ipalib import api, _, ngettext
 from ipalib import Flag, Str, StrEnum
@@ -92,6 +94,12 @@
 
 dn_ipaconfig = str(DN('cn=ipaconfig,cn=etc,%s' % api.env.basedn))
 
+
+def validate_name(ugettext, name):
+    if re.search('[^-a-zA-Z0-9 _]', name):
+        return _('May only contain alphanumerics or -, _, space')
+
+
 def check_attrs(attrs, type):
     # Trying to delete attributes - no need for validation
     if attrs is None:
@@ -150,7 +158,7 @@ class permission(LDAPObject):
     label_singular = _('Permission')
 
     takes_params = (
-        Str('cn',
+        Str('cn', validate_name,
             cli_name='name',
             label=_('Permission name'),
             primary_key=True,
diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py
index 6f843d469be2e5c29fb7587fcef98069b839eec5..45306707893babbddf1a838e9bf5ff29454af6bc 100644
--- a/ipalib/plugins/selfservice.py
+++ b/ipalib/plugins/selfservice.py
@@ -18,6 +18,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import copy
+import re
+
 from ipalib import api, _, ngettext
 from ipalib import Flag, Str
 from ipalib.request import context
@@ -60,6 +62,12 @@
     ),
 )
 
+
+def validate_name(ugettext, name):
+    if re.search('[^-a-zA-Z0-9 _]', name):
+        return _('May only contain alphanumerics or -, _, space')
+
+
 class selfservice(Object):
     """
     Selfservice object.
@@ -72,7 +80,7 @@ class selfservice(Object):
     label_singular = _('Self Service Permission')
 
     takes_params = (
-        Str('aciname',
+        Str('aciname', validate_name,
             cli_name='name',
             label=_('Self-service name'),
             doc=_('Self-service name'),
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index 51546732ca3ca4b3eea777024d0e9a21b75b83cf..b9c3e9bbdb8dde652cc935f5a3c775d32b2372db 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -45,6 +45,8 @@
 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 alphanumerics or -, _, space'),
+        ),
 
     ]
diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py
index d457627c5c5f8318af8c89c606b6ca037e7559c6..8d043494411382a399daf5dcb2a4ca1a9018c8f5 100644
--- a/tests/test_xmlrpc/test_selfservice_plugin.py
+++ b/tests/test_xmlrpc/test_selfservice_plugin.py
@@ -26,6 +26,7 @@
 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 alphanumerics or -, _, space'),
+        ),
+
     ]
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to