Re: (ITS#6828) TLS fails to start when LDAP_OPT_CONNECT_ASYNC is used

[hyc@symas.com]

ipuleston@SonicWALL.com wrote:
> The patch that I submitted above has a memory leak due not freeing "ber"
when returning error LDAP_X_CONNECTING from function
ldap_send_initial_request(). I have uploaded a new version of the patch with
this fixed to ftp.openldap.org/incoming as ian-puleston-110412.patch.2 (note
the .2 on the end - it was a 2nd attempt after I forgot to specify ASCII mode).

Unfortunately the patch was made against 2.4.23 and not against HEAD. It does 
not apply cleanly to current git. Patches must always be submitted against the 
development code, not against released code.

I'm pretty sure that this chunk can never do anything useful.

diff -u libldap.orig/request.c libldap/request.c
--- libldap.orig/request.c	2010-06-10 10:39:48.000000000 -0700
+++ libldap/request.c	2011-04-12 12:11:20.769166100 -0700
@@ -446,7 +462,21 @@
  		lc->lconn_server = ldap_url_dup( srv );
  	}

-	lc->lconn_status = async ? LDAP_CONNST_CONNECTING : LDAP_CONNST_CONNECTED;
+	if ( async ) {
+#ifdef HAVE_TLS
+		void *ssl = NULL;
+		ber_sockbuf_ctrl( lc->lconn_sb, LBER_SB_OPT_GET_SSL, (void *)&ssl );
+		if ( ssl ) {
+			/* the connect finished and tls was started */
+			lc->lconn_status = LDAP_CONNST_CONNECTED;
+		} else
+#endif
+		{
+			lc->lconn_status = LDAP_CONNST_CONNECTING;
+		}
+	} else {
+		lc->lconn_status = LDAP_CONNST_CONNECTED;
+	}
  #ifdef LDAP_R_COMPILE
  	ldap_pvt_thread_mutex_lock( &ld->ld_conn_mutex );
  #endif

The connection has just been created, asynchronously, so there's no way that 
the TLS layer was already started when it got here. Also, even if this was 
valid, it obviously would only apply for OPT_X_TLS_HARD || ldaps, but you 
haven't checked that condition here. Ultimately it appears that this test is a 
no-op.

Also, I suggest that you only check for CONNST_CONNECTING in the callers, and 
do the TLS check in the check function, like this:

+int
+ldap_int_check_async_open( LDAP *ld, LDAPConn *lc, LDAPURLDesc *srv, 
ber_socket_t sd )
+{
+       struct timeval tv = { 0 };
+       int rc;
+
+       rc = ldap_int_poll( ld, sd, &tv );
+       switch ( rc ) {
+       case 0:
+               /* now ready to start tls */
+               lc->lconn_status = LDAP_CONNST_CONNECTED;
+               break;
+
+       default:
+               rc = -1;
+		return rc;
+       case -2:
+               /* connect not completed yet */
+		ld->ld_errno = LDAP_X_CONNECTING;
+               return rc;
+       }
+
+#ifdef HAVE_TLS
+       if ( ld->ld_options.ldo_tls_mode == LDAP_OPT_X_TLS_HARD ||
+               !strcmp( srv->lud_scheme, "ldaps" )) {
+
+               ++lc->lconn_refcnt;     /* avoid premature free */
+
+               rc = ldap_int_tls_start( ld, lc, srv );
+
+               --lc->lconn_refcnt;
+       }
+#endif
+       return rc;
+}

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/