Fixes to SASL interactive bind, as posted to the list by Isaac Boukris

diff --git a/raddb/mods-available/ldap b/raddb/mods-available/ldap
index 7167064..3d58baa 100644 (file)
--- a/raddb/mods-available/ldap
+++ b/raddb/mods-available/ldap
@@ -24,6 +24,7 @@ ldap {
 #      port = 389
 
        #  Administrator account for searching and possibly modifying.
+       #  If using SASL + KRB5 these should be commented out.
 #      identity = 'cn=admin,dc=example,dc=org'
 #      password = mypass
 
@@ -31,10 +32,25 @@ ldap {
        #  searches will start from.
        base_dn = 'dc=example,dc=org'
 
+       #
        #  SASL parameters to use for admin binds
        #
        #  When we're prompted by the SASL library, these control
-       #  the responses given.
+       #  the responses given, as well as the identity and password
+       #  directives above.
+       #
+       #  If any directive is commented out, a NULL response will be
+       #  provided to cyrus-sasl.
+       #
+       #  Unfortunately the only way to control Keberos here is through
+       #  environmental variables, as cyrus-sasl provides no API to
+       #  set the krb5 config directly.
+       #
+       #  Full documentation for MIT krb5 can be found here:
+       #
+       #       http://web.mit.edu/kerberos/krb5-devel/doc/admin/env_variables.html
+       #
+       #  At a minimum you probably want to set KRB5_CLIENT_KTNAME.
        #
        sasl {
                # SASL mechanism
@@ -256,7 +272,7 @@ ldap {
 
                #  Override the normal cache attribute (<inst>-LDAP-Group or
                #  LDAP-Group if using the default instance) and create a
-               #  custom attribute.  This can help if multiple module instances 
+               #  custom attribute.  This can help if multiple module instances
                #  are used in fail-over.
 #              cache_attribute = 'LDAP-Cached-Membership'
        }

diff --git a/src/modules/rlm_ldap/ldap.c b/src/modules/rlm_ldap/ldap.c
index 416a122..7cf8cae 100644 (file)
--- a/src/modules/rlm_ldap/ldap.c
+++ b/src/modules/rlm_ldap/ldap.c
@@ -717,7 +717,7 @@ ldap_rcode_t rlm_ldap_bind(rlm_ldap_t const *inst, REQUEST *request, ldap_handle
        num = retry ? fr_connection_get_num(inst->pool) : 0;
        for (i = num; i >= 0; i--) {
 #ifdef HAVE_LDAP_SASL_INTERACTIVE_BIND
-               if (sasl->mech) {
+               if (sasl && sasl->mech) {
                        status = rlm_ldap_sasl_interactive(inst, request, *pconn, dn, password, sasl,
                                                           &error, &extra);
                } else

diff --git a/src/modules/rlm_ldap/ldap.h b/src/modules/rlm_ldap/ldap.h
index bcd0219..d77a19d 100644 (file)
--- a/src/modules/rlm_ldap/ldap.h
+++ b/src/modules/rlm_ldap/ldap.h
@@ -465,5 +465,5 @@ char const *edir_errstr(int code);
 ldap_rcode_t rlm_ldap_sasl_interactive(rlm_ldap_t const *inst, REQUEST *request,
                                       ldap_handle_t *pconn, char const *dn,
                                       char const *password, ldap_sasl *sasl,
-                                      char const **error, char **extra);
+                                      char const **error, char **error_extra);
 #endif

diff --git a/src/modules/rlm_ldap/sasl.c b/src/modules/rlm_ldap/sasl.c
index 0e58bba..159dbeb 100644 (file)
--- a/src/modules/rlm_ldap/sasl.c
+++ b/src/modules/rlm_ldap/sasl.c
@@ -31,8 +31,11 @@
 
 #include <sasl/sasl.h>
 
+/** Data passed to the _sasl interact callback.
+ *
+ */
 typedef struct rlm_ldap_sasl_ctx {
-       rlm_ldap_t      const   *inst;          //!< LDAP instance
+       rlm_ldap_t const        *inst;          //!< LDAP instance
        REQUEST                 *request;       //!< The current request.
 
        char const              *identity;      //!< User's DN or identity.
@@ -41,43 +44,62 @@ typedef struct rlm_ldap_sasl_ctx {
        ldap_sasl               *extra;         //!< Extra fields (realm and proxy id).
 } rlm_ldap_sasl_ctx_t;
 
+/** Callback for ldap_sasl_interactive_bind
+ *
+ * @param handle used for the SASL bind.
+ * @param flags data as provided to ldap_sasl_interactive_bind.
+ * @param ctx Our context data, containing the identity, password, realm and various other things.
+ * @param array of callbacks to provide responses for.
+ * @return SASL_OK.
+ */
 static int _sasl_interact(UNUSED LDAP *handle, UNUSED unsigned flags, void *ctx, void *sasl_callbacks)
 {
-       rlm_ldap_sasl_ctx_t *this = ctx;
-       REQUEST *request = this->request;
-       rlm_ldap_t const *inst = this->inst;
-       sasl_interact_t *cb = sasl_callbacks;
-       sasl_interact_t *cb_p;
+       rlm_ldap_sasl_ctx_t     *this = ctx;
+       REQUEST                 *request = this->request;
+       rlm_ldap_t const        *inst = this->inst;
+       sasl_interact_t         *cb = sasl_callbacks;
+       sasl_interact_t         *cb_p;
 
        for (cb_p = cb; cb_p->id != SASL_CB_LIST_END; cb_p++) {
                MOD_ROPTIONAL(RDEBUG3, DEBUG3, "SASL challenge : %s", cb_p->challenge);
                MOD_ROPTIONAL(RDEBUG3, DEBUG3, "SASL prompt    : %s", cb_p->prompt);
 
                switch (cb_p->id) {
-                       case SASL_CB_AUTHNAME:
-                               cb_p->result = this->identity;
-                               break;
+               case SASL_CB_AUTHNAME:
+                       cb_p->result = this->identity;
+                       break;
 
-                       case SASL_CB_PASS:
-                               cb_p->result = this->password;
-                               break;
+               case SASL_CB_PASS:
+                       cb_p->result = this->password;
+                       break;
 
-                       case SASL_CB_USER:
-                               cb_p->result = this->extra->proxy ? this->extra->proxy : this->identity;
-                               break;
+               case SASL_CB_USER:
+                       cb_p->result = this->extra->proxy ? this->extra->proxy : this->identity;
+                       break;
 
-                       case SASL_CB_GETREALM:
-                               if (this->extra->realm) cb_p->result = this->extra->realm;
-                               break;
+               case SASL_CB_GETREALM:
+                       if (this->extra->realm) cb_p->result = this->extra->realm;
+                       break;
 
-                       default:
-                               break;
+               default:
+                       break;
                }
                MOD_ROPTIONAL(RDEBUG3, DEBUG3, "SASL result    : %s", cb_p->result ? (char const *)cb_p->result : "");
        }
        return SASL_OK;
 }
 
+/** Initiate an LDAP interactive bind
+ *
+ * @param[in] inst rlm_ldap configuration.
+ * @param[in] request Current request, this may be NULL, in which case all debug logging is done with radlog.
+ * @param[in] conn to use. May change as this function calls functions which auto re-connect.
+ * @param[in] identity of the use.
+ * @param[in] sasl mechanism to use for bind, and additional parameters.
+ * @param[out] error message resulting from bind.
+ * @param[out] extra information about the error.
+ * @return One of the LDAP_PROC_* (#ldap_rcode_t) values.
+ */
 ldap_rcode_t rlm_ldap_sasl_interactive(rlm_ldap_t const *inst, REQUEST *request,
                                       ldap_handle_t *conn, char const *identity,
                                       char const *password, ldap_sasl *sasl,
@@ -90,12 +112,11 @@ ldap_rcode_t rlm_ldap_sasl_interactive(rlm_ldap_t const *inst, REQUEST *request,
        LDAPMessage             *result = NULL;
        rlm_ldap_sasl_ctx_t     sasl_ctx;               /* SASL defaults */
 
-       memset(&sasl_ctx, 0, sizeof(sasl_ctx));
-
        sasl_ctx.inst = inst;
        sasl_ctx.request = request;
        sasl_ctx.identity = identity;
        sasl_ctx.password = password;
+       sasl_ctx.extra = sasl;
 
        MOD_ROPTIONAL(RDEBUG2, DEBUG2, "Starting SASL mech(s): %s", sasl->mech);
        do {