Bug 14029 - LDAP not obeying extended DN control correctly
Summary: LDAP not obeying extended DN control correctly
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.11.0rc1
Hardware: All All
: P5 regression (vote)
Target Milestone: 4.11
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-08 04:58 UTC by Garming Sam
Modified: 2019-08-09 08:12 UTC (History)
2 users (show)

See Also:


Attachments
Patch for master (no test) (969 bytes, patch)
2019-07-08 05:01 UTC, Garming Sam
no flags Details
Backported patch for 4.11 (1.18 KB, patch)
2019-07-17 04:47 UTC, Garming Sam
no flags Details
Patch for 4.11 (with test) (11.45 KB, patch)
2019-08-01 22:57 UTC, Garming Sam
abartlet: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Garming Sam 2019-07-08 04:58:43 UTC
The change to build encoded responses while the database lock is held in current master (commit 0559430ab6e5c48d6e853fda0d8b63f2e149015c) doesn't obey the extended DN control anymore.

extended_type is used after it has already been assigned to the callback context.
Comment 1 Garming Sam 2019-07-08 05:01:09 UTC
Created attachment 15293 [details]
Patch for master (no test)

Still needs a test to show what behaviour changed without this patch.
Comment 2 Garming Sam 2019-07-08 05:02:34 UTC
The observation was that 2012 R2 was failing to join Samba during its initial checks of the credentials and domain level versioning. From the network traces, it wasn't obvious what was different between the successful and unsuccessful cases (and so I did a git bisection).
Comment 3 Garming Sam 2019-07-08 07:32:29 UTC
I think the variable should just be removed, leaving it around unnecessarily is what caused it to be overlooked.
Comment 4 Garming Sam 2019-07-09 02:56:46 UTC
Interestingly enough, since this code is only in the ldap server, ldbsearch on a local database doesn't respect the extended_dn controls for the top level DN of each object (while respecting it elsewhere).
Comment 5 Garming Sam 2019-07-09 02:58:39 UTC
This doesn't seem to be trivially able to be tested in python, because the msg->dn has already been parsed as an actual DN object and returned to python. By this point, the original information on how the GUID was encoded is now lost.
Comment 6 Garming Sam 2019-07-09 03:07:57 UTC
(In reply to Garming Sam from comment #4)

Correction: ldbsearch always prints extended_type = 1 specifically. I don't think there's anywhere straightforward in our client stack where we could detect this correctly. I would really like to write a test for this, but there doesn't seem to be an obvious way to do so.

To add: Joining the Windows machine as a regular computer was fine without this patch, but the DC provisioning failed.
Comment 7 Garming Sam 2019-07-17 04:47:46 UTC
Created attachment 15314 [details]
Backported patch for 4.11
Comment 8 Garming Sam 2019-08-01 22:57:36 UTC
Created attachment 15363 [details]
Patch for 4.11 (with test)
Comment 9 Garming Sam 2019-08-01 23:00:14 UTC
Added patch with tests.
Comment 10 Andrew Bartlett 2019-08-01 23:49:25 UTC
G'Day Karolin,

This needs to be in before samba 4.11.0.

Thanks!
Comment 11 Karolin Seeger 2019-08-07 10:48:18 UTC
(In reply to Andrew Bartlett from comment #10)
Pushed to autobuild-v4-11-test.
Comment 12 Karolin Seeger 2019-08-09 08:12:03 UTC
(In reply to Karolin Seeger from comment #11)
Pushed to v4-11-test.
Closing out bug report.

Thanks!