[Freeipa-devel] Ticket #2866 - referential integrity in IPA

[Rich Megginson]

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.
>
> 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