[Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
Alexander Bokovoy
abokovoy at redhat.com
Thu Sep 20 12:31:24 UTC 2012
On Thu, 20 Sep 2012, Martin Kosek wrote:
>On 09/20/2012 01:58 PM, Alexander Bokovoy wrote:
>> On Thu, 20 Sep 2012, Petr Viktorin wrote:
>>> On 09/20/2012 12:12 PM, Martin Kosek wrote:
>>>> On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, 20 Sep 2012, Martin Kosek wrote:
>>>>>> On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch adds validation of SID for trusted domain when adding or
>>>>>>> modifying ID range for the domain. We only allow creating ranges for
>>>>>>> trusted domains when the trust is already established -- the default
>>>>>>> range is created automatically right after the trust is added.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/3087
>>>>>>>
>>>>>>
>>>>>> Basic functionality looks OK, but I saw few issues with exception formatting:
>>>>>>
>>>>>> diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
>>>>>> index
>>>>>> efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
>>>>>>
>>>>>>
>>>>>> 100644
>>>>>> --- a/ipalib/plugins/idrange.py
>>>>>> +++ b/ipalib/plugins/idrange.py
>>>>>> @@ -26,6 +26,12 @@ from ipapython import ipautil
>>>>>> from ipalib import util
>>>>>> from ipapython.dn import DN
>>>>>>
>>>>>> +if api.env.in_server and api.env.context in ['lite', 'server']:
>>>>>> + try:
>>>>>> + import ipaserver.dcerpc
>>>>>> + _dcerpc_bindings_installed = True
>>>>>> + except Exception, e:
>>>>>> + _dcerpc_bindings_installed = False
>>>>>>
>>>>>>
>>>>>> Variable "e" is not used, so it can be removed.
>>>>> Then Exception, e should be omitted completely :)
>>>>
>>>> As per PEP8, "except Exception:" is preffered over bare "except:" as otherwise
>>>> it would also catch SystemExit or KeyboardInterrupt.
>>>
>>> You should use the most specific exception you want to handle. In this case
>>> it's probably ImportError.
>> New patch is attached.
>>
>
>The patch looks OK, I would just also like to have the rest of the name=_('ID
>Range setup') messages fixed so that we don't print confusing errors:
>
>$ git grep "ID Range setup" ipalib/
>ipalib/plugins/idrange.py: raise
>errors.ValidationError(name=_('ID Range setup'),
>ipalib/plugins/idrange.py: raise
>errors.ValidationError(name=_('ID Range setup'),
>ipalib/plugins/idrange.py: raise
>errors.ValidationError(name=_('ID Range setup'),
Yep. Done.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 86fef992edd0f6e7b5c818c774352067e90e02a3 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Wed, 19 Sep 2012 19:09:22 +0300
Subject: [PATCH 1/4] validate SID for trusted domain when adding/modifying ID
range
https://fedorahosted.org/freeipa/ticket/3087
---
ipalib/plugins/idrange.py | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index ee50613bbaeb70aecf830ad480773a253f88a136..8f2d4efdc0463e7d81cd72fba7769e38dc0c638b 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+ try:
+ import ipaserver.dcerpc
+ _dcerpc_bindings_installed = True
+ except ImportError:
+ _dcerpc_bindings_installed = False
__doc__ = _("""
ID ranges
@@ -249,6 +255,18 @@ class idrange(LDAPObject):
error=_('range modification leaving objects with ID out '
'of the defined range is not allowed'))
+ def validate_trusted_domain_sid(self, sid):
+ if not _dcerpc_bindings_installed:
+ raise errors.NotFound(reason=_('Cannot perform SID validation without Samba 4 support installed. '
+ 'Make sure you have installed server-trust-ad sub-package of IPA on the server'))
+ domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+ if not domain_validator.is_configured():
+ raise errors.NotFound(reason=_('Cross-realm trusts are not configured. '
+ 'Make sure you have run ipa-adtrust-install on the IPA server first'))
+ if not domain_validator.is_trusted_sid_valid(sid):
+ raise errors.ValidationError(name='domain SID',
+ error=_('SID is not recognized as a valid SID for a trusted domain'))
+
class idrange_add(LDAPCreate):
__doc__ = _("""
Add new ID range.
@@ -278,19 +296,22 @@ class idrange_add(LDAPCreate):
if 'ipanttrusteddomainsid' in options:
if 'ipasecondarybaserid' in options:
- raise errors.ValidationError(name=_('ID Range setup'),
+ raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and secondary_rid_base cannot ' \
'be used together'))
if 'ipabaserid' not in options:
- raise errors.ValidationError(name=_('ID Range setup'),
+ raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and rid_base must ' \
'be used together'))
+ # Validate SID as the one of trusted domains
+ self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+ # Finally, add trusted AD domain range object class
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
else:
if (('ipasecondarybaserid' in options) != ('ipabaserid' in options)):
- raise errors.ValidationError(name=_('ID Range setup'),
+ raise errors.ValidationError(name='ID Range setup',
error=_('Options secondary_rid_base and rid_base must ' \
'be used together'))
@@ -366,6 +387,10 @@ class idrange_mod(LDAPUpdate):
except errors.NotFound:
self.obj.handle_not_found(*keys)
+ if 'ipanttrusteddomainsid' in options:
+ # Validate SID as the one of trusted domains
+ self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+
old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
old_range_size = int(old_attrs.get('ipaidrangesize', [0])[0])
new_base_id = entry_attrs.get('ipabaseid')
--
1.7.12
More information about the Freeipa-devel
mailing list