On Wed, Oct 26, 2011 at 05:10:41PM -0700, Quanah Gibson-Mount wrote:

> >I'm using simple binds as I have since postfix 2.3.  I actually was not
> >aware the code for using SASL mechanism binds had been added to postfix.
> >Very happy to know that. ;)  I have my own test server set up now so I
> >can better get the information you're asking for.
> >
> >First, a normal postmap -q when the user exists and it has a working
> >password:
> 
> I will note that I build with -DUSE_LDAP_SASL and use OpenLDAP as
> the API. It *looks* like this should use the old dict_ldap_bind_st
> function, but since there is no logging in it or dict_ldap_bind_sasl
> specifically noting which is used, I can't be 100% sure.

I hate people who document interface semantics poorly and then have
the gall to change said undocumented sematics!

Someone "clever" decided to change the implementation of
ldap_parse_sasl_bind_result() between OpenLDAP 2.3 and OpenLDAP
2.4. The 2.3 version returned an error when the bind failed. The
2.4 version sets ld->ld_error as a side-effect and returns
LDAP_SUCCESS.  Apparently, the routine now just tells you whether
the result was decoded successfully, and the the decoded failure
status has to be determined by the caller.

I think this change was unwise. That said, there appears to be
a related issue in the Postfix code that should have the issue
in ldap_result() rather than in ldap_parse_sasl_bind_result().

Therefore, I propose the following Postfix fix/work-around which
is required for anyone running Postfix 2.3 or later, linked with
OpenLDAP 2.4 or later (perhaps even late 2.3.x releases, I just
compared OpenLDAP 2.3.4 with 2.4.23).

By the way, switching to "bind = sasl" may solve the problem for
you, if "sasl_mechs = plain" is compatible with "simple" binds in
your server. The new SASL code leaves more of the error handling
to the LDAP library, since the state-machine for SASL is not exposed
by OpenLDAP to applications to handle timeouts, and getting timeouts
right in this case would in any case probably not justify the extra
code, since we could still block in Kerberos libraries, ... (it
is turtles all the way down!).

Index: src/global/dict_ldap.c
--- src/global/dict_ldap.c      1 Mar 2011 04:44:42 -0000       1.1.1.2
+++ src/global/dict_ldap.c      27 Oct 2011 04:04:11 -0000
@@ -498,6 +498,7 @@
 static int dict_ldap_result(LDAP *ld, int msgid, int timeout, LDAPMessage 
**res)
 {
     struct timeval mytimeval;
+    int err;
 
     mytimeval.tv_sec = timeout;
     mytimeval.tv_usec = 0;
@@ -506,10 +507,14 @@
     if (ldap_result(ld, msgid, GET_ALL, &mytimeval, res) == -1)
        return (dict_ldap_get_errno(ld));
 
-    if (dict_ldap_get_errno(ld) == LDAP_TIMEOUT) {
-       (void) dict_ldap_abandon(ld, msgid);
-       return (dict_ldap_set_errno(ld, LDAP_TIMEOUT));
+    if ((err = dict_ldap_get_errno(ld)) != LDAP_SUCCESS) {
+       if (err == LDAP_TIMEOUT) {
+           (void) dict_ldap_abandon(ld, msgid);
+           return (dict_ldap_set_errno(ld, LDAP_TIMEOUT));
+       }
+       return err;
     }
+
     return LDAP_SUCCESS;
 }
 

-- 
        Viktor.

Reply via email to