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..
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.
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.
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.
Does anybody know the pattern openam / opensso uses on the error string? I can't quickly find it with a github search.
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.
(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.
(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.
(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.
Created attachment 13019 [details] Patches for v4-6-test
Comment on attachment 13019 [details] Patches for v4-6-test Looks great!
Pushed to autobuild-v4-6-test.
Created attachment 13020 [details] Patches for v4-5-test
(In reply to Stefan Metzmacher from comment #12) Pushed to autobuild-v4-5-test.
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 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. :-)
Created attachment 13025 [details] Patches for v4-5-test (part1 without tests)
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 :-)
Fixed in Samba 4.5.6 and 4.6.0