Bug 13095 - Broken linked attribute handling
Broken linked attribute handling
Status: REOPENED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB
4.7.0
All All
: P5 normal
: ---
Assigned To: Stefan Metzmacher
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-23 16:23 UTC by Björn Baumbach
Modified: 2017-11-09 09:41 UTC (History)
7 users (show)

See Also:


Attachments
user 3 times in group (3.57 KB, text/plain)
2017-10-23 16:23 UTC, Björn Baumbach
no flags Details
user 1 time removed from group (3.43 KB, text/plain)
2017-10-23 16:25 UTC, Björn Baumbach
no flags Details
dbcheck output (863 bytes, text/plain)
2017-10-23 16:26 UTC, Björn Baumbach
no flags Details
patch to test and fix the MOD_REPLACE case (4.81 KB, patch)
2017-10-25 04:07 UTC, Douglas Bagnall
no flags Details
patch for 4.7.1 (5.04 KB, patch)
2017-10-26 22:22 UTC, Douglas Bagnall
abartlet: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Baumbach 2017-10-23 16:23:04 UTC
Created attachment 13715 [details]
user 3 times in group

Since 4.7.0 there is an issue with the linked attribute handling.

This allows for example to add a user multiple times to one group. Editing memberships leads to inconsistent memberships within the AD with multiple DCs.
Comment 1 Björn Baumbach 2017-10-23 16:25:07 UTC
Created attachment 13716 [details]
user 1 time removed from group
Comment 2 Björn Baumbach 2017-10-23 16:26:13 UTC
Created attachment 13717 [details]
dbcheck output
Comment 3 Björn Baumbach 2017-10-23 16:59:12 UTC
This is reproducible by using the ldbedit tool. Open a group object, just copy the "member" attribute and insert it multiple times.

Versions prior 4.7.0 block this change with the following message:
Failed to commit transaction: Failed to add backlink from CN=g_grouptest1,CN=Users,DC=DOM,DC=local to CN=u_grouptest1,CN=Users,DC=DOM,DC=local - attribute 'memberOf': value #0 on 'CN=u_grouptest1,CN=Users,DC=DOM,DC=local' already exists
Comment 4 Garming Sam 2017-10-23 22:28:25 UTC
This appears to only trigger in the replace case (which is triggered by ldbedit). A standard modify with the member add triggers an explicit check (which it did not used to have). The replace case doesn't appear to have this check.
Comment 5 Douglas Bagnall 2017-10-25 04:07:48 UTC
Created attachment 13721 [details]
patch to test and fix the MOD_REPLACE case

thanks Björn.

I think this should fix the problem.

Looking into this sparked related patches which you can see in 

http://git.catalyst.net.nz/gitweb?p=samba.git;a=shortlog;h=refs/heads/la-fix

but I think these two are enough to fix the bug and should be backported.
Comment 6 Björn Baumbach 2017-10-25 11:07:16 UTC
(In reply to Douglas Bagnall from comment #5)
Thank you Douglas!

I confirm that the patch is working. I've verified this by using the patch on v4-7-test with ldbedit and ldbmodify using the replace command.
Comment 7 Douglas Bagnall 2017-10-26 22:22:18 UTC
Created attachment 13729 [details]
patch for 4.7.1

The previously attached patches cherry-picked from master onto origin/v4-7-test.
Comment 8 Andrew Bartlett 2017-10-27 00:05:05 UTC
Comment on attachment 13729 [details]
patch for 4.7.1

Thanks.  Please pick for 4.7.next
Comment 9 Karolin Seeger 2017-11-01 09:40:04 UTC
(In reply to Andrew Bartlett from comment #8)
Pushed to autobuild-v4-7-test.
Comment 10 Karolin Seeger 2017-11-03 10:04:10 UTC
Pushed to v4-7-test.
Closing out bug report.

Thanks!
Comment 11 Stefan Metzmacher 2017-11-03 10:05:43 UTC
Keep the bug open for the dbcheck fixes
Comment 12 Maxence Sartiaux 2017-11-08 13:01:57 UTC
Hello,

Users who got impacted by this bug in 4.7.0 has now an inconsistent database and replication errors in 4.7.1.

I'm unable to fix it using dbcheck neither manually by removing members from the group and ldbedit/ldbmodify 

"objectclass_attrs: attribute 'memberOf' on entry 'CN=test user,OU=CPAS,OU=MUSERS,DC=contoso,DC=com" must not be modified directly, it is a linked attribute"
Comment 13 Stefan Metzmacher 2017-11-08 13:56:41 UTC
(In reply to Maxence Sartiaux from comment #12)

http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/abartlet-dbcheck-links-2017-11 has fixes for dbcheck.

Hopefully they will be ready for 4.7.2