Bug 41864 - UCS@school: Allow re-creation of deleted user object with same objectSid in Samba/AD
UCS@school: Allow re-creation of deleted user object with same objectSid in S...
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: S4 Connector
UCS 4.1
Other Linux
: P2 major (vote)
: UCS 4.1-2-errata
Assigned To: Arvid Requate
Stefan Gohmann
https://download.samba.org/pub/samba/...
:
Depends on: 32263 41756
Blocks: 41906
  Show dependency treegraph
 
Reported: 2016-07-27 14:18 CEST by Arvid Requate
Modified: 2018-07-30 09:55 CEST (History)
6 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 4: A User would return the product
User Pain: 0.343
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): External feedback, Release Goal, Troubleshooting
Max CVSS v3 score:
requate: Patch_Available+


Attachments
ucs2con_allow_recreation_of_deleted_object_with_same_sAMAccountName_and_objectSid.diff (2.26 KB, patch)
2016-07-27 14:18 CEST, Arvid Requate
Details | Diff
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff (3.04 KB, patch)
2016-07-28 14:23 CEST, Arvid Requate
Details | Diff
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff (3.21 KB, patch)
2016-07-28 14:32 CEST, Arvid Requate
Details | Diff
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff (3.81 KB, patch)
2016-07-28 22:15 CEST, Arvid Requate
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2016-07-27 14:18:12 CEST
Created attachment 7841 [details]
ucs2con_allow_recreation_of_deleted_object_with_same_sAMAccountName_and_objectSid.diff

UCS@school 4.1 R2 allows users to get replicated into multiple schools 
(Bug 40870, Bug 41115, Bug 41118), but the S4-Connector on the School-DCs fails to re-create a user object in Samba/AD that has been removed before.

When a user object is deleted in Samba/AD, a stripped down version of it is kept in the "CN=Deleted Objects" container. In UCS@school, due to selective replication, user objects can get deleted and recreated (see Bug 41756) easily just by moving them or by changing the "schools" property in UDM (affecting the ucsschoolSchool attribute in OpenLDAP).

In this case the OpenLDAP user objects don't only keep their OpenLDAP entryUUID but also their sambaSID. In UCS@school the S4-Connector forces Samba/AD to accept the OpenLDAP sambaSID (UCR: connector/s4/mapping/sid_to_s4=yes). Since the Deleted Object still holds that SID in it's objectSid attribute, the S4-Connector will get an exception from Samba/AD (ldap.ALREADY_EXISTS exception: "Failed to re-index objectSid" due to "unique index violation on objectSid").

Reducing tombstoneLifetime to the minimum of 1 day is not enough for production use.

