Eap channel bindings cleanup

Simplify radius buffer construction and parse service-specifics correctly.

diff --git a/mech_eap/Makefile.am b/mech_eap/Makefile.am
index 23de6af..860d914 100644 (file)
--- a/mech_eap/Makefile.am
+++ b/mech_eap/Makefile.am
@@ -107,6 +107,7 @@ mech_eap_la_SOURCES =                       \
        util_name.c                             \
        util_oid.c                              \
        util_ordering.c                         \
+       util_radius.cpp                         \
        util_sm.c                               \
        util_tld.c                              \
        util_token.c                            \
@@ -146,8 +147,7 @@ mech_eap_la_SOURCES +=                              \
        set_name_attribute.c                    \
        util_attr.cpp                           \
        util_base64.c                           \
-       util_json.cpp                           \
-       util_radius.cpp
+       util_json.cpp
 
 if OPENSAML
 mech_eap_la_SOURCES += util_saml.cpp

diff --git a/mech_eap/accept_sec_context.c b/mech_eap/accept_sec_context.c
index b089bae..5b878b1 100644 (file)
--- a/mech_eap/accept_sec_context.c
+++ b/mech_eap/accept_sec_context.c
@@ -368,30 +368,21 @@ setAcceptorIdentity(OM_uint32 *minor,
 
     if (KRB_PRINC_LENGTH(krbPrinc) > 2) {
         /* Acceptor-Service-Specific */
-        krb5_principal_data ssiPrinc = *krbPrinc;
-        char *ssi;
-
-        KRB_PRINC_LENGTH(&ssiPrinc) -= 2;
-        KRB_PRINC_NAME(&ssiPrinc) += 2;
-
-        *minor = krb5_unparse_name_flags(krbContext, &ssiPrinc,
-                                         KRB5_PRINCIPAL_UNPARSE_NO_REALM, &ssi);
+        *minor = krbPrincUnparseServiceSpecifics(krbContext,
+                                                 krbPrinc, &nameBuf);
         if (*minor != 0)
             return GSS_S_FAILURE;
 
-        nameBuf.value = ssi;
-        nameBuf.length = strlen(ssi);
-
         major = gssEapRadiusAddAvp(minor, vps,
                                    PW_GSS_ACCEPTOR_SERVICE_SPECIFIC,
                                    VENDORPEC_UKERNA,
                                    &nameBuf);
 
         if (GSS_ERROR(major)) {
-            krb5_free_unparsed_name(krbContext, ssi);
+            krbFreeUnparsedName(krbContext, &nameBuf);
             return major;
         }
-        krb5_free_unparsed_name(krbContext, ssi);
+        krbFreeUnparsedName(krbContext, &nameBuf);
     }
 
     krbPrincRealmToGssBuffer(krbPrinc, &nameBuf);

diff --git a/mech_eap/init_sec_context.c b/mech_eap/init_sec_context.c
index 929a7f3..417ad4e 100644 (file)
--- a/mech_eap/init_sec_context.c
+++ b/mech_eap/init_sec_context.c
@@ -197,23 +197,24 @@ static struct eapol_callbacks gssEapPolicyCallbacks = {
 extern int wpa_debug_level;
 #endif
 
-/* */
-static u8 componentToAttrMap[] =
-{
-    128, /* GSS-Acceptor-Service-Name */
-    129, /* GSS-Acceptor-Host-Name    */
-    130  /* GSS-Acceptor-Service-specific */
-};
-#define CHBIND_REALM_FLAG (1 << sizeof(componentToAttrMap))
+#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();
 
 static OM_uint32
 peerInitEapChannelBinding(OM_uint32 *minor, gss_ctx_id_t ctx)
 {
-    struct wpabuf *buf;
-    radius_vendor_attr vendor_attr;
+    struct wpabuf *buf = NULL;
     int component, components = 0;
     unsigned int requested = 0;
     krb5_principal princ;
+    gss_buffer_desc nameBuf;
+    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? */
@@ -223,53 +224,61 @@ peerInitEapChannelBinding(OM_uint32 *minor, gss_ctx_id_t ctx)
     }
 
     princ = ctx->acceptorName->krbPrincipal;
-    if (KRB_PRINC_LENGTH(princ) > sizeof(componentToAttrMap)) {
-        *minor = GSSEAP_BAD_ACCEPTOR_NAME;
-        return GSS_S_BAD_NAME;
+
+    krbPrincComponentToGssBuffer(princ, 0, &nameBuf);
+    if (nameBuf.length > 0) {
+        major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_SERVICE_NAME,
+                                    VENDORPEC_UKERNA, &nameBuf);
+        if (GSS_ERROR(major))
+            goto init_chbind_cleanup;
+        requested |= CHBIND_SERVICE_NAME_FLAG;
     }
 
-    /* allocate a buffer to hold channel binding data to be used by libeap */
-    buf = wpabuf_alloc(256);
-    if (!buf) {
-        *minor = ENOMEM;
-        return GSS_S_FAILURE;
+    krbPrincComponentToGssBuffer(princ, 1, &nameBuf);
+    if (nameBuf.length > 0) {
+        major = gssEapRadiusAddAttr(minor, &buf, PW_GSS_ACCEPTOR_HOST_NAME,
+                                    VENDORPEC_UKERNA, &nameBuf);
+        if (GSS_ERROR(major))
+            goto init_chbind_cleanup;
+        requested |= CHBIND_HOST_NAME_FLAG;
     }
 
-    for (component=0; component < KRB_PRINC_LENGTH(princ); component++)
-    {
-        krb5_data* name_data = KRB_PRINC_COMPONENT(princ, component);
-        if (name_data->length > 0)
-        {
-            components++;
-            vendor_attr = radius_vendor_attr_start(buf, VENDORPEC_UKERNA);
-            vendor_attr = radius_vendor_attr_add_subtype(vendor_attr,
-                componentToAttrMap[component],
-                (unsigned char *) name_data->data,
-                name_data->length);
-            requested |= 1<<component;
-            vendor_attr = radius_vendor_attr_finish(vendor_attr);
+    GSSEAP_KRB_INIT(&krbContext);
+    *minor = krbPrincUnparseServiceSpecifics(krbContext, princ, &nameBuf);
+    if (*minor)
+        goto init_chbind_cleanup;
+
+    if (nameBuf.length > 0) {
+        major = gssEapRadiusAddAttr(minor, &buf,
+                                    PW_GSS_ACCEPTOR_SERVICE_SPECIFIC,
+                                    VENDORPEC_UKERNA, &nameBuf);
+        if (GSS_ERROR(major)) {
+            krbFreeUnparsedName(krbContext, &nameBuf);
+            goto init_chbind_cleanup;
         }
+        requested |= CHBIND_SERVICE_SPECIFIC_FLAG;
     }
-
-    if (KRB_PRINC_REALM(princ) && (KRB_PRINC_REALM(princ)->length > 0)) {
-        components++;
-        requested |= CHBIND_REALM_FLAG;
-        vendor_attr = radius_vendor_attr_start(buf, VENDORPEC_UKERNA);
-        vendor_attr = radius_vendor_attr_add_subtype(vendor_attr, 131,
-                                            (unsigned char *) KRB_PRINC_REALM(princ)->data,
-                                            KRB_PRINC_REALM(princ)->length);
-        vendor_attr = radius_vendor_attr_finish(vendor_attr);
+    krbFreeUnparsedName(krbContext, &nameBuf);
+
+    krbPrincRealmToGssBuffer(princ, &nameBuf);
+    if (nameBuf.length > 0) {
+        major = gssEapRadiusAddAttr(minor, &buf,
+                                    PW_GSS_ACCEPTOR_REALM_NAME,
+                                    VENDORPEC_UKERNA, &nameBuf);
+        requested |= CHBIND_REALM_NAME_FLAG;
     }
 
-    if ((components==0) || (vendor_attr == VENDOR_ATTR_INVALID)) {
+    if (requested==0) {
         wpabuf_free(buf);
         *minor = GSSEAP_BAD_ACCEPTOR_NAME;
         return GSS_S_BAD_NAME;
     }
-    /* @TODO: realloc buf to actual size? */
     ctx->initiatorCtx.chbindData = buf;
     ctx->initiatorCtx.chbindReqFlags = requested;
-    return GSS_S_COMPLETE;
+    buf = NULL;
+init_chbind_cleanup:
+    wpabuf_free(buf);
+    return major;
 }
 
 static void
@@ -306,15 +315,19 @@ peerProcessChbindResponse(void *context, int code, int nsid,
                                                    &vendor_type,
                                                    &unused_data,
                                                    &unused_len) == 0) {
-            if (vendor_type == 131) {
-                accepted |= CHBIND_REALM_FLAG;
-            } else {
-                for (i=0; i<sizeof(componentToAttrMap); i++) {
-                    if (componentToAttrMap[i]==vendor_type) {
-                        accepted |= 1<<i;
-                        break;
-                    }
-                }
+            switch (vendor_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_SPECIFIC:
+                accepted |= CHBIND_SERVICE_SPECIFIC_FLAG;
+                break;
+            case PW_GSS_ACCEPTOR_REALM_NAME:
+                accepted |= CHBIND_REALM_NAME_FLAG;
+                break;
             }
         }
         radius_parser_finish(vendor_specific);

diff --git a/mech_eap/util.h b/mech_eap/util.h
index 350bb04..488d0fa 100644 (file)
--- a/mech_eap/util.h
+++ b/mech_eap/util.h
@@ -981,13 +981,51 @@ static inline void
 krbPrincComponentToGssBuffer(krb5_principal krbPrinc,
                              int index, gss_buffer_t buffer)
 {
+    if (KRB_PRINC_LENGTH(krbPrinc) < index) {
+        buffer->value = NULL;
+        buffer->length = 0;
+    } else {
 #ifdef HAVE_HEIMDAL_VERSION
-    buffer->value = (void *)KRB_PRINC_NAME(krbPrinc)[index];
-    buffer->length = strlen((char *)buffer->value);
+        buffer->value = (void *)KRB_PRINC_NAME(krbPrinc)[index];
+        buffer->length = strlen((char *)buffer->value);
 #else
-    buffer->value = (void *)krb5_princ_component(NULL, krbPrinc, index)->data;
-    buffer->length = krb5_princ_component(NULL, krbPrinc, index)->length;
+        buffer->value = (void *)krb5_princ_component(NULL, krbPrinc, index)->data;
+        buffer->length = krb5_princ_component(NULL, krbPrinc, index)->length;
 #endif /* HAVE_HEIMDAL_VERSION */
+    }
+}
+
+static inline krb5_error_code
+krbPrincUnparseServiceSpecifics(krb5_context krbContext, krb5_principal krbPrinc,
+                                gss_buffer_t nameBuf)
+{
+    krb5_error_code result = 0;
+    if (KRB_PRINC_LENGTH(krbPrinc) > 2) {
+        /* Acceptor-Service-Specific */
+        krb5_principal_data ssiPrinc = *krbPrinc;
+        char *ssi;
+
+        KRB_PRINC_LENGTH(&ssiPrinc) -= 2;
+        KRB_PRINC_NAME(&ssiPrinc) += 2;
+
+        result = krb5_unparse_name_flags(krbContext, &ssiPrinc,
+                                         KRB5_PRINCIPAL_UNPARSE_NO_REALM, &ssi);
+        if (result != 0)
+            return result;
+
+        nameBuf->value = ssi;
+        nameBuf->length = strlen(ssi);
+    } else {
+        nameBuf->value = NULL;
+        nameBuf->length = 0;
+    }
+    return result;
+}
+
+static inline void
+krbFreeUnparsedName(krb5_context krbContext, gss_buffer_t nameBuf)
+{
+    krb5_free_unparsed_name(krbContext, (char *)(nameBuf->value));
 }
 
 static inline void

diff --git a/mech_eap/util_radius.cpp b/mech_eap/util_radius.cpp
index 9111e20..9c5f36f 100644 (file)
--- a/mech_eap/util_radius.cpp
+++ b/mech_eap/util_radius.cpp
@@ -35,6 +35,10 @@
  */
 
 #include "gssapiP_eap.h"
+#include "util_radius.h"
+#include "utils/radius_utils.h"
+
+#ifdef GSSEAP_ENABLE_ACCEPTOR
 
 /* stuff that should be provided by libradsec/libfreeradius-radius */
 #define VENDORATTR(vendor, attr)            (((vendor) << 16) | (attr))
@@ -897,3 +901,17 @@ fail:
 
     return major;
 }
+
+#endif /* GSSEAP_ENABLE_ACCEPTOR */
+
+OM_uint32
+gssEapRadiusAddAttr(OM_uint32 *minor, struct wpabuf **buf, uint16_t attr,
+                    uint16_t vendor, gss_buffer_t buffer)
+{
+    if (radius_add_tlv(buf, attr, vendor, (u8 *)buffer->value,
+                       buffer->length) < 0) {
+        *minor = ENOMEM; /* could be length too long, though */
+        return GSS_S_FAILURE;
+    }
+    return GSS_S_COMPLETE;
+}

diff --git a/mech_eap/util_radius.h b/mech_eap/util_radius.h
index 5d15011..8c34167 100644 (file)
--- a/mech_eap/util_radius.h
+++ b/mech_eap/util_radius.h
@@ -39,6 +39,8 @@
 
 #ifdef __cplusplus
 
+#ifdef GSSEAP_ENABLE_ACCEPTOR
+
 struct gss_eap_radius_attr_provider : gss_eap_attr_provider {
 public:
     gss_eap_radius_attr_provider(void);
@@ -118,6 +120,8 @@ private:
     bool m_authenticated;
 };
 
+#endif /* GSSEAP_ENABLE_ACCEPTOR */
+
 /* For now */
 extern "C" {
 #endif
@@ -161,7 +165,17 @@ gssEapCreateRadiusContext(OM_uint32 *minor,
                           gss_cred_id_t cred,
                           struct rs_context **pRadContext);
 
-#endif
+#endif /* GSSEAP_ENABLE_ACCEPTOR */
+
+/* initiator utilities that require only libeap, and not freeradius */
+struct wpabuf;
+
+OM_uint32
+gssEapRadiusAddAttr(OM_uint32 *minor,
+                    struct wpabuf **dst,
+                    uint16_t type,
+                    uint16_t vendor,
+                    gss_buffer_t value);
 
 /* This really needs to be a function call on Windows */
 #define RS_CONFIG_FILE      SYSCONFDIR "/radsec.conf"