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

[Rich Megginson]

On 08/29/2012 09:32 AM, Martin Kosek wrote:
> 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.

Ok.  Please file an RFE with 389 and indicate the time frame and IPA 
release this is targeted for.

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