[Freeipa-devel] [PATCH] 0137 ipasam: remove child domains before removing trust

[Alexander Bokovoy]

On Mon, 20 Jan 2014, Martin Kosek wrote:
>On 01/20/2014 03:49 PM, Alexander Bokovoy wrote:
>> Hi!
>>
>> Make sure we delete child domains before removing the trust itself as
>> LDAP protocol does not allow removing non-leaf objects.
>>
>> This has non-obvious effect -- old code did remove cross-realm
>> principals and then removed trust object. However, for trusts with child
>> domains the trust domain object was not removed as LDAP server prevents
>> removing non-leaf objects. It resulted in the object still existing but
>> cross-realm principals missing. The trust is thus non-functioning. This
>> situation can be triggered with a second 'ipa trust-add' call.
>>
>> Fix the code by removing child domains first and then remove the forest
>> root trusted domain object.
>>
>> https://fedorahosted.org/freeipa/ticket/4126
>
>Thanks for the patch! I did not test, I am just thinking about this search:
>
>+
>+	rc = smbldap_search(ldap_state->smbldap_state, dn, scope, filter, NULL, 0,
>&result);
>+	TALLOC_FREE(filter);
>+
>
> - shouldn't you search with SCOPE_ONELEVEL given we do not dive deeper anyway?
No. We need to remove dn but to remove it we need to remove everything
under it. Thus, we don't care what is there, since whole dn
(cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX) will be deleted anyway.

>- shouldn't we search with filter "(objectclass=ipaNTTrustedDomain)" just to
>make sure we do not delete anything we do not want to be deleted? For example
>if the function gets a wrong DN, we may want to make sure we don't delete the
>whole DIT
We should delete everything under 'dn' which is cn=TRUSTNAME,cn=ad,cn=trusts,$SUFFIX

>Additionally, I think we should add few DEBUG messages, so that in debug log we
>see we are doing this deletion.
We'll see them at level 5 anyway because of smbldap_delete():
[2014/01/20 17:14:02.965144,  5, pid=5111, effective(874400000, 874400000), real(874400000, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
   smbldap_delete: dn => [cn=ad12y.ad12x.weald.vda.li,cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]
[2014/01/20 17:14:03.034982,  5, pid=5111, effective(874400000, 874400000), real(874400000, 0)] ../source3/lib/smbldap.c:1535(smbldap_delete)
   smbldap_delete: dn => [cn=ad12x.weald.vda.li,cn=ad,cn=trusts,dc=ipa,dc=weald,dc=vda,dc=li]

I don't think we need to add more.
-- 
/ Alexander Bokovoy