Bug 734831

+++ This bug was initially created as a clone of Bug #722292 +++

Description of problem:

Certain entries in DS are not being updated properly over Active Directory agreement when WinSync API plugin is enabled (Using the test plugin code that is provided in winsync-plugin.h).

Version-Release number of selected component (if applicable):

1.2.8.3

How reproducible:
100%


Steps to Reproduce:
1. Compile and configure a WinSync API plugin (I used the exact example that is in the winsync-plugin.h file)
2. Create sync agreement between 389 and Active Directory, replicating the DS subtree (ou=People,dc=example,dc=com) and the Windows subtree (dc=example,dc=local)
3. In AD create an Test OU under dc=example,dc=local (ou=Test,dc=example,dc=local)
4. Create two users, one in the dc=example,dc=local subtree (user1), and the other user (user2) in the Test OU (ou=Test,dc=example,dc=local)
5. Run and sync, either incrmental or total and the two entries will be created on the DS side in the ou=People,dc=example,dc=com.
6. In AD, perform updates on both accounts (ie Add an email address or something)
7. Run incremental update.
8. In DS, you should now see that user1 should have the updated attribute, but user2 does not.  

  
Actual results:

User1 should be updated properly, but user2 will not be.  If you have nsslapd-errorlog-level set to 65536, there should be an error similar to this:

[14/Jul/2011:13:21:13 -0700] NSMMReplicationPlugin - failed to rename entry (uid=User2,ou=People,dc=example,dc=com); LDAP error - 32

Expected results:

Both users should be updated properly

Additional info:

The plugin works correctly on version 1.2.5

--- Additional comment from nkinder on 2011-08-25 14:04:15 EDT ---

Here is a more complete log snippet that demonstrates what is logged when we receive the update to the user contained in an OU:

=============================================================================
[25/Aug/2011:10:57:04 -0700] NSMMReplicationPlugin - received entry from dirsync: CN=Test 2,CN=level0,CN=Users,DC=example,DC=com
[25/Aug/2011:10:57:04 -0700] NSMMReplicationPlugin - agmt="cn=meTo10.14.54.184389" (10:389): map_entry_dn_inbound: looking for local entry matching AD entry [CN=Test 2,CN=level0,CN=Users,DC=example,DC=com]
[25/Aug/2011:10:57:04 -0700] NSMMReplicationPlugin - agmt="cn=meTo10.14.54.184389" (10:389): map_entry_dn_inbound: looking for local entry by guid [3b120754626be2438de88d4f43404fd2]
[25/Aug/2011:10:57:04 -0700] test_winsync_api - --> test_winsync_pre_ds_search_cb -- begin
[25/Aug/2011:10:57:04 -0700] test_winsync_api - <-- test_winsync_pre_ds_search_cb -- end
[25/Aug/2011:10:57:04 -0700] NSMMReplicationPlugin - agmt="cn=meTo10.14.54.184389" (10:389): map_entry_dn_inbound: found local entry [uid=test2,ou=People,dc=example,dc=com]
[25/Aug/2011:10:57:04 -0700] - Calling windows entry search request plugin
[25/Aug/2011:10:57:04 -0700] test_winsync_api - --> test_winsync_pre_ad_search_cb -- begin
[25/Aug/2011:10:57:04 -0700] test_winsync_api - <-- test_winsync_pre_ad_search_cb -- end
[25/Aug/2011:10:57:05 -0700] - windows_search_entry: received 2 messages, 1 entries, 0 references
[25/Aug/2011:10:57:05 -0700] - _csngen_adjust_local_time: gen state before 4e568cf00001:1314295024:0:0
[25/Aug/2011:10:57:05 -0700] - _csngen_adjust_local_time: gen state after 4e568cf10000:1314295025:0:0
[25/Aug/2011:10:57:05 -0700] NSMMReplicationPlugin - ruv_add_csn_inprogress: successfully inserted csn 4e568cf1000000010000 into pending list
[25/Aug/2011:10:57:05 -0700] NSMMReplicationPlugin -  csn=4e568cf1000000010000 process postop: canceling operation csn
[25/Aug/2011:10:57:05 -0700] NSMMReplicationPlugin - failed to rename entry (uid=test2,ou=People,dc=example,dc=com); LDAP error - 32
[25/Aug/2011:10:57:05 -0700] NSMMReplicationPlugin - agmt="cn=meTo10.14.54.184389" (10:389): windows_process_dirsync_entry: failed to update inbound entry for CN=Test 2,CN=level0,CN=Users,DC=example,DC=com.
=============================================================================

