Hi Howard,
-----Original Message----- From: Howard Chu [mailto:hyc@symas.com]
Ian Puleston wrote:
I'm working on a fix now, and I think what is needed is:
- A call to ldap_int_poll in ldap_int_tls_start if async. Then it
should abort without calling ldap_int_tls_connect if not ready (probably return LDAP_X_CONNECTING).
- In ldap_send_initial_request don't call ldap_send_server_request
if lconn_status is LDAP_CONNST_CONNECTING and using TLS (since TLS needs to be started first). Instead return LDAP_X_CONNECTING.
- Probably something similar to #1 for the case where
ldap_start_tls_s is called from ldap_new_connection.
If this all works I'll put in a bug report with a patch.
Sounds to me like you should just file the bug report first. The patch can come later.
I need to get this fixed in the port I am using so I will do that and attach the patch to the bug report. The relevant openldap developers can then choose whether to use that patch as-is or work out something different, and if the latter I can replace the fix in my port with it when it gets released.
Also I don't believe the proper fix should touch so many places. All you should need is to fix the caller of ldap_int_tls_connect(), and as that's a static function there is only one place to fix.
ldap_int_open_connection calls ldap_connect_to_host and then ldap_start_tls_s, so to prevent the latter calling ldap_int_connect after ldap_connect_to_host has returned -2 it will have to be in either ldap_int_open_connection or ldap_start_tls_s. But ldap_int_open_connection probably is a better place than in ldap_int_tls_start.
But we then still have to stop ldap_send_server_request getting called since tls has not started, so that will require a change in ldap_send_initial_request too.
Below is a diagram of the calls involved that I put together when analyzing what was happening here (hopefully email won't munge it too badly). You can see from this that there is no one place that could be modified to prevent both ldap_int_tls_connect and ldap_send_server_request getting called after ldap_pvt_connect has returned -2:
ldap_sasl_bind -> ldap_send_initial_request |-> ldap_open_defconn | -> ldap_new_connection | |-> ldap_int_open_connection | | |-> ldap_connect_to_host | | | -> ldap_pvt_connect | | | -> connect | | | | | -> ldap_int_tls_start | | -> ldap_int_tls_connect | | | |- lconn_status -> LDAP_CONNST_CONNECTING if async. | | | -> ldap_start_tls_s (only if start-tls cmd is to be sent!) | | -> ldap_send_server_request -> ldap_int_poll
For a non-blocking async connect:
- ldap_pvt_connect retrns -2 if EINPROGRESS || EWOULDBLOCK. - ldap_connect_to_host returns the rc returned by ldap_pvt_connect. - ldap_int_open_connection then continues as per success & returns 0. - ldap_new_connection always sets the status to LDAP_CONNST_CONNECTING if async. - ldap_send_server_request returns LDAP_X_CONNECTING if the poll returns -2.