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

[Rich Megginson]

On 09/03/2012 08:42 AM, Martin Kosek wrote:
> On 08/27/2012 06:29 PM, 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.
>>
> Hello Rich,
> I am back to investigate this ticket. In order to be able to deliver some
> working solution to IPA 3.0, I plan to take advantage of current Referential
> Integrity Plugin to clean up dangling references.
>
> This is the plan I plan to take:
> 1) Add pres,eq indexes for all un-indexed attributes that we want to check:
> sourcehost
> memberservice
> managedby
> memberallowcmd
> memberdenycmd
> ipasudorunas
> ipasudorunasgroup
ok
>
> 2) Add missing "pres" index for attributes we want to check, but only have eq
> index:
> manager
> secretary
> memberuser
> memberhost
>
> I assume this step is also needed in order to keep the server performance.
yes
>
> 3) Add all these attributes do Referential Integrity Plugin attribute list of
> not already
ok
>
> 4) Also add Index task (nsIndexAttribute) for all these new indexes so that
> they are created during IPA server upgrade.
ok
>
> Is this procedure OK DS-wise?
Yes
>
> I also have question regarding the following note in RHDS doc chapter 3.6.
> Maintaining Referential Integrity:
>
> "The Referential Integrity Plug-in should only be enabled on one supplier
> replica in a multi-master replication environment to avoid conflict resolution
> loops..."
>
> Currently, we enable this plugin on all IPA replicas. Is this something we need
> to be concerned about and fix ASAP (before we do all this RefInt effort)?

Mark/Nathan - I know you guys have looked at this issue.

>
> Thank you,
> Martin