The main issue is that applying a new ACL to directory objects only fully propagates down the tree on the server on which the ACL modification was performed. On other DCs replication only seems to propagate the new ACLs down one or two levers at most. This affects delegation in ADUC as well. The only solution is to run a samba-tool drs replicate --full-sync on the other DCs. I post below the posts I've sent to the ML with no response so far: >> I've been testing Samba 4.5.1 extensively as an AD DC. We have 3 DC set up, and replication of users, groups, OUs, DNS etc has been working fine. >> >> However we wanted to add some custom attributes and a class to the schema (an assortment of string and numericalString) for our own purposes. This also worked fine (and the Schema replication worked), but some oddness happened when we wanted to restrict access to one of these attributes. >> >> The class was added to the "user" AD class as an auxiliary, and then the following procedure was used: >> >> https://support.microsoft.com/en-us/kb/320528 >> >> To add anonymous access to the Public Information, Phone and Mail, and our additional attributes (excepting the restricted one). Then a deny ACL for "Everyone" and "Anonymous Logon" was added recursively for the restricted attribute at the root of the domain tree on descendent User objects. >> >> This seemed to work on the server that ADSI edit was connected to when tested with ldapsearch, but *not* on the other two DCs. They behaved as if no ACLs had been changed. When I connected ADSI edit to the other DCs I could see that the ACLs seemed to be present at the domain root, but were not propagating down the tree even though the inherit box was checked on subordinates. >> >> I had to do: >> >> samba-tool drs replicate s4-dc-01 s4-dc-02 DC=my,DC=ifa,DC=net --full-sync >> >> (where s4-dc-02 was the "working" DC) and all seemed to be fixed, both using ldapsearch and ADSI Edit. >> >> Is it a known issue that ACLs aren't completely replicated? Or is it that "custom" attributes cause problems with ACLs (even though they are applied as auxiliary to the "user" AD class. >> >> Otherwise everything is working 100% correctly (showrepl gives no errors). >> >> >> Cheers, >> >> Alex >> > Another update on this problem: This seems only to occur if ACLs on directory objects or delegation is changed /after/ DCs have been joined to the domain. A fresh DC will get all of the ACLs correct as it does seem to do a full sync of objects from an existing DC when joined. However if we change, say, delegation in ADUC, we will seem to get some part of the corresponding ACLs not replicated on other existing DCs (sometimes it's all the others than the one the change hit, sometimes just one). For instance, we delegated control of user objects below a certain ou (to all descendant user objects) and this is visible from ADUC in the descendant objects on two DCs, but on a third, the ACL only shows on that ou itself - and despite it indeed saying it applies to all descendant user objects in ADUC, when we look at any entry in the tree below, it is not applied in the list of ACLs.
This is still affecting us. Every time we need to enable additional access to any attribute we need to perform this workaround. Does anyone have any input or similar experience?
I think the fundemental issue is that the descriptor module is above repl_meta_data in the list in samba_dsdb. This means that the recursive SD propagation does not happen for changes that come over replication. Because DSDB_CONTROL_SEC_DESC_PROPAGATION_OID is set by the descriptor module, the repl_meta_data module does not update the metadata for the child objects, so the changes made to nTSecurityDescriptor are not seen by other DCs until a full sync.
(In reply to Andrew Bartlett from comment #2) dsdb_module_schedule_sd_propagation() uses DSDB_FLAG_TOP_MODULE, so it's likely to be something else.
(In reply to Stefan Metzmacher from comment #3) OK, that makes things more difficult. Thanks for the pointer.
Steps to reproduce: SELFTEST_TESTENV=vampire_dc make testenv bin/samba-tool ou create ou=l1 -H ldap://$SERVER -U$USERNAME%$PASSWORD bin/samba-tool ou create ou=l2,ou=l1 -H ldap://$SERVER -U$USERNAME%$PASSWORD bin/samba-tool dsacl set --sddl "(A;CIOI;GA;;;SY)" --objectdn=ou=l1,DC=samba,DC=example,DC=com -H ldap://$SERVER -U$USERNAME%$PASSWORD bin/samba-tool dsacl get --objectdn=ou=l2,ou=l1,DC=samba,DC=example,DC=com -H ldap://$DC_SERVER -U$USERNAME%$PASSWORD bin/samba-tool dsacl get --objectdn=ou=l2,ou=l1,DC=samba,DC=example,DC=com -H ldap://$SERVER -U$USERNAME%$PASSWORD These should give the same ACL, but don't. A full sync fixes it: bin/samba-tool drs replicate --full-sync $SERVER $DC_SERVER DC=samba,DC=example,DC=com -U$USERNAME%$PASSWORD bin/samba-tool drs replicate --full-sync $DC_SERVER $SERVER DC=samba,DC=example,DC=com -U$USERNAME%$PASSWORD
To get this under the debugger, after creating the ou object in the testenv, run: bin/samba-tool drs options --dsa-option=+DISABLE_INBOUND_REPL -s st/vampire_dc/etc/smb.conf -U$USERNAME%$PASSWORD bin/samba-tool drs options --dsa-option=+DISABLE_OUTBOUND_REPL -s st/vampire_dc/etc/smb.conf -U$USERNAME%$PASSWORD bin/samba-tool drs options --dsa-option=+DISABLE_OUTBOUND_REPL -s st/ad_dc_ntvfs/etc/smb.conf -U$USERNAME%$PASSWORD bin/samba-tool drs options --dsa-option=+DISABLE_INBOUND_REPL -s st/ad_dc_ntvfs/etc/smb.conf -U$USERNAME%$PASSWORD (that stops background replication) Then set the SD: bin/samba-tool dsacl set --sddl "(A;CIOI;GA;;;SY)" --objectdn=ou=l1,DC=samba,DC=example,DC=com -H ldap://$DC_SERVER -U$USERNAME%$PASSWORD and the debug: gdb --args python3 bin/samba-tool drs replicate $SERVER $DC_SERVER DC=samba,DC=example,DC=com -U$USERNAME%$PASSWORD --sync-forced --local -s st/vampire_dc/etc/smb.conf
Bug 8621 interestingly added the inheritance here, but specifically only handled the originating update case.
While this issue is well known and well described, it has now finally been triaged as a security issue. Therefore marking this as embargoed while we get a CVE and prepare a security release.
Created attachment 15633 [details] initial patch for master (needs tests) This patch addresses the primary issue, that being that replicated changes almost never cause inherited ACEs in an ACL to apply over replication.
With a bit of work due to missing commands in older versions, I've reproduced the issue back to before Samba 4.3.
CVSS score: CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:U/C:L/I:L/A:N
I'm making good progress on this but don't have a working fix right now, the tests need to defend against a race, if you run the reproduction advise in the bug too fast then they will pass. If you slow down then it matches what I have in my branch, abartlet-bug-12497-inherit, on the private gitlab.
Created attachment 15670 [details] a patch that passes the tests Still some cleanup needed but this patch seems to do what we need.
Created attachment 15678 [details] first finished patch for master
Created attachment 15679 [details] first patch for 4.11
Created attachment 15680 [details] first patch for 4.10
Created attachment 15681 [details] first patch for 4.9
Created attachment 15685 [details] patch for master (v2)
Created attachment 15686 [details] patch for Samba 4.11 (cherry-picked from master patch) v2
Created attachment 15687 [details] patch for Samba 4.10 (cherry-picked/backported from master patch) v2
Created attachment 15688 [details] patch for Samba 4.9 (cherry-picked/backported from master patch) v2 I've updated the patch with a change to no longer be named-based (and so now much less sensitive to renames), and to test that we have inheritance, not just that it matches.
Created attachment 15689 [details] first draft security advisory
Thanks gary, assigning to Karolin for the next security release (which is why I've not included specific release versions in the advisory).
Planned release date: Tuesday, January 14 2020
Created attachment 15706 [details] updated advisory (v2) with manual steps included On reflection, a dbcheck rule would be ideal, but we don't have a dbcheck rule for ACL inheritance re-calculation at this stage.
Created attachment 15708 [details] updated advisory (v3) Clarifying that the replication just needs to be TO each DC, not pair-wise.
(In reply to Andrew Bartlett from comment #25) Also note that Windows uses the dSCorePropagationData attribute to maintain state for their background task... (I don't know more details, I just recently noticed that attribute again and wanted to find out what it's for on Windows).
Waiting for review on the advisory.
Will be delayed. The new planned release date will be published as soon as possible.
Planned release date: Tuesday, January 21 2020 Opening bug report for vendors.
Has anyone managed to backport this to 4.3 yet?
Created attachment 15738 [details] Updated advisory with version numbers
Samba 4.11.5, 4.10.12 and 4.9.18 have been shipped to address these defects.
Merged into v4-{11,10,9}-test.
Pushed to autobuild-master.
Pushed to master. Closing out bug report. Thanks!
Removing embargo, after checking that there are no comments or attachments that need additional privacy.