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

[Martin Kosek]

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.

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