From cb1ef86c225632f7ac2d267a5e3687b156ffc3bf Mon Sep 17 00:00:00 2001

From: Noriko Hosoi <nhosoi@redhat.com>

Date: Fri, 21 Mar 2014 14:21:13 -0700

Subject: [PATCH 190/225] 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.

https://fedorahosted.org/389/ticket/47748

Reviewed by rmeggins@redhat.com (Thank you, Rich!!)

(cherry picked from commit 4fc53e1a63222d0ff67c30a59f2cff4b535f90a8)

(cherry picked from commit f8f063c3fb2c7642506cbad923c71972f78edac2)

(cherry picked from commit feed9ba773766ade744ca4d45aca46c91d4b7f4f)

(cherry picked from commit f9b2bec732645bf0d219e790448b191c4652858e)

---

 ldap/servers/slapd/bind.c    | 105 ++++++++++++++++++++++++++-----------------

 ldap/servers/slapd/entry.c   |  18 ++++----

 ldap/servers/slapd/pw_mgmt.c |   3 ++

 3 files changed, 75 insertions(+), 51 deletions(-)

diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c

index ec874cc..97b236e 100644

--- a/ldap/servers/slapd/bind.c

+++ b/ldap/servers/slapd/bind.c

@@ -429,6 +429,7 @@ do_bind( Slapi_PBlock *pb )

         if (!strcasecmp (saslmech, LDAP_SASL_EXTERNAL)) {

             /* call preop plugins */

             if (plugin_call_plugins( pb, SLAPI_PLUGIN_PRE_BIND_FN ) != 0){

+                send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "", 0, NULL);

                 goto free_and_return;

             }

@@ -474,10 +475,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 );

@@ -553,6 +554,8 @@ do_bind( Slapi_PBlock *pb )

                 /* call postop plugins */

                 plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );

+            } else {

+                send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "", 0, NULL);

             }

             goto free_and_return;

         /* Check if unauthenticated binds are allowed. */

@@ -651,7 +654,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 );

@@ -675,6 +678,8 @@ do_bind( Slapi_PBlock *pb )

             /* call postop plugins */

             plugin_call_plugins( pb, SLAPI_PLUGIN_POST_BIND_FN );

+        } else {

+            send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "", 0, NULL);

         }

         goto free_and_return;

     }

@@ -733,9 +738,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,53 +761,68 @@ 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) {

+                                rc = slapi_check_account_lock(pb, bind_target_entry,

+                                                              pw_response_requested, 1, 1);

+                                if (1 == rc) { /* account is locked */

+                                    goto account_locked;

+                                }

+                            } else {

+                                send_ldap_result(pb, LDAP_NO_SUCH_OBJECT, NULL, "", 0, NULL);

+                                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 (rc) {

+                            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; need_new_pw already send_ldap_result in it. */

+                        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 {

+account_locked:                

                 if(cred.bv_len == 0) {

                     /* its an UnAuthenticated Bind, DN specified but no pw */

                     slapi_counter_increment(g_get_global_snmp_vars()->ops_tbl.dsUnAuthBinds);

@@ -837,7 +858,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/entry.c b/ldap/servers/slapd/entry.c

index 04bac0d..d9226af 100644

--- a/ldap/servers/slapd/entry.c

+++ b/ldap/servers/slapd/entry.c

@@ -2654,8 +2654,8 @@ slapi_entry_attr_get_charray( const Slapi_Entry* e, const char *type)

 char *

 slapi_entry_attr_get_charptr( const Slapi_Entry* e, const char *type)

 {

-    char *p= NULL;

-    Slapi_Attr* attr;

+	char *p= NULL;

+	Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

 	if(attr!=NULL)

 	{

@@ -2674,7 +2674,7 @@ int

 slapi_entry_attr_get_int( const Slapi_Entry* e, const char *type)

 {

     int r= 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2689,7 +2689,7 @@ unsigned int

 slapi_entry_attr_get_uint( const Slapi_Entry* e, const char *type)

 {

     unsigned int r= 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2704,7 +2704,7 @@ long

 slapi_entry_attr_get_long( const Slapi_Entry* e, const char *type)

 {

     long r = 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2719,7 +2719,7 @@ unsigned long

 slapi_entry_attr_get_ulong( const Slapi_Entry* e, const char *type)

 {

     unsigned long r = 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2734,7 +2734,7 @@ long long

 slapi_entry_attr_get_longlong( const Slapi_Entry* e, const char *type)

 {

     long long r = 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

     slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2749,7 +2749,7 @@ unsigned long long

 slapi_entry_attr_get_ulonglong( const Slapi_Entry* e, const char *type)

 {

     unsigned long long r = 0;

-    Slapi_Attr* attr;

+    Slapi_Attr* attr = NULL;

     slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

@@ -2764,7 +2764,7 @@ PRBool

 slapi_entry_attr_get_bool( const Slapi_Entry* e, const char *type)

 {

     PRBool r = PR_FALSE; /* default if no attr */

-    Slapi_Attr* attr;

+	Slapi_Attr* attr = NULL;

 	slapi_entry_attr_find(e, type, &attr);

     if (attr!=NULL)

     {

diff --git a/ldap/servers/slapd/pw_mgmt.c b/ldap/servers/slapd/pw_mgmt.c

index f173128..7a8d390 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