Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
On 04/10/2012 08:06 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/10/2012 03:46 PM, Rob Crittenden wrote: Petr Viktorin wrote: ... 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 I made a minor change. VERSION shoudl just update the minor version number. I changed this, ACK, pushed to master and ipa-2-2 rob I followed the comment in the VERSION file, which says: # The version of the IPA API. This controls which # # client versions can use the XML-RPC and json APIs# # # # A change to existing API requires a MAJOR version# # update. The addition of new API bumps the MINOR # # version. # I see the actual policy is different. How does this really work? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
Petr Viktorin wrote: On 04/10/2012 08:06 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 04/10/2012 03:46 PM, Rob Crittenden wrote: Petr Viktorin wrote: ... 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 I made a minor change. VERSION shoudl just update the minor version number. I changed this, ACK, pushed to master and ipa-2-2 rob I followed the comment in the VERSION file, which says: # The version of the IPA API. This controls which # # client versions can use the XML-RPC and json APIs # # # # A change to existing API requires a MAJOR version # # update. The addition of new API bumps the MINOR # # version. # I see the actual policy is different. How does this really work? Yes, I guess that isn't too clear. You can ADD new API without causing backwards compatibility issues. You can modify the validation without causing backwards compat issues (other than the data itself may no longer be allowed, but that is unrelated to the API). In these cases just bump the minor version. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
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?
--
Petr³
From ece4a28d2b6dc3c35e7fbc6fc3ef80a063312846 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 +-
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 +
5 files changed, 45 insertions(+), 13 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',
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
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. 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. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
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:
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
Petr Viktorin wrote: 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 I made a minor change. VERSION shoudl just update the minor version number. I changed this, ACK, pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0034 Limit permission and selfservice names
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
