From: Luke Howard Date: Tue, 2 Apr 2013 05:48:02 +0000 (+1100) Subject: Chbind cleanups X-Git-Tag: 0.9.2~43^2 X-Git-Url: http://www.project-moonshot.org/gitweb/?p=mech_eap.git;a=commitdiff_plain;h=refs%2Fheads%2Feap-chbind Chbind cleanups * indentation * don't use non-booleans as truth values * consistent cleanup handling * improved variable names --- diff --git a/mech_eap/init_sec_context.c b/mech_eap/init_sec_context.c index 2e4d3f3..6a8de69 100644 --- a/mech_eap/init_sec_context.c +++ b/mech_eap/init_sec_context.c @@ -197,10 +197,10 @@ static struct eapol_callbacks gssEapPolicyCallbacks = { extern int wpa_debug_level; #endif -#define CHBIND_SERVICE_NAME_FLAG 0x01 -#define CHBIND_HOST_NAME_FLAG 0x02 -#define CHBIND_SERVICE_SPECIFIC_FLAG 0x04 -#define CHBIND_REALM_NAME_FLAG 0x08 +#define CHBIND_SERVICE_NAME_FLAG 0x01 +#define CHBIND_HOST_NAME_FLAG 0x02 +#define CHBIND_SERVICE_SPECIFIC_FLAG 0x04 +#define CHBIND_REALM_NAME_FLAG 0x08 extern void TestFunc(); @@ -208,18 +208,17 @@ static OM_uint32 peerInitEapChannelBinding(OM_uint32 *minor, gss_ctx_id_t ctx) { struct wpabuf *buf = NULL; - unsigned int requested = 0; - krb5_principal princ; - gss_buffer_desc nameBuf; + unsigned int chbindReqFlags = 0; + krb5_principal princ = NULL; + gss_buffer_desc nameBuf = GSS_C_EMPTY_BUFFER; OM_uint32 major = GSS_S_COMPLETE; krb5_context krbContext = NULL; - /* must have acceptor name, but already checked in - * eapGssSmInitAcceptorName(), so maybe redunadant - * to do so here as well? */ - if (!ctx->acceptorName) { + /* XXX is this check redundant? */ + if (ctx->acceptorName == GSS_C_NO_NAME) { + major = GSS_S_BAD_NAME; *minor = GSSEAP_NO_ACCEPTOR_NAME; - return GSS_S_BAD_NAME; + goto cleanup; } princ = ctx->acceptorName->krbPrincipal; @@ -229,8 +228,9 @@ peerInitEapChannelBinding(OM_uint32 *minor, gss_ctx_id_t ctx) major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_SERVICE_NAME, 0, &nameBuf); if (GSS_ERROR(major)) - goto init_chbind_cleanup; - requested |= CHBIND_SERVICE_NAME_FLAG; + goto cleanup; + + chbindReqFlags |= CHBIND_SERVICE_NAME_FLAG; } krbPrincComponentToGssBuffer(princ, 1, &nameBuf); @@ -238,45 +238,55 @@ peerInitEapChannelBinding(OM_uint32 *minor, gss_ctx_id_t ctx) major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_HOST_NAME, 0, &nameBuf); if (GSS_ERROR(major)) - goto init_chbind_cleanup; - requested |= CHBIND_HOST_NAME_FLAG; + goto cleanup; + + chbindReqFlags |= CHBIND_HOST_NAME_FLAG; } GSSEAP_KRB_INIT(&krbContext); + *minor = krbPrincUnparseServiceSpecifics(krbContext, princ, &nameBuf); - if (*minor) - goto init_chbind_cleanup; + if (*minor != 0) + goto cleanup; if (nameBuf.length > 0) { major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_SERVICE_SPECIFICS, 0, &nameBuf); - if (GSS_ERROR(major)) { - krbFreeUnparsedName(krbContext, &nameBuf); - goto init_chbind_cleanup; - } - requested |= CHBIND_SERVICE_SPECIFIC_FLAG; + if (GSS_ERROR(major)) + goto cleanup; + + chbindReqFlags |= CHBIND_SERVICE_SPECIFIC_FLAG; } - krbFreeUnparsedName(krbContext, &nameBuf); + krbFreeUnparsedName(krbContext, &nameBuf); krbPrincRealmToGssBuffer(princ, &nameBuf); + if (nameBuf.length > 0) { major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_REALM_NAME, 0, &nameBuf); - requested |= CHBIND_REALM_NAME_FLAG; + chbindReqFlags |= CHBIND_REALM_NAME_FLAG; } - if (requested==0) { - wpabuf_free(buf); + if (chbindReqFlags == 0) { + major = GSS_S_BAD_NAME; *minor = GSSEAP_BAD_ACCEPTOR_NAME; - return GSS_S_BAD_NAME; + goto cleanup; } + ctx->initiatorCtx.chbindData = buf; - ctx->initiatorCtx.chbindReqFlags = requested; + ctx->initiatorCtx.chbindReqFlags = chbindReqFlags; + buf = NULL; -init_chbind_cleanup: + + major = GSS_S_COMPLETE; + *minor = 0; + +cleanup: + krbFreeUnparsedName(krbContext, &nameBuf); wpabuf_free(buf); + return major; } @@ -284,46 +294,46 @@ static void peerProcessChbindResponse(void *context, int code, int nsid, u8 *data, size_t len) { - radius_parser msg; + radius_parser msg; gss_ctx_id_t ctx = (gss_ctx_id_t )context; void *vsadata; u8 type; u32 vendor_id; - u32 accepted = 0; + u32 chbindRetFlags = 0; size_t vsadata_len; if (nsid != CHBIND_NSID_RADIUS) return; + msg = radius_parser_start(data, len); - if (!msg) + if (msg == NULL) return; + while (radius_parser_parse_tlv(msg, &type, &vendor_id, &vsadata, &vsadata_len) == 0) { - - switch (type) { - case PW_GSS_ACCEPTOR_SERVICE_NAME: - accepted |= CHBIND_SERVICE_NAME_FLAG; - break; - case PW_GSS_ACCEPTOR_HOST_NAME: - accepted |= CHBIND_HOST_NAME_FLAG; - break; - case PW_GSS_ACCEPTOR_SERVICE_SPECIFICS: - accepted |= CHBIND_SERVICE_SPECIFIC_FLAG; - break; - case PW_GSS_ACCEPTOR_REALM_NAME: - accepted |= CHBIND_REALM_NAME_FLAG; - break; - } + switch (type) { + case PW_GSS_ACCEPTOR_SERVICE_NAME: + chbindRetFlags |= CHBIND_SERVICE_NAME_FLAG; + break; + case PW_GSS_ACCEPTOR_HOST_NAME: + chbindRetFlags |= CHBIND_HOST_NAME_FLAG; + break; + case PW_GSS_ACCEPTOR_SERVICE_SPECIFICS: + chbindRetFlags |= CHBIND_SERVICE_SPECIFIC_FLAG; + break; + case PW_GSS_ACCEPTOR_REALM_NAME: + chbindRetFlags |= CHBIND_REALM_NAME_FLAG; + break; + } } + radius_parser_finish(msg); - if ((code == CHBIND_CODE_SUCCESS) && - ((accepted & ctx->initiatorCtx.chbindReqFlags) == ctx->initiatorCtx.chbindReqFlags)) { + + if (code == CHBIND_CODE_SUCCESS && + ((chbindRetFlags & ctx->initiatorCtx.chbindReqFlags) == ctx->initiatorCtx.chbindReqFlags)) { ctx->flags |= CTX_FLAG_EAP_CHBIND_ACCEPT; ctx->gssFlags |= GSS_C_MUTUAL_FLAG; - /* Accepted! */ - } else { - /* log failures? */ - } + } /* else log failures? */ } static OM_uint32 @@ -391,11 +401,9 @@ peerConfigInit(OM_uint32 *minor, gss_ctx_id_t ctx) eapPeerConfig->altsubject_match = (unsigned char *)cred->subjectAltNameConstraint.value; /* eap channel binding */ - if (ctx->initiatorCtx.chbindData) - { + if (ctx->initiatorCtx.chbindData != NULL) { struct eap_peer_chbind_config *chbind_config = - (struct eap_peer_chbind_config *) - GSSEAP_MALLOC(sizeof(struct eap_peer_chbind_config)); + (struct eap_peer_chbind_config *)GSSEAP_MALLOC(sizeof(struct eap_peer_chbind_config)); if (chbind_config == NULL) { *minor = ENOMEM; return GSS_S_FAILURE; @@ -412,6 +420,7 @@ peerConfigInit(OM_uint32 *minor, gss_ctx_id_t ctx) eapPeerConfig->chbind_config = NULL; eapPeerConfig->chbind_config_len = 0; } + *minor = 0; return GSS_S_COMPLETE; } @@ -732,8 +741,7 @@ eapGssSmInitAcceptorName(OM_uint32 *minor, /* * Generate channel binding data */ - if (ctx->initiatorCtx.chbindData == NULL) - { + if (ctx->initiatorCtx.chbindData == NULL) { major = peerInitEapChannelBinding(minor, ctx); if (GSS_ERROR(major)) return major; @@ -1180,8 +1188,6 @@ gssEapInitSecContext(OM_uint32 *minor, if (ret_flags != NULL) *ret_flags = ctx->gssFlags; - if (major == GSS_S_COMPLETE) - major = major; if (time_rec != NULL) gssEapContextTime(&tmpMinor, ctx, time_rec); diff --git a/mech_eap/util.h b/mech_eap/util.h index 488d0fa..71f3473 100644 --- a/mech_eap/util.h +++ b/mech_eap/util.h @@ -776,20 +776,20 @@ verifyTokenHeader(OM_uint32 *minor, enum gss_eap_token_type *ret_tok_type); /* Helper macros */ +#ifndef GSSEAP_MALLOC #if _WIN32 #include -#define GSSEAP_MALLOC gssalloc_malloc -#define GSSEAP_CALLOC gssalloc_calloc -#define GSSEAP_FREE gssalloc_free -#define GSSEAP_REALLOC gssalloc_realloc -#endif - -#ifndef GSSEAP_MALLOC +#define GSSEAP_MALLOC gssalloc_malloc +#define GSSEAP_CALLOC gssalloc_calloc +#define GSSEAP_FREE gssalloc_free +#define GSSEAP_REALLOC gssalloc_realloc +#else #define GSSEAP_CALLOC calloc #define GSSEAP_MALLOC malloc #define GSSEAP_FREE free #define GSSEAP_REALLOC realloc -#endif +#endif /* _WIN32 */ +#endif /* !GSSEAP_MALLOC */ #ifndef GSSAPI_CALLCONV #define GSSAPI_CALLCONV KRB5_CALLCONV @@ -1019,6 +1019,7 @@ krbPrincUnparseServiceSpecifics(krb5_context krbContext, krb5_principal krbPrinc nameBuf->value = NULL; nameBuf->length = 0; } + return result; } @@ -1026,6 +1027,8 @@ static inline void krbFreeUnparsedName(krb5_context krbContext, gss_buffer_t nameBuf) { krb5_free_unparsed_name(krbContext, (char *)(nameBuf->value)); + nameBuf->value = NULL; + nameBuf->length = 0; } static inline void