On Mon, Jan 20, 2014 at 05:18:30PM +0200, Alexander Bokovoy wrote:
> 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

The user samba uses to bind to LDAP should not have the privileges to
remove the whole tree. So in the worst case it will just remove all
trusts.

I agree with Alexander that we should remove everything below the DN of
the forest root. The original idea behind putting the other domains in
the forest below was that be removing the sub-tree all information about
the trust was gone (except for the idranges, but that's a different
story).

bye,
Sumit

> 
> >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
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to