From: Luke Howard Date: Mon, 27 Sep 2010 10:42:40 +0000 (+0200) Subject: Refactor channel binding code X-Git-Url: http://www.project-moonshot.org/gitweb/?p=cyrus-sasl.git;a=commitdiff_plain;h=78e8d1bad2ad9f819fb103b36b7aef60adb899ff Refactor channel binding code --- diff --git a/include/sasl.h b/include/sasl.h index b082057..8c475b6 100755 --- a/include/sasl.h +++ b/include/sasl.h @@ -791,12 +791,13 @@ LIBSASL_API int sasl_getprop(sasl_conn_t *conn, int propnum, * is particularly useful for servers that respond to multiple names. */ #define SASL_GSS_LOCAL_NAME 20 +/* Channel binding information. Memory is managed by the caller. */ typedef struct sasl_channel_binding { - char *type; + const char *name; int critical; unsigned long len; - unsigned char *data; -} sasl_channel_binding; + const unsigned char *data; +} sasl_channel_binding_t; #define SASL_CHANNEL_BINDING 21 diff --git a/include/saslplug.h b/include/saslplug.h index c114cb6..d9547fe 100755 --- a/include/saslplug.h +++ b/include/saslplug.h @@ -193,12 +193,12 @@ typedef struct sasl_out_params { void *client_creds; /* for additions which don't require a version upgrade; set to 0 */ - void *gss_peer_name; - void *gss_local_name; - void *spare_ptr4; + const void *gss_peer_name; + const void *gss_local_name; + const char *cbindingname; /* channel binding name from packet */ int (*spare_fptr1)(); int (*spare_fptr2)(); - int chanbindingflag; + unsigned int cbindingdisp; /* channel binding disposition from client */ int spare_int2; int spare_int3; int spare_int4; @@ -219,7 +219,18 @@ typedef enum { SASL_INFO_LIST_END } sasl_info_callback_stage_t; +/****************************** + * Channel binding macros ** + ******************************/ + +/* TRUE if channel binding is non-NULL */ +#define SASL_CB_DISP_NONE 0x00 /* client did not support CB */ +#define SASL_CB_DISP_USED 0x01 /* client supports CB, thinks server does not */ +#define SASL_CB_DISP_WANT 0x02 /* client supports and used CB */ +#define SASL_CB_PRESENT(params) ((params)->cbinding != NULL) +#define SASL_CB_CRITICAL(params) (SASL_CB_PRESENT(params) && \ + (params)->cbinding->critical) /****************************** * Client Mechanism Functions * @@ -254,9 +265,9 @@ typedef struct sasl_client_params { sasl_ssf_t external_ssf; /* external SSF active */ /* for additions which don't require a version upgrade; set to 0 */ - void *gss_creds; - void *chanbindingtype; - void *chanbindingdata; + const void *gss_creds; /* GSS credential handle */ + const sasl_channel_binding_t *cbinding; /* client channel binding */ + void *spare_ptr3; void *spare_ptr4; /* Canonicalize a user name from on-wire to internal format @@ -289,13 +300,8 @@ typedef struct sasl_client_params { int (*spare_fptr1)(); -#define SASL_CB_FLAG_NONE 0x00 /* client did not support CB */ -#define SASL_CB_FLAG_USED 0x01 /* client supports CB, thinks server does not */ -#define SASL_CB_FLAG_WANT 0x02 /* client supports and used CB */ -#define SASL_CB_FLAG_CRIT 0x10 /* client requires CB */ - int chanbindingflags; -#define SASL_CB_PRESENT(params) ((params)->chanbindingtype != NULL && (params)->chanbindinglen) - int chanbindinglen; + unsigned int cbindingdisp; + unsigned int spare_int2; int spare_int3; /* flags field as passed to sasl_client_new */ @@ -561,14 +567,14 @@ typedef struct sasl_server_params { struct propctx *propctx; /* for additions which don't require a version upgrade; set to 0 */ - void *gss_creds; - void *chanbindingtype; - void *chanbindingdata; + const void *gss_creds; /* GSS credential handle */ + const sasl_channel_binding_t *cbinding; /* server channel binding */ + void *spare_ptr3; void *spare_ptr4; int (*spare_fptr1)(); int (*spare_fptr2)(); - int chanbindinglen; - int chanbindingcrit; + int spare_int1; + int spare_int2; int spare_int3; /* flags field as passed to sasl_server_new */ diff --git a/lib/auxprop.c b/lib/auxprop.c index 1be6c2a..edc3106 100644 --- a/lib/auxprop.c +++ b/lib/auxprop.c @@ -1005,8 +1005,6 @@ _sasl_print_mechanism ( void *rock ) { - char delimiter; - if (stage == SASL_INFO_LIST_START) { printf ("List of auxprop plugins follows\n"); return; diff --git a/lib/client.c b/lib/client.c index 0cff60d..9f3e594 100644 --- a/lib/client.c +++ b/lib/client.c @@ -485,7 +485,8 @@ int sasl_client_start(sasl_conn_t *conn, cmechanism_t *m=NULL,*bestm=NULL; size_t i, list_len; sasl_ssf_t bestssf = 0, minssf = 0; - int result, server_can_cb = 0, cb_flag; + int result, server_can_cb = 0; + unsigned int cbindingdisp = 0; if(_sasl_client_active==0) return SASL_NOTINIT; @@ -520,15 +521,14 @@ int sasl_client_start(sasl_conn_t *conn, goto done; if (SASL_CB_PRESENT(c_conn->cparams)) { - if (server_can_cb == 0 && - (c_conn->cparams->chanbindingflags & SASL_CB_FLAG_CRIT)) { + if (server_can_cb == 0 && SASL_CB_CRITICAL(c_conn->cparams)) { result = SASL_NOMECH; goto done; } else { - cb_flag = SASL_CB_FLAG_WANT; + cbindingdisp = SASL_CB_DISP_WANT; } } else { - cb_flag = SASL_CB_FLAG_NONE; + cbindingdisp = SASL_CB_DISP_NONE; } for (i = 0, name = ordered_mechs; i < list_len; i++) { @@ -616,8 +616,9 @@ int sasl_client_start(sasl_conn_t *conn, } /* Prefer server advertised CB mechanisms */ - if (SASL_CB_PRESENT(c_conn->cparams) && plus) - cb_flag = SASL_CB_FLAG_USED; + if (SASL_CB_PRESENT(c_conn->cparams) && plus) { + cbindingdisp = SASL_CB_DISP_USED; + } if (mech) { *mech = m->m.plug->mech_name; @@ -649,7 +650,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->chanbindingflags |= cb_flag; + c_conn->cparams->cbindingdisp = cbindingdisp; c_conn->mech = bestm; /* init that plugin */ diff --git a/lib/common.c b/lib/common.c index 7827808..0267513 100644 --- a/lib/common.c +++ b/lib/common.c @@ -1209,22 +1209,12 @@ int sasl_setprop(sasl_conn_t *conn, int propnum, const void *value) ((sasl_client_conn_t *)conn)->cparams->gss_creds = (void *)value; break; case SASL_CHANNEL_BINDING: { - struct sasl_channel_binding *cb = (struct sasl_channel_binding *)value; + const struct sasl_channel_binding *cb = (const struct sasl_channel_binding *)value; - if (conn->type == SASL_CONN_SERVER) { - ((sasl_server_conn_t *)conn)->sparams->chanbindingtype = cb->type; - ((sasl_server_conn_t *)conn)->sparams->chanbindingdata = cb->data; - ((sasl_server_conn_t *)conn)->sparams->chanbindinglen = cb->len; - ((sasl_server_conn_t *)conn)->sparams->chanbindingcrit = cb->critical; - } else { - ((sasl_client_conn_t *)conn)->cparams->chanbindingtype = cb->type; - ((sasl_client_conn_t *)conn)->cparams->chanbindingdata = cb->data; - ((sasl_client_conn_t *)conn)->cparams->chanbindinglen = cb->len; - if (cb->critical != 0) - ((sasl_client_conn_t *)conn)->cparams->chanbindingflags |= SASL_CB_FLAG_CRIT; - else - ((sasl_client_conn_t *)conn)->cparams->chanbindingflags &= ~(SASL_CB_FLAG_CRIT); - } + if (conn->type == SASL_CONN_SERVER) + ((sasl_server_conn_t *)conn)->sparams->cbinding = cb; + else + ((sasl_client_conn_t *)conn)->cparams->cbinding = cb; break; } default: diff --git a/lib/server.c b/lib/server.c index f4a50bd..a34bccb 100644 --- a/lib/server.c +++ b/lib/server.c @@ -1454,17 +1454,38 @@ int sasl_server_step(sasl_conn_t *conn, } /* Validate channel bindings */ - if (conn->oparams.chanbindingflag == SASL_CB_FLAG_NONE && - s_conn->sparams->chanbindingcrit) { - sasl_seterror(conn, 0, - "server requires channel binding but client provided none"); - ret = SASL_BADAUTH; - } else if (conn->oparams.chanbindingflag == SASL_CB_FLAG_WANT && - SASL_CB_PRESENT(s_conn->sparams)) { - sasl_seterror(conn, 0, - "client incorrectly assumed server had no channel binding"); - ret = SASL_BADAUTH; - } else if (conn->oparams.user == NULL || conn->oparams.authid == NULL) { + switch (conn->oparams.cbindingdisp) { + case SASL_CB_DISP_NONE: + if (SASL_CB_CRITICAL(s_conn->sparams)) { + sasl_seterror(conn, 0, + "server requires channel binding but client provided none"); + ret = SASL_BADAUTH; + } + break; + case SASL_CB_DISP_WANT: + if (SASL_CB_PRESENT(s_conn->sparams)) { + sasl_seterror(conn, 0, + "client incorrectly assumed server had no channel binding"); + ret = SASL_BADAUTH; + } + break; + case SASL_CB_DISP_USED: + if (!SASL_CB_PRESENT(s_conn->sparams)) { + sasl_seterror(conn, 0, + "client provided channel binding but server had none"); + ret = SASL_BADAUTH; + } else if (strcmp(conn->oparams.cbindingname, + s_conn->sparams->cbinding->name) != 0) { + sasl_seterror(conn, 0, + "client channel binding %s does not match server %s", + conn->oparams.cbindingname, s_conn->sparams->cbinding->name); + ret = SASL_BADAUTH; + } + break; + } + + if (ret == SASL_OK && + (conn->oparams.user == NULL || conn->oparams.authid == NULL)) { sasl_seterror(conn, 0, "mech did not call canon_user for both authzid " \ "and authid"); @@ -1572,7 +1593,8 @@ int _sasl_server_listmech(sasl_conn_t *conn, * he non-PLUS-variant due to policy reasons, it MUST advertise * only the PLUS-variant. */ - if (!s_conn->sparams->chanbindingcrit) { + if (!SASL_CB_PRESENT(s_conn->sparams) || + !SASL_CB_CRITICAL(s_conn->sparams)) { if (pcount != NULL) (*pcount)++; if (flag) diff --git a/sample/client.c b/sample/client.c index ec0514f..c6e5b98 100644 --- a/sample/client.c +++ b/sample/client.c @@ -342,7 +342,7 @@ int main(int argc, char *argv[]) int niflags, error; struct sockaddr_storage local_ip, remote_ip; int cb_flag = 0; - sasl_channel_binding cb; + sasl_channel_binding_t cb; while ((c = getopt(argc, argv, "cp:s:m:")) != EOF) { switch(c) { @@ -426,7 +426,7 @@ int main(int argc, char *argv[]) if (r != SASL_OK) saslfail(r, "allocating connection state"); if (cb_flag) { - cb.type = "sasl-sample"; + cb.name = "sasl-sample"; cb.data = "this is a test of channel binding"; cb.len = strlen(cb.data); diff --git a/sample/server.c b/sample/server.c index fec6355..20b6bd0 100644 --- a/sample/server.c +++ b/sample/server.c @@ -373,7 +373,7 @@ int main(int argc, char *argv[]) int nfds, fd = -1; FILE *in, *out; fd_set readfds; - sasl_channel_binding cb; + sasl_channel_binding_t cb; FD_ZERO(&readfds); for (i = 1; i <= l[0]; i++) @@ -445,7 +445,7 @@ int main(int argc, char *argv[]) NULL, 0, &conn); if (r != SASL_OK) saslfail(r, "allocating connection state"); - cb.type = "sasl-sample"; + cb.name = "sasl-sample"; cb.critical = cb_flag; cb.data = "this is a test of channel binding"; cb.len = strlen(cb.data);