It appears that the problem is not in the winsync API plug-in itself based off of the logs.  Something in the replication plug-in must be getting tripped up due to the flattening of the DN that is done by the winsync API plug-in.

--- Additional comment from rmeggins on 2011-08-25 14:31:10 EDT ---

I have a IPA winsync plugin setup already - want me to take a look at this?

--- Additional comment from nkinder on 2011-08-25 16:19:30 EDT ---

(In reply to comment #2)
> I have a IPA winsync plugin setup already - want me to take a look at this?

No, I already have this reproduced using the sample winsync API plug-in.

--- Additional comment from nkinder on 2011-08-25 16:34:17 EDT ---

The problem was introduced when we added support for moving an entry in AD, which triggers a rename (MODRDN) in DS.

Since the winsync API plug-in in this test flattens the DN, the windows_update_local_entry() function thinks that the entry was renamed on the AD side.  We then attempt to do a MODRN operation on the DS side against the mapped DN, but that entry doesn't exist since the winsync API plug-in rewrote it when the entry was initially added.

We need some way of detecting a rename operation without interfering with winsync API plug-ins that rewrite the DN of the entry.  We might be able to do this by making windows_update_local_entry() call the registered winsync_get_new_dn_cb callback to determine if the entry has really been renamed or not.

--- Additional comment from nkinder on 2011-08-30 16:29:28 EDT ---

Created attachment 520685 [details]
Patch

--- Additional comment from rmeggins on 2011-08-30 17:03:09 EDT ---

Comment on attachment 520685 [details]
Patch

in https://bugzilla.redhat.com/attachment.cgi?id=520685&action=diff#a/ldap/servers/plugins/replication/windows_protocol_util.c_sec5

Do you need to use slapi_create_dn_string() instead?  That is, if it is possible that local_pndn, remote_subtree, or local_subtree could contain a character that needs to be DN escaped, you would need to call slapi_create_dn_string() instead - for example, see process_replay_add()

--- Additional comment from nkinder on 2011-08-30 17:10:53 EDT ---

(In reply to comment #6)
> Comment on attachment 520685 [details]
> Patch
> 
> in
> https://bugzilla.redhat.com/attachment.cgi?id=520685&action=diff#a/ldap/servers/plugins/replication/windows_protocol_util.c_sec5
> 
> Do you need to use slapi_create_dn_string() instead?  That is, if it is
> possible that local_pndn, remote_subtree, or local_subtree could contain a
> character that needs to be DN escaped, you would need to call
> slapi_create_dn_string() instead - for example, see process_replay_add()

Yes, we should use slapi_create_dn_string() instead of PR_smprintf().  I will make the change and attach a revised patch.

--- Additional comment from nkinder on 2011-08-30 17:20:45 EDT ---

Created attachment 520694 [details]
Revised Patch

--- Additional comment from rmeggins on 2011-08-30 18:22:49 EDT ---

Comment on attachment 520694 [details]
Revised Patch

assuming local_pndn, remote_subtree, and local_subtree have no DN special characters, this is fine

--- Additional comment from nkinder on 2011-08-31 10:56:44 EDT ---

Pushed to master.  Thanks to Rich and Noriko for their reviews!

Counting objects: 13, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 2.56 KiB, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   4fdd413..402cd42  master -> master