sdap: provide error message when password change fail in ldap_modify mode

[pbrezina]

Steps to reproduce:

1. Configure LDAP server to enable password constraints
2. Set ldap_pwmodify_mode = ldap_modify in [domain]
3. Run SSSD and authenticate as a user
4. Run passwd to change password, use password that does not meet requirements

It will print "password change successful" without this patch and server
error message with this patch applied.

Resolves:
https://pagure.io/SSSD/sssd/issue/4148

[pbrezina]

Downstream tests passed.

[mzidek-gh]

TLDR please initialize ldap_msg to NULL here:

 909 static void sdap_modify_passwd_done(struct tevent_req *subreq)
 910 {
 911     struct tevent_req *req;
 912     struct sdap_modify_passwd_state *state;
 913     int ldap_result;
 914     char *ldap_msg;
               ^^^^^^^^ init this to NULL
 915     errno_t ret;

Explanation:

If in sdap_modify_recv the _ldap_msg equals NULL here:

 842     if (_ldap_msg != NULL) {
             ^^^^^^^^^^^^^^^^^ If this is false and we do not get here
 843         *_ldap_msg = talloc_steal(mem_ctx, state->ldap_msg);
 844     }
 845 
 846     return EOK;
 847 }

then in sdap_modify_passwd_done we enter the function sdap_chpass_result
with uninitialized value of ldap_msg here:

 929     ret = sdap_chpass_result(state, ldap_result, ldap_msg, &state->user_msg);
                                                      ^^^^^^^^ This is not initialized

and then all conditions in sdap_chpass_result with ldap_msg may have some garbage value in it.

Other than that the code LGTM but I still have to test it.

[pbrezina]

TLDR please initialize ldap_msg to NULL here:

 909 static void sdap_modify_passwd_done(struct tevent_req *subreq)
 910 {
 911     struct tevent_req *req;
 912     struct sdap_modify_passwd_state *state;
 913     int ldap_result;
 914     char *ldap_msg;
               ^^^^^^^^ init this to NULL
 915     errno_t ret;

Explanation:

If in sdap_modify_recv the _ldap_msg equals NULL here:

 842     if (_ldap_msg != NULL) {
             ^^^^^^^^^^^^^^^^^ If this is false and we do not get here
 843         *_ldap_msg = talloc_steal(mem_ctx, state->ldap_msg);
 844     }
 845 
 846     return EOK;
 847 }

Sorry, I can't see how we can get sdap_modify_recv(state, subreq, &ldap_result, NULL) from sdap_modify_recv(state, subreq, &ldap_result, &ldap_msg).

then in sdap_modify_passwd_done we enter the function sdap_chpass_result
with uninitialized value of ldap_msg here:

 929     ret = sdap_chpass_result(state, ldap_result, ldap_msg, &state->user_msg);
                                                      ^^^^^^^^ This is not initialized

and then all conditions in sdap_chpass_result with ldap_msg may have some garbage value in it.

Other than that the code LGTM but I still have to test it.

[mzidek-gh]

Sorry, I can't see how we can get sdap_modify_recv(state, subreq, &ldap_result, NULL) from sdap_modify_recv(state, subreq, &ldap_result, &ldap_msg).

Well, of course you can not :) My bad. Removing changes requested.

[mzidek-gh]

When changing passwd to "foo" (does not meet requirements).

Fedora 30 with git master:

Changing password for user user-1.
Current Password: 
New password: 
Retype new password: 
passwd: all authentication tokens updated successfully.

master + this patch:

Current Password: 
New password: 
Retype new password: 
Password change failed. Server message: invalid password syntax - password must be at least 8 characters long
passwd: Authentication token is no longer valid; new one required

ACK.

[pbrezina]

• master
  • e4c6ebf - sdap: provide error message when password change fail in ldap_modify mode
• sssd-1-16
  • ddf0a59 - sdap: provide error message when password change fail in ldap_modify mode