StartTLS now grabs conn->c_mutex before mucking with connection.

Add comment concerning sequencing issues which need to be resolved
by reworking of connection state machine.  Add note that a race
condition exists until this rework is complete.
Rework extended operations to return pointer to static error text.

servers/slapd/back-ldbm/passwd.c

@@ -74,7 +74,7 @@ ldbm_back_exop_passwd(
 	hash = slap_passwd_hash( new );
 
 	if( hash == NULL || hash->bv_len == 0 ) {
-		*text = ch_strdup("password hash failed");
+		*text = "password hash failed";
 		rc = LDAP_OPERATIONS_ERROR;
 		goto done;
 	}
@@ -85,7 +85,7 @@ ldbm_back_exop_passwd(
 		dn, id ? " (proxy)" : "", 0 );
 
 	if( dn == NULL || dn[0] == '\0' ) {
-		*text = ch_strdup("No password is associated with the Root DSE");
+		*text = "No password is associated with the Root DSE";
 		rc = LDAP_OPERATIONS_ERROR;
 		goto done;
 	}
@@ -93,20 +93,20 @@ ldbm_back_exop_passwd(
 	e = dn2entry_w( be, dn, NULL );
 
 	if( e == NULL ) {
-		*text = ch_strdup("could not locate authorization entry");
+		*text = "could not locate authorization entry";
 		rc = LDAP_OPERATIONS_ERROR;
 		goto done;
 	}
 
 	if( ! access_allowed( be, conn, op, e, entry, NULL, ACL_WRITE ) ) {
-		*text = ch_strdup("access to authorization entry denied");
+		*text = "access to authorization entry denied";
 		rc = LDAP_INSUFFICIENT_ACCESS;
 		goto done;
 	}
 
 	if( is_entry_alias( e ) ) {
 		/* entry is an alias, don't allow operation */
-		*text = ch_strdup("authorization entry is alias");
+		*text = "authorization entry is alias";
 		rc = LDAP_ALIAS_PROBLEM;
 		goto done;
 	}
@@ -115,7 +115,7 @@ ldbm_back_exop_passwd(
 
 	if( is_entry_referral( e ) ) {
 		/* entry is an referral, don't allow operation */
-		*text = ch_strdup("authorization entry is referral");
+		*text = "authorization entry is referral";
 		goto done;
 	}
 

servers/slapd/extended.c

@@ -172,9 +172,6 @@ do_extended(
 	if ( rspdata != NULL )
 		ber_bvfree( rspdata );
 
-	if ( text != NULL )
-		free(text);
-
 done:
 	if ( reqdata != NULL ) {
 		ber_bvfree( reqdata );

servers/slapd/libslapd.dsp

@@ -304,6 +304,10 @@ SOURCE=.\slap.h
 # End Source File
 # Begin Source File
 
+SOURCE=.\starttls.c
+# End Source File
+# Begin Source File
+
 SOURCE=.\str2filter.c
 # End Source File
 # Begin Source File

servers/slapd/passwd.c

@@ -35,14 +35,14 @@ int passwd_extop(
 	assert( strcmp( LDAP_EXOP_X_MODIFY_PASSWD, reqoid ) == 0 );
 
 	if( op->o_dn == NULL || op->o_dn[0] == '\0' ) {
-		*text = ch_strdup("only authenicated users may change passwords");
+		*text = "only authenicated users may change passwords";
 		return LDAP_STRONG_AUTH_REQUIRED;
 	}
 
 	if( conn->c_authz_backend != NULL && conn->c_authz_backend->be_extended )
 	{
 		if( global_readonly || conn->c_authz_backend->be_readonly ) {
-			*text = ch_strdup("authorization database is read only");
+			*text = "authorization database is read only";
 			rc = LDAP_UNWILLING_TO_PERFORM;
 
 		} else if( conn->c_authz_backend->be_update_ndn != NULL ) {
@@ -59,7 +59,7 @@ int passwd_extop(
 		}
 
 	} else {
-		*text = ch_strdup("operation not supported for current user");
+		*text = "operation not supported for current user";
 		rc = LDAP_UNWILLING_TO_PERFORM;
 	}
 
@@ -86,7 +86,7 @@ int slap_passwd_parse( struct berval *reqdata,
 	if( ber == NULL ) {
 		Debug( LDAP_DEBUG_TRACE, "slap_passwd_parse: ber_init failed\n",
 			0, 0, 0 );
-		*text = ch_strdup("password decoding error");
+		*text = "password decoding error";
 		return LDAP_PROTOCOL_ERROR;
 	}
 
@@ -162,7 +162,7 @@ decoding_error:
 			"slap_passwd_parse: decoding error, len=%ld\n",
 			(long) len, 0, 0 );
 
-		*text = ch_strdup("data decoding error");
+		*text = "data decoding error";
 		rc = LDAP_PROTOCOL_ERROR;
 	}
 

servers/slapd/starttls.c

@@ -32,15 +32,23 @@ starttls_extop (
 	struct berval *** refs )
 {
 	void *ctx;
+	int rc;
 
 	if ( reqdata != NULL ) {
 		/* no request data should be provided */
+		*text = "no request data expected";
 		return LDAP_PROTOCOL_ERROR;
 	}
 
+	/* acquire connection lock */
+	ldap_pvt_thread_mutex_lock( &conn->c_mutex );
+
 	/* can't start TLS if it is already started */
-	if (conn->c_is_tls != 0)
-		return(LDAP_OPERATIONS_ERROR);
+	if (conn->c_is_tls != 0) {
+		*text = "TLS already started";
+		rc = LDAP_OPERATIONS_ERROR;
+		goto done;
+	}
 
 	/* fail if TLS could not be initialized */
 	if (ldap_pvt_tls_get_option(NULL, LDAP_OPT_X_TLS_CERT, &ctx) != 0
@@ -48,25 +56,41 @@ starttls_extop (
 	{
 		if (default_referral != NULL) {
 			/* caller will put the referral into the result */
-			return(LDAP_REFERRAL);
+			rc = LDAP_REFERRAL;
+			goto done;
 		}
-		return(LDAP_UNAVAILABLE);
+
+		*text = "Could not initialize TLS";
+		rc = LDAP_UNAVAILABLE;
+		goto done;
 	}
 
 	/* can't start TLS if there are other op's around */
-	if (conn->c_ops != NULL) {
-		if (conn->c_ops != op || op->o_next != NULL)
-			return(LDAP_OPERATIONS_ERROR);
-	}
-	if (conn->c_pending_ops != NULL) {
-		if (conn->c_pending_ops != op || op->o_next != NULL)
-			return(LDAP_OPERATIONS_ERROR);
+	if (( conn->c_ops != NULL &&
+			(conn->c_ops != op || op->o_next != NULL)) ||
+		( conn->c_pending_ops != NULL))
+	{
+		*text = "cannot start TLS when operations our outstanding";
+		rc = LDAP_OPERATIONS_ERROR;
+		goto done;
 	}
 
     conn->c_is_tls = 1;
     conn->c_needs_tls_accept = 1;
 
-    return(LDAP_SUCCESS);
+    rc = LDAP_SUCCESS;
+
+done:
+	/* give up connection lock */
+	ldap_pvt_thread_mutex_lock( &conn->c_mutex );
+
+	/*
+	 * RACE CONDITION: we give up lock before sending result
+	 * Should be resolved by reworking connection state, not
+	 * by moving send here (so as to ensure proper TLS sequencing)
+	 */
+
+	return rc;
 }
 
 #endif	/* HAVE_TLS */