From 4cef53d3cdcab00346dee5872e43a3c1c2a9c1dc Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Thu, 20 Mar 2014 17:40:45 -0700 Subject: [PATCH] Ticket #47748 - Simultaneous adding a user and binding as the user could fail in the password policy check Bug description: In do_bind, bind_target_entry is retrieved from the DB or the entry cache. There was a small window that the entry failed to retrieve from there but the bind procedure in the backend (be_bind) succeeds. In the case, NULL bind_target_entry is passed to the Pass- word Policy check and it fails. Fix description: If be_bind returns SUCCESS and bind_target_entry is NULL, retrieve bind_target_entry agian, which is guaranteed since the entry was retrieved in the backend and placed in the entry cache. --- ldap/servers/slapd/bind.c | 91 ++++++++++++++++++++++++-------------------- ldap/servers/slapd/pw_mgmt.c | 3 ++ 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c index cb563f0..0a889a0 100644 --- a/ldap/servers/slapd/bind.c +++ b/ldap/servers/slapd/bind.c @@ -474,10 +474,10 @@ do_bind( Slapi_PBlock *pb ) goto free_and_return; } - if (!isroot ) { + if (!isroot) { /* check if the account is locked */ bind_target_entry = get_entry(pb, pb->pb_conn->c_external_dn); - if ( bind_target_entry != NULL && slapi_check_account_lock(pb, bind_target_entry, + if ( bind_target_entry && slapi_check_account_lock(pb, bind_target_entry, pw_response_requested, 1 /*check password policy*/, 1 /*send ldap result*/) == 1) { /* call postop plugins */ plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN ); @@ -651,7 +651,7 @@ do_bind( Slapi_PBlock *pb ) bind_credentials_set( pb->pb_conn, SLAPD_AUTH_SIMPLE, slapi_ch_strdup(slapi_sdn_get_ndn(sdn)), NULL, NULL, NULL , NULL); } else { - /* + /* * right dn, wrong passwd - reject with invalid credentials */ send_ldap_result( pb, LDAP_INVALID_CREDENTIALS, NULL, NULL, 0, NULL ); @@ -733,9 +733,10 @@ do_bind( Slapi_PBlock *pb ) (rc == SLAPI_BIND_ANONYMOUS))) ) { long t; char* authtype = NULL; - - if(auto_bind) + /* rc is SLAPI_BIND_SUCCESS or SLAPI_BIND_ANONYMOUS */ + if(auto_bind) { rc = SLAPI_BIND_SUCCESS; + } switch ( method ) { case LDAP_AUTH_SIMPLE: @@ -755,52 +756,60 @@ do_bind( Slapi_PBlock *pb ) /* authtype = SLAPD_AUTH_SASL && saslmech: */ PR_snprintf(authtypebuf, sizeof(authtypebuf), "%s%s", SLAPD_AUTH_SASL, saslmech); authtype = authtypebuf; - break; - default: /* ??? */ + break; + default: break; } if ( rc == SLAPI_BIND_SUCCESS ) { - if(!auto_bind) - bind_credentials_set( pb->pb_conn, - authtype, slapi_ch_strdup( - slapi_sdn_get_ndn(sdn)), - NULL, NULL, NULL, bind_target_entry ); - if ( auth_response_requested ) { - slapi_add_auth_response_control( pb, - slapi_sdn_get_ndn(sdn)); + if (!auto_bind) { + /* + * There could be a race that bind_target_entry was not added + * when bind_target_entry was retrieved before be_bind, but it + * was in be_bind. Since be_bind returned SLAPI_BIND_SUCCESS, + * the entry is in the DS. So, we need to retrieve it once more. + */ + if (!bind_target_entry) { + bind_target_entry = get_entry(pb, slapi_sdn_get_ndn(sdn)); + if (!bind_target_entry) { + goto free_and_return; + } + } + bind_credentials_set(pb->pb_conn, authtype, + slapi_ch_strdup(slapi_sdn_get_ndn(sdn)), + NULL, NULL, NULL, bind_target_entry); + if (!slapi_be_is_flag_set(be, SLAPI_BE_FLAG_REMOTE_DATA)) { + /* check if need new password before sending + the bind success result */ + rc = need_new_pw(pb, &t, bind_target_entry, pw_response_requested); + switch (need_new_pw(pb, &t, bind_target_entry, pw_response_requested)) { + case 1: + (void)slapi_add_pwd_control(pb, LDAP_CONTROL_PWEXPIRED, 0); + break; + case 2: + (void)slapi_add_pwd_control(pb, LDAP_CONTROL_PWEXPIRING, t); + break; + default: + break; + } + } + } + if (auth_response_requested) { + slapi_add_auth_response_control(pb, slapi_sdn_get_ndn(sdn)); } + if (-1 == rc) { /* neeed_new_pw failed */ + goto free_and_return; + } } else { /* anonymous */ + /* set bind creds here so anonymous limits are set */ - bind_credentials_set( pb->pb_conn, authtype, NULL, - NULL, NULL, NULL, NULL ); + bind_credentials_set(pb->pb_conn, authtype, NULL, NULL, NULL, NULL, NULL); if ( auth_response_requested ) { - slapi_add_auth_response_control( pb, - "" ); + slapi_add_auth_response_control(pb, ""); } } - - if ( 0 == auto_bind && (rc != SLAPI_BIND_ANONYMOUS) && - ! slapi_be_is_flag_set(be, SLAPI_BE_FLAG_REMOTE_DATA)) { - /* check if need new password before sending - the bind success result */ - switch ( need_new_pw (pb, &t, bind_target_entry, pw_response_requested )) { - case 1: - (void)slapi_add_pwd_control ( pb, - LDAP_CONTROL_PWEXPIRED, 0); - break; - case 2: - (void)slapi_add_pwd_control ( pb, - LDAP_CONTROL_PWEXPIRING, t); - break; - case -1: - goto free_and_return; - default: - break; - } - } /* end if */ - }else{ + } else { if(cred.bv_len == 0) { /* its an UnAuthenticated Bind, DN specified but no pw */ @@ -837,7 +846,7 @@ do_bind( Slapi_PBlock *pb ) "Function not implemented", 0, NULL ); } - free_and_return:; +free_and_return:; if (be) slapi_be_Unlock(be); slapi_pblock_get(pb, SLAPI_BIND_TARGET_SDN, &sdn); diff --git a/ldap/servers/slapd/pw_mgmt.c b/ldap/servers/slapd/pw_mgmt.c index 8e8d850..5a0ecb2 100644 --- a/ldap/servers/slapd/pw_mgmt.c +++ b/ldap/servers/slapd/pw_mgmt.c @@ -68,6 +68,9 @@ need_new_pw( Slapi_PBlock *pb, long *t, Slapi_Entry *e, int pwresponse_req ) int pwdGraceUserTime = 0; char graceUserTime[8]; + if (NULL == e) { + return (-1); + } slapi_mods_init (&smods, 0); sdn = slapi_entry_get_sdn_const( e ); dn = slapi_entry_get_ndn( e ); -- 1.8.1.4