On 08/27/2012 11:32 PM, Rich Megginson wrote: > On 08/27/2012 02:27 PM, Martin Kosek wrote: >> On Mon, 2012-08-27 at 10:29 -0600, Rich Megginson wrote: >>> On 08/27/2012 10:24 AM, Martin Kosek wrote: >>>> On 08/17/2012 04:00 PM, Rich Megginson wrote: >>>>> On 08/17/2012 07:44 AM, Martin Kosek wrote: >>>>>> Hi guys, >>>>>> >>>>>> I am now investigating ticket #2866: >>>>>> https://fedorahosted.org/freeipa/ticket/2866 >>>>>> >>>>>> And I am thinking about possible solutions for this problem. In a >>>>>> nutshell, we do not properly check referential integrity in some IPA >>>>>> objects where we keep one-way DN references to other objects, e.g. in >>>>>> - managedBy attribute for a host object >>>>>> - memberhost attribute for HBAC rule object >>>>>> - memberuser attribute for user object >>>>>> - memberallowcmd or memberdenycmd for SUDO command object (reported in >>>>>> #2866) >>>>>> ... >>>>>> >>>>>> Currently, I see 2 approaches to solve this: >>>>>> 1) Add relevant checks to our ipalib plugins where problematic >>>>>> operations with these operations are being executed (like we do for >>>>>> selinuxusermap's seealso attribute in HBAC plugin) >>>>>> This of course would not prevent direct LDAP deletes. >>>>>> >>>>>> 2) Implement a preop DS plugin that would hook to MODRDN and DELETE >>>>>> callbacks and check that this object's DN is not referenced in other >>>>>> objects. And if it does, it would reject such modification. Second >>>>>> option would be to delete the attribute value with now invalid >>>>>> reference. This would be probably more suitable for example for >>>>>> references to user objects. >>>>>> >>>>>> Any comments to these possible approaches are welcome. >>>>>> >>>>>> Rich, do you think that as an alternative to these 2 approaches, >>>>>> memberOf plugin could be eventually modified to do this task? >>>>> This is very similar to the referential integrity plugin already in 389, >>>>> except >>>>> instead of cleaning up references to moved and deleted entries, you want >>>>> it to >>>>> prevent moving or deleting an entry if that entry is referenced by the >>>>> managedby/memberhost/memberuser/memberallowcmd/memberdenycmd of some other >>>>> entry. >>>> I think that using or enhancing current DS referential integrity plugin >>>> will be >>>> the best and the most consistent way to go. >>>> >>>> We already use that plugin for some user attributes like "manager" or >>>> "secretary". seeAlso is already covered by default, so for example seeAlso >>>> attribute in SELinux usermap object referencing an HBAC rule will get >>>> removed >>>> when relevant HBAC rule is removed (I just checked that). >>>> >>>>> Note that the managed entry plugin (mep) already handles this for the >>>>> managedby >>>>> attribute. >>>> I assume you are referencing "mepmanagedby" and "mepmanagedentry" >>>> attributes >>>> which then produce errors like this one: >>>> >>>> # ipa netgroup-del foo >>>> ipa: ERROR: Server is unwilling to perform: Deleting a managed entry is not >>>> allowed. It needs to be manually unlinked first. >>>> >>>> managedBy attribute used by hosts objects I had in mind seems to not be >>>> covered. >>>> >>>> But you are right, this is pretty much what I wanted. Though in case of >>>> MEP, >>>> there is a link in both referenced objects, but in our case, we have just >>>> the >>>> one-way link. >>>> >>>>> Are you already using the memberof plugin for >>>>> memberhost/memberuser/memberallowcmd/memberdenycmd? >>>>> >>>>> This doesn't seem like a job for memberof, this seems like more of a new >>>>> check >>>>> for the referential integrity plugin. >>>>> >>>> I am now considering if current move/delete clean up already present in >>>> Referential Integrity plugin would not be sufficient for us. >>>> >>>> Rich, please correct me if I am wrong, but in that case, we would just >>>> need to >>>> add relevant attribute names >>>> (memberhost/memberuser/memberallowcmd/memberdenycmd...) to Referential >>>> Integrity plugin configuration as nsslapd-pluginarg7, nsslapd-pluginarg8, >>>> ... >>>> I wonder if there would be some performance issues if we add attributes to >>>> the >>>> list this way. >>> No, not if they are indexed for presence and equality. >>> >>> But referential integrity will not prevent deletion or moving entries - >>> it will delete/move references. >> I understand that. After some reconsideration, I think that cleaning up >> dangling references as postop should be OK for most of the referential >> attributes we use. But I would like a second opinion on that. >> >> So do I understand it correctly, that in case we want to go this way in >> IPA, the recommended approach DS-wise would be to: >> - add presence and equality indexes to IPA for the attributes we want to >> have checked for referential integrity >> - configure DS Referential Integrity plugin to check these attributes >> >> Or would it be better to wait on relevant DS changes you mentioned that >> Noriko is working on? > > Also look at the Linked Attribute plugin - it may be able to do what you want > right now - http://port389.org/wiki/Linked_Attributes_Design
Yes, but this plugin is only relevant for bi-directional links and not for uni-directional links we have in IPA (memberhost/memberuser/memberallowcmd/memberdenycmd...), isn't it? I.e. there is nothing to be filled for "managedType" attribute of the config. Maybe if the link plugin is enhanced for the uni-directional links too it would help us. It would help if I would be able to specify a links like that: dn: cn=memberhost Link, cn=Linked Attribute Plugin, cn=config objectclass: extensibleObject linkType: memberhost linkScope: cn=sudorules,cn=sudo,$SUFFIX Without having to specify a managed link. The plugin would then just hda pretty much do what we discussed before - keep referential integrity of the uni-directional plugin. Martin > >> >> Thanks, >> Martin >> >>>> Rob, do you think that cleaning up the broken references during a DS postop >>>> instead of raising an preop error is OK for IPA references? I went through >>>> the >>>> referential attributes we use ("git grep LDAPAddMember") and I think it >>>> should >>>> be sufficient. We could cover some special cases with a query in our >>>> framework >>>> like you did in hbacrule-del. >>>> >>>> Martin >> > _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel