Summary: Replication with DRSUAPI_DRS_CRITICAL_ONLY and DRSUAPI_DRS_GET_ANC results in...
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.5.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
Depends on: 12453
  Show dependency treegraph
Reported: 2016-10-30 21:13 UTC by Stefan Metzmacher
Modified: 2017-02-15 10:49 UTC (History)
3 users (show)

See Also:

Workarround for the problem (1.16 KB, text/plain)
2016-10-30 23:31 UTC, Stefan Metzmacher
no flags Details
Possible patch for master (as a first step) (8.27 KB, patch)
2017-01-12 09:03 UTC, Stefan Metzmacher
no flags Details
Patches for v4-6-test (62.78 KB, patch)
2017-02-09 08:10 UTC, Stefan Metzmacher
abartlet: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Metzmacher 2016-10-30 21:13:22 UTC
If the administrator account (with isCriticalSystemObject=TRUE) is moved
away from CN=Users,DC=example,DC=com into an OU (without isCriticalSystemObject=TRUE) the DRSUAPI_DRS_GET_ANC behaviour is
not correctly implemented.

Our drsuapi server doesn't include the required parent object object,
even with DRSUAPI_DRS_GET_ANC, because DRSUAPI_DRS_CRITICAL_ONLY filters it out.
Comment 1 Andrew Bartlett 2016-10-30 21:53:02 UTC
I can confirm this is expected behaviour, in that I expected we might see this when I changed the replication code not to allow replication without a parent object.  (I didn't want to open this particular can of worms at the time). 

I don't see a test, so I'm presuming I didn't check what windows does.
Comment 2 Stefan Metzmacher 2016-10-30 23:31:30 UTC
Created attachment 12612 [details]
Workarround for the problem
Comment 3 Andrew Bartlett 2016-10-30 23:44:47 UTC
Comment on attachment 12612 [details]
Workarround for the problem

A better workaround will be to have the repl.replicate code return a decent exception and to catch and re-try without the flag.

I'm looking into a proper exception generator for this for another case (attempted RID pool allocation on not-rid-master) so that should help here.
Comment 4 Andrew Bartlett 2016-10-31 00:06:33 UTC
In particular, I would change libnet_vampire_cb_store_chunk() to return WERROR so that we do not loose the WERR_DS_DRA_MISSING_PARENT error in the conversion.

Then we can raise a good exception in py_net_replicate_chunk() using the code I'm about to write.  The join.py code can then catch this specific exception, and re-try without DRSUAPI_DRS_CRITICAL_ONLY
Comment 5 Andrew Bartlett 2016-10-31 04:12:27 UTC
Expect some patches soon that should make this easy to fix, from the 'create the right exceptions' side of things.
Comment 6 Andrew Bartlett 2016-11-06 21:31:13 UTC
You should now be able to catch WERR_DS_DRA_MISSING_PARENT in join.py, given the recent patches (tagged to this bug) on master.

With that in place can you handle that and add a test for this?
Comment 7 Stefan Metzmacher 2016-11-29 09:09:16 UTC
(In reply to Andrew Bartlett from comment #6)

The correct fix needs to be in the server.
A Windows server provides all required objects.

I have a domain with a Samba and a Windows DC.

samba-tool drs clone-dc-database works against Windows,
but fails against Samba. I think our implementation
of DRSUAPI_DRS_GET_ANC is wrong.

I guess having a different sorting function based on

DRSUAPI_DRS_GET_ANC should make sure that we inject all parent
objects to the objects still sorted in the typical usn based sort order.
Comment 8 Stefan Metzmacher 2017-01-12 09:03:59 UTC
Created attachment 12824 [details]
Possible patch for master (as a first step)
Comment 9 Stefan Metzmacher 2017-02-09 08:10:49 UTC
Created attachment 12911 [details]
Patches for v4-6-test
Comment 10 Andrew Bartlett 2017-02-09 23:01:06 UTC
Comment on attachment 12911 [details]
Patches for v4-6-test

Thanks.  This would be great to see in 4.6.
Comment 11 Karolin Seeger 2017-02-13 15:34:42 UTC
Pushed to autobuild-v4-6-test.
Comment 12 Karolin Seeger 2017-02-13 15:35:32 UTC
(In reply to Karolin Seeger from comment #11)
Pushed to v4-6-test.
Closing out bug report.

Comment 13 Stefan Metzmacher 2017-02-13 15:54:11 UTC
Not in v4-6-test yet...
Comment 14 Karolin Seeger 2017-02-13 15:58:01 UTC
(In reply to Stefan Metzmacher from comment #13)
user@host:/data/git/samba/v4-6-test$ git am tmp46.diff.txt 
Applying: s4:dsdb/repl: s/highestCommitedUsn/highestCommittedUSN
error: patch failed: source4/dsdb/repl/drepl_service.h:89
error: source4/dsdb/repl/drepl_service.h: patch does not apply
Patch failed at 0001 s4:dsdb/repl: s/highestCommitedUsn/highestCommittedUSN
Comment 15 Stefan Metzmacher 2017-02-13 16:08:41 UTC
(In reply to Karolin Seeger from comment #14)

Hi Karo,

see you comment #11, you already applied it, I just reopened it
as it's only in autobuild-v4-6-test, but not yet in v4-6-test (that's why I reopened the bug).

If you try to apply it twice it fails for sure...

Comment 16 Karolin Seeger 2017-02-15 10:49:14 UTC
Pushed to v4-6-test.
Closing out bug report.