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

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

Reply via email to