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

[Martin Kosek]

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