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 (file)
--- 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 (file)
--- 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 <gssapi/gssapi_alloc.h>
-#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