Bug 9048 - Samba4 ldap error codes
Summary: Samba4 ldap error codes
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.6.0rc2
Hardware: All All
: P5 enhancement (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: samba4-qa@samba.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-18 08:59 UTC by miquel
Modified: 2017-03-13 10:16 UTC (History)
4 users (show)

See Also:


Attachments
Work in progress patch for master (2.25 KB, patch)
2017-02-24 17:38 UTC, Stefan Metzmacher
no flags Details
Patches for v4-6-test (21.58 KB, patch)
2017-03-06 08:11 UTC, Stefan Metzmacher
abartlet: review+
Details
Patches for v4-5-test (21.58 KB, patch)
2017-03-06 09:20 UTC, Stefan Metzmacher
abartlet: review+
Details
Patches for v4-6-test (part1 without tests) (3.68 KB, patch)
2017-03-06 18:38 UTC, Stefan Metzmacher
metze: review? (slow)
abartlet: review+
Details
Patches for v4-5-test (part1 without tests) (3.68 KB, patch)
2017-03-07 07:26 UTC, Stefan Metzmacher
abartlet: review+
metze: review? (slow)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description miquel 2012-07-18 08:59:50 UTC
The error codes from samba4, not follow the same sintax that AD error codes:

for the same error:
Samba4:
Enter LDAP Password:
ldap_bind: Invalid credentials (49)
        additional info: Simple Bind Failed: NT_STATUS_PASSWORD_MUST_CHANGE

AD
Enter LDAP Password:                                                                                                                                        
ldap_bind: Invalid credentials (49)
        additional info: 80090308: LdapErr: DSID-0C0903A9, comment: AcceptSecurityContext error, data 773, v1db1


Some softwares like openam/opensso uses this return codes to manage password change/expiration, etc..
Comment 1 Arvid Requate 2013-06-26 17:32:48 UTC
The error code is the same: "code = 49, message = Invalid credentials", only the extendederrormessage contains the reason in the code after the "data" keyword. Samba cannot guess which kind of pattern matching different clients try to extract this information. I guess it's a bit much to ask from Samba to mimic this error message 1:1, at least at this stage of development. I guess the proper way to detect this would have to be implemented in the client by trying a netlogon to retrieve additional information on the circumstances of the problem.
Comment 2 Matthias Dieter Wallnöfer 2014-04-24 19:25:44 UTC
Yes, we do our best to match the main LDAP error code and I have not seen any tool yet which expects also the extended error message to be the same as on Windows.
The bug is valid, but I mark it as ENHANCEMENT.
Comment 3 Andrew Bartlett 2017-01-27 11:04:28 UTC
For the bind case this is easier than some of the others, because of layering.  The error strings are directly emitted by source4/ldap_server/ldap_bind.c, so they can be fixed there without a big stack above them.

For keycloak we need to output something that matches 
(".*AcceptSecurityContext error, data ([0-9a-f]*), v.*");

https://github.com/keycloak/keycloak/blob/b2d1a1a17fc8f665f4ba83d62e3c2
2d4dfa0048a/federation/ldap/src/main/java/org/keycloak/storage/ldap/map
pers/msad/MSADUserAccountControlStorageMapper.java#L56

That just needs the windows error mapping of the
NT_STATUS_PWD_MUST_CHANGE code in 'data', which isn't hard to get. 

The main trick will be lots of testing, and finding the match patterns for other tools we want to care about.
Comment 4 Andrew Bartlett 2017-01-30 23:53:21 UTC
Does anybody know the pattern openam / opensso uses on the error string?

I can't quickly find it with a github search.
Comment 5 Stefan Metzmacher 2017-02-24 17:38:24 UTC
Created attachment 12975 [details]
Work in progress patch for master

I'll try an autobuild with this and see what existing test might fail.

Then I'll add specific regresion tests.
Comment 6 Andrew Bartlett 2017-02-24 19:18:11 UTC
(In reply to Stefan Metzmacher from comment #5)
Thanks for decoding the HRESULT end of this puzzle.  The patch looks like what I expected.

When writing the tests, remember we now have the windows errors available as python constants, which should make this much nicer to test.  

Hopefully source4/dsdb/tests/python/password_lockout.py can use this.
Comment 7 Stefan Metzmacher 2017-03-03 07:25:27 UTC
(In reply to Andrew Bartlett from comment #6)

I added a test to source4/dsdb/tests/python/sam.py,
but we don't have python bindings for HRESULT yet,
only ntstatus and werror.
Comment 8 Andrew Bartlett 2017-03-04 18:24:46 UTC
(In reply to Stefan Metzmacher from comment #7)
Thanks.  The patches are in master now as ad07bbcb9057e8cfc9ef6c6e8c4da846882f87bb

I'll ask Bob to extend python generation to HRESULT when I next see him, it shouldn't be too hard.
Comment 9 Stefan Metzmacher 2017-03-06 08:11:21 UTC
Created attachment 13019 [details]
Patches for v4-6-test
Comment 10 Andrew Bartlett 2017-03-06 08:51:52 UTC
Comment on attachment 13019 [details]
Patches for v4-6-test

Looks great!
Comment 11 Karolin Seeger 2017-03-06 09:08:42 UTC
Pushed to autobuild-v4-6-test.
Comment 12 Stefan Metzmacher 2017-03-06 09:20:15 UTC
Created attachment 13020 [details]
Patches for v4-5-test
Comment 13 Karolin Seeger 2017-03-06 12:27:17 UTC
(In reply to Stefan Metzmacher from comment #12)
Pushed to autobuild-v4-5-test.
Comment 14 Stefan Metzmacher 2017-03-06 18:38:29 UTC
Created attachment 13024 [details]
Patches for v4-6-test (part1 without tests)

The tests don't work in 4.6 and 4.5, we'll backport them later.
Comment 15 Andrew Bartlett 2017-03-06 21:08:11 UTC
Comment on attachment 13024 [details]
Patches for v4-6-test (part1 without tests)

Adding it without tests is fine - any changes will have go via master where the tests are anyway, and we are unlikely to accidentally muck this up.

I should have remembered this was building on the werror in python work from your other comments. :-)
Comment 16 Stefan Metzmacher 2017-03-07 07:26:45 UTC
Created attachment 13025 [details]
Patches for v4-5-test (part1 without tests)
Comment 17 Andrew Bartlett 2017-03-07 07:39:03 UTC
Comment on attachment 13025 [details]
Patches for v4-5-test (part1 without tests)

As before, no tests is fine as it is tested in master and pretty hard to break.

BTW, I think nt_status_squash() is being called twice, but that is harmless :-)
Comment 18 Stefan Metzmacher 2017-03-13 10:16:43 UTC
Fixed in Samba 4.5.6 and 4.6.0