On 04/10/2012 03:46 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 04/09/2012 03:55 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
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.
Is there a reason you didn't use pattern/pattern_errmsg instead?
You'd need to change the regex as patterns use re.match rather than
re.search.
rob
Right, that makes more sense.
It changes API.txt though. Do I need to bump VERSION in this case?
Also, is there a reason pattern_errmsg is included in API.txt?
Yes, please bump VERSION.
Attaching updated patch.
pattern_errmsg should probably be removed from API.txt. We've been
paring back the amount of data to validate slowly as we've run into
these questionable items. Please open a ticket for this.
Done: https://fedorahosted.org/freeipa/ticket/2619
--
Petr³
From 0454790c2c037f70ad268e6bdd4a25ca8927ce6a 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
---
API.txt | 26 +++++++++++++-------------
VERSION | 4 ++--
ipalib/plugins/permission.py | 4 ++++
ipalib/plugins/selfservice.py | 4 ++++
tests/test_xmlrpc/test_permission_plugin.py | 11 +++++++++++
tests/test_xmlrpc/test_selfservice_plugin.py | 13 +++++++++++++
6 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/API.txt b/API.txt
index c1b2ba05c40576abf6328f834eadb11e2d030b49..a49376817daf06b504e8148783e50091a62b6363 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,25 +2057,25 @@ 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')
option: Str('privilege*', alwaysask=True, cli_name='privileges', csv=True)
output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
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)
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,23 +2110,23 @@ 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')
option: Str('privilege*', alwaysask=True, cli_name='privileges', csv=True)
output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
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 77572ea68f483b6fe7c4d4962dc27bf653b05d1d..002ae26495c8bd340e5fac5de361747ca7ae40f3 100644
--- a/VERSION
+++ b/VERSION
@@ -78,5 +78,5 @@ IPA_DATA_VERSION=20100614120000
# The format is a whole number #
# #
########################################################
-IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=33
+IPA_API_VERSION_MAJOR=3
+IPA_API_VERSION_MINOR=0
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index ce2536d9921ede73d2c26468f5d99609552e1881..9b669d9f57e81e885bd080703ba6c405395f6608 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 @@
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 6f843d469be2e5c29fb7587fcef98069b839eec5..a60475b7cc7b109fdebdfecdbf71408d6041a0ba 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 @@
),
)
+
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 51546732ca3ca4b3eea777024d0e9a21b75b83cf..ab28588609caf080911a29c6e76e7c81e8f296ef 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 letters, numbers, -, _, and space'),
+ ),
]
diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py
index d457627c5c5f8318af8c89c606b6ca037e7559c6..3546701d527cf1f4725e25463aa5a444029c0762 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 letters, numbers, -, _, and space'),
+ ),
+
]
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
