Fixed handling of channel bindings on the client side

[cyrus-sasl.git] / lib / client.c

diff --git a/lib/client.c b/lib/client.c
index d4b37f0..1c5374a 100644 (file)
--- a/lib/client.c
+++ b/lib/client.c
@@ -456,11 +456,49 @@ _sasl_client_order_mechs(const sasl_utils_t *utils,
            break;
     } while (1);
 
+    if (*count == 0) {
+        utils->free(list);
+        return SASL_NOMECH;
+    }
+
     *ordered_mechs = list;
 
     return SASL_OK;
 }
 
+static inline int
+_sasl_cbinding_disp(sasl_client_params_t *cparams,
+                    int mech_nego,
+                    int server_can_cb,
+                    sasl_cbinding_disp_t *cbindingdisp)
+{
+    /*
+     * If negotiating mechanisms, then we fail immediately if the
+     * client requires channel binding and the server does not
+     * advertise support. Otherwise we send "y" (which later will
+     * become "p" if we select a supporting mechanism).
+     *
+     * If the client explicitly selected a mechanism, then we only
+     * send channel bindings if they're marked critical.
+     */
+
+    *cbindingdisp = SASL_CB_DISP_NONE;
+
+    if (SASL_CB_PRESENT(cparams)) {
+        if (mech_nego) {
+            if (!server_can_cb && SASL_CB_CRITICAL(cparams)) {
+               return SASL_NOMECH;
+            } else {
+                *cbindingdisp = SASL_CB_DISP_WANT;
+            }
+        } else if (SASL_CB_CRITICAL(cparams)) {
+            *cbindingdisp = SASL_CB_DISP_USED;
+        }
+    }
+
+    return SASL_OK;
+}
+
 /* select a mechanism for a connection
  *  mechlist      -- mechanisms server has available (punctuation ignored)
  *  secret        -- optional secret from previous session
@@ -485,11 +523,13 @@ int sasl_client_start(sasl_conn_t *conn,
 {
     sasl_client_conn_t *c_conn= (sasl_client_conn_t *) conn;
     char *ordered_mechs = NULL, *name;
-    cmechanism_t *m=NULL,*bestm=NULL;
+    cmechanism_t *m = NULL, *bestm = NULL;
     size_t i, list_len;
     sasl_ssf_t bestssf = 0, minssf = 0;
     int result, server_can_cb = 0;
-    unsigned int cbindingdisp;
+    sasl_cbinding_disp_t cbindingdisp;
+    sasl_cbinding_disp_t cur_cbindingdisp;
+    sasl_cbinding_disp_t best_cbindingdisp = SASL_CB_DISP_NONE;
 
     if(_sasl_client_active==0) return SASL_NOTINIT;
 
@@ -514,6 +554,7 @@ int sasl_client_start(sasl_conn_t *conn,
        minssf = conn->props.min_ssf - conn->external.ssf;
     }
 
+    /* Order mechanisms so -PLUS are preferred */
     result = _sasl_client_order_mechs(c_conn->cparams->utils,
                                      mechlist,
                                      SASL_CB_PRESENT(c_conn->cparams),
@@ -523,24 +564,28 @@ int sasl_client_start(sasl_conn_t *conn,
     if (result != 0)
        goto done;
 
-    if (SASL_CB_PRESENT(c_conn->cparams)) {
-       if (server_can_cb == 0 && SASL_CB_CRITICAL(c_conn->cparams)) {
-           result = SASL_BADBINDING;
-           goto done;
-       } else {
-           cbindingdisp = SASL_CB_DISP_WANT;
-       }
-    } else {
-       cbindingdisp = SASL_CB_DISP_NONE;
-    }
+    /*
+     * Determine channel binding disposition based on whether we
+     * are doing mechanism negotiation and whether server supports
+     * channel bindings.
+     */
+    result = _sasl_cbinding_disp(c_conn->cparams,
+                                 (list_len > 1),
+                                 server_can_cb,
+                                 &cbindingdisp);
+    if (result != 0)
+       goto done;
 
     for (i = 0, name = ordered_mechs; i < list_len; i++) {
+
        /* foreach in client list */
        for (m = cmechlist->mech_list; m != NULL; m = m->next) {
            int myflags, plus;
 
-           if (!_sasl_is_equal_mech(name, m->m.plug->mech_name, &plus))
+           if (!_sasl_is_equal_mech(name, m->m.plug->mech_name,
+                                     strlen(m->m.plug->mech_name), &plus)) {
                continue;
+            }
 
            /* Do we have the prompts for it? */
            if (!have_prompts(conn, m->m.plug))
@@ -564,7 +609,7 @@ int sasl_client_start(sasl_conn_t *conn,
            }
 
            /* Can we meet it's features? */
-          if (SASL_CB_PRESENT(c_conn->cparams) &&
+           if (cbindingdisp == SASL_CB_DISP_USED &&
                !(m->m.plug->features & SASL_FEAT_CHANNEL_BINDING)) {
                break;
            }
@@ -580,20 +625,6 @@ int sasl_client_start(sasl_conn_t *conn,
                break;
            }
 
-#ifdef PREFER_MECH
-           if (strcasecmp(m->m.plug->mech_name, PREFER_MECH) &&
-               bestm && m->m.plug->max_ssf <= bestssf) {
-               /* this mechanism isn't our favorite, and it's no better
-                  than what we already have! */
-               break;
-           }
-#else
-           if (bestm && m->m.plug->max_ssf <= bestssf) {
-               /* this mechanism is no better than what we already have! */
-               break;
-           }
-#endif
-
            /* compare security flags, only take new mechanism if it has
             * all the security flags of the previous one.
             *
@@ -618,14 +649,35 @@ int sasl_client_start(sasl_conn_t *conn,
                break;
            }
 
-           /* Prefer server advertised CB mechanisms */
            if (SASL_CB_PRESENT(c_conn->cparams) && plus) {
-               cbindingdisp = SASL_CB_DISP_USED;
+               cur_cbindingdisp = SASL_CB_DISP_USED;
+           } else {
+               cur_cbindingdisp = cbindingdisp;
+            }
+
+            if (bestm && (best_cbindingdisp > cur_cbindingdisp)) {
+                break;
+            }
+
+#ifdef PREFER_MECH
+           if (strcasecmp(m->m.plug->mech_name, PREFER_MECH) &&
+               bestm && m->m.plug->max_ssf <= bestssf) {
+               /* this mechanism isn't our favorite, and it's no better
+                  than what we already have! */
+               break;
+           }
+#else
+           if (bestm && m->m.plug->max_ssf <= bestssf) {
+               /* this mechanism is no better than what we already have! */
+               break;
            }
+#endif
 
            if (mech) {
                *mech = m->m.plug->mech_name;
            }
+
+            best_cbindingdisp = cur_cbindingdisp;
            bestssf = m->m.plug->max_ssf;
            bestm = m;
            break;
@@ -653,7 +705,7 @@ int sasl_client_start(sasl_conn_t *conn,
 
     c_conn->cparams->external_ssf = conn->external.ssf;
     c_conn->cparams->props = conn->props;
-    c_conn->cparams->cbindingdisp = cbindingdisp;
+    c_conn->cparams->cbindingdisp = best_cbindingdisp;
     c_conn->mech = bestm;
 
     /* init that plugin */