The attached patch proposal (plus patch for Bug 41756) may fix the issue.
Comment 1 Markus Dählmann 2016-07-27 18:53:25 CEST
Just as a note, as far as I know, tombstone objects are important for DRS replication. Since UCS@School allows having a second logon server in a school (http://docs.software-univention.de/ucsschool-handbuch-4.1R2.html#school:performance:secondarySamba4DC), I hope this patch doesn't break anything there. We don't use this scenario in our environment though.
Comment 2 Arvid Requate univentionstaff 2016-07-27 20:37:44 CEST
Yes, I agree, a better approach would be to restore the Deleted Object, keeping its objectGUID. Let's see, maybe we can improve upon this at some point. This was the quick fix.

If I re-vive the Deleted Object it only has a reduced subset of attributes. In that case the current architecture of the S4-Connector would probably require to reject the UCS object in the first step after the initial restore of the AD-object, to initiate a new computation of the attribute modification list that is required to bring the AD-object back in sync with the UCS object. That would mean a delay and resorting of changes which could possibly cause other issues. Fixing this in the S4-Connector to make it resolve the conflict directly at first attempt requires some careful plumbing.
Comment 3 Arvid Requate univentionstaff 2016-07-28 14:23:38 CEST
Created attachment 7845 [details]
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff

This is a first sketch for the "tombstone reanimate" approach. It's incomplete, as I said in the comment before, as the modlist should be computed again. This first sketch only tries to "reanimate" the deleted object and then rejects the add. No clue what happens next. I didn't even test this patch yet, so don't try it in production, chances are that the S4-Connector will exit with a traceback.
Comment 4 Arvid Requate univentionstaff 2016-07-28 14:32:21 CEST
Created attachment 7846 [details]
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff

Maybe this works, I just call sync_from_ucs again after tombstone reanimation.
Comment 5 Florian Best univentionstaff 2016-07-28 16:07:51 CEST
(In reply to Arvid Requate from comment #4)
> Created attachment 7846 [details]
> ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objec
> tSid.diff
> 
> Maybe this works, I just call sync_from_ucs again after tombstone
> reanimation.

Missing filter-escaping! use ldap.filter.filter_format().
Comment 6 Arvid Requate univentionstaff 2016-07-28 22:15:22 CEST
Created attachment 7847 [details]
ucs2con_tombstone_reanimate_deleted_object_with_same_sAMAccountName_and_objectSid.diff

Ok, fixed some glitches. The first reanimation step works, but the final sync fails with some object class violation during ldap.MOD_REPLACE on primaryGroupID.
This needs more R&D time learning from the Samba code how this needs to be done.
There are some more steps in source4/dsdb/samdb/ldb_modules/tombstone_reanimate.c regarding primaryGroupID which I didn't reimplement yet.
Comment 7 Arvid Requate univentionstaff 2016-07-29 00:01:03 CEST
Ok, I checked the source, the tombstone reanimation via python will be blocked due to the fact that samldb_prim_group_change in ./source4/dsdb/samdb/ldb_modules/samldb.c doesn't allow this. The lost primaryGroupID attribute can only be modified if it is present or if the object itself is getting added. The only way around this is implemented in the "tombstone_reanimate" LDB module, which for some reason yet unknown is not activated in the current source code. Apparently the test case has shown flaky behaviour. None the less, to take Comment 1 into account, I would recommend enabling it by patching the Samba source and then implementing the proper tombstone reanimation procedure (via LDAP modify) in the S4-Connector. Until then, the initial (first) patch attached here is the only workaround I can currently propose.
Comment 8 Arvid Requate univentionstaff 2016-08-02 23:00:44 CEST
Package rebuilt in errata4.1-2 with the first patch applied.

The second patch is not usable, we will postpone the "proper" tombstone reanimation until Samba 4.5.x has been released. The feature is already activated in the 4.5.0 RC1, see URL above.


Advisory: univention-s4-connector.yaml
Comment 9 Stefan Gohmann univentionstaff 2016-08-03 06:36:50 CEST
Code review: Failed

From the code:

sAMAccountName_attr_value = object['attributes'].get('sAMAccountName')[0]
objectSid_attr_value = object['attributes'].get('objectSid')[0]
objectSid = decode_sid(objectSid_attr_value)
if not (sAMAccountName_attr_value and objectSid):
    raise   ## unknown situation

I think it is a problem if the attributes dict doesn't have the attributes sAMAccountName or objectSid, e.g.:

>>> ob = {}
>>> ob['attributes'] = {}
>>> ob['attributes']['sAMAccountName']=['foo']
>>> a = ob['attributes'].get('sAMAccountName')[0]
>>> a = ob['attributes'].get('objectSid')[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object has no attribute '__getitem__'
>>> 

YAML: OK

Jenkins tests: tbd

Manual tests: tbd
Comment 10 Stefan Gohmann univentionstaff 2016-08-03 06:40:16 CEST
Can you also change the debug levels?


ud.debug(ud.LDAP, ud.WARN, "sync_from_ucs: Error during add, searching for conflicting deleted object in S4. Filter: %s" % filter_s4)

→ PROCESS or INFO

ud.debug(ud.LDAP, ud.WARN,"sync_from_ucs: No conflicting object found.")

→ PROCESS

ud.debug(ud.LDAP, ud.INFO,"sync_from_ucs: Ok, deleting conflicting object: %s"% result[0][0])

→ PROCESS
Comment 11 Arvid Requate univentionstaff 2016-08-03 12:58:40 CEST
Package rebuilt with suggested changes, advisory updated.
Comment 12 Stefan Gohmann univentionstaff 2016-08-03 15:10:03 CEST
Code review: OK

YAML: OK

Jenkins tests: OK

Manual tests: OK

That looks good:

22.12.2015 22:56:13,64 LDAP        (PROCESS): __sync_file_from_ucs: Object with entryUUID b237ab5a-3cd5-1035-9ba1-4d9447a69253 has been removed before but became visible again.
22.12.2015 22:56:13,73 LDAP        (PROCESS): sync from ucs: [          user] [       add] cn=anton12-schule1,cn=schueler,cn=users,ou=schule1,DC=test,DC=de
22.12.2015 22:56:13,112 LDAP        (PROCESS): sync_from_ucs: error during add, searching for conflicting deleted object in S4
22.12.2015 22:56:13,112 LDAP        (PROCESS): sync_from_ucs: deleting conflicting object: cn=anton12-schule1\0ADEL:ae7dc243-9470-4920-ba51-604c3758816a,CN=Deleted Objects,DC=test,DC=de
22.12.2015 22:56:14,424 LDAP        (PROCESS): sync to ucs:   [          user] [    modify] uid=anton12-schule1,cn=schueler,cn=users,ou=schule1,dc=test,dc=de
Comment 13 Janek Walkenhorst univentionstaff 2016-08-03 15:56:57 CEST
<http://errata.software-univention.de/ucs/4.1/224.html>