More error reporting

diff --git a/mech_eap/accept_sec_context.c b/mech_eap/accept_sec_context.c
index 2fe3abf..e75abd0 100644 (file)
--- a/mech_eap/accept_sec_context.c
+++ b/mech_eap/accept_sec_context.c
@@ -43,7 +43,7 @@ eapGssSmAcceptGssReauth(OM_uint32 *minor,
 #endif
 
 /*
- * Mark a context as ready for cryptographic operations
+ * Mark an acceptor context as ready for cryptographic operations
  */
 static OM_uint32
 acceptReadyEap(OM_uint32 *minor, gss_ctx_id_t ctx, gss_cred_id_t cred)
@@ -107,6 +107,10 @@ acceptReadyEap(OM_uint32 *minor, gss_ctx_id_t ctx, gss_cred_id_t cred)
     return GSS_S_COMPLETE;
 }
 
+/*
+ * Emit a identity EAP request to force the initiator (peer) to identify
+ * itself.
+ */
 static OM_uint32
 eapGssSmAcceptIdentity(OM_uint32 *minor,
                        gss_ctx_id_t ctx,
@@ -150,6 +154,9 @@ eapGssSmAcceptIdentity(OM_uint32 *minor,
     return GSS_S_CONTINUE_NEEDED;
 }
 
+/*
+ * Pass the asserted acceptor identity to the authentication server.
+ */
 static OM_uint32
 setAcceptorIdentity(OM_uint32 *minor,
                     gss_ctx_id_t ctx,
@@ -237,6 +244,9 @@ setAcceptorIdentity(OM_uint32 *minor,
     return GSS_S_COMPLETE;
 }
 
+/*
+ * Allocate a RadSec handle
+ */
 static OM_uint32
 createRadiusHandle(OM_uint32 *minor,
                    gss_cred_id_t cred,
@@ -297,6 +307,9 @@ fail:
     return gssEapRadiusMapError(minor, err);
 }
 
+/*
+ * Process a EAP response from the initiator.
+ */
 static OM_uint32
 eapGssSmAcceptAuthenticate(OM_uint32 *minor,
                            gss_ctx_id_t ctx,
@@ -389,7 +402,7 @@ eapGssSmAcceptAuthenticate(OM_uint32 *minor,
     if (frresp->code == PW_ACCESS_CHALLENGE) {
         major = gssEapRadiusGetAvp(minor, frresp->vps, PW_STATE, 0,
                                    &ctx->acceptorCtx.state, TRUE);
-        if (major != GSS_S_UNAVAILABLE && GSS_ERROR(major))
+        if (GSS_ERROR(major) && *minor != GSSEAP_NO_SUCH_ATTR)
             goto cleanup;
     } else {
         ctx->acceptorCtx.vps = frresp->vps;
@@ -497,10 +510,11 @@ makeErrorToken(OM_uint32 *minor,
     case GSSEAP_MISSING_EAP_REQUEST:
         break;
     default:
-        /* Don't return system error codes */
         if (IS_RADIUS_ERROR(minorStatus))
+            /* Squash RADIUS error codes */
             minorStatus = GSSEAP_GENERIC_RADIUS_ERROR;
         else
+            /* Don't return system error codes */
             return GSS_S_COMPLETE;
     }
 
@@ -629,7 +643,7 @@ gss_accept_sec_context(OM_uint32 *minor,
                                    input_chan_bindings,
                                    &innerOutputToken);
         if (GSS_ERROR(major)) {
-            /* Generate an error token */
+            /* Possibly generate an error token */
             tmpMajor = makeErrorToken(&tmpMinor, major, *minor, &innerOutputToken);
             if (GSS_ERROR(tmpMajor)) {
                 major = tmpMajor;

diff --git a/mech_eap/display_status.c b/mech_eap/display_status.c
index 3194ae3..d285113 100644 (file)
--- a/mech_eap/display_status.c
+++ b/mech_eap/display_status.c
@@ -59,6 +59,13 @@ createStatusInfoKey(void)
     GSSEAP_KEY_CREATE(&gssEapStatusInfoKey, destroyStatusInfo);
 }
 
+/*
+ * Associate a message with a mechanism (minor) status code. This function
+ * takes ownership of the message regardless of success. The message must
+ * be explicitly cleared, if required, so it is suggested that a specific
+ * minor code is either always or never associated with a message, to avoid
+ * dangling (and potentially confusing) error messages.
+ */
 static void
 saveStatusInfoNoCopy(OM_uint32 minor, char *message)
 {
@@ -69,7 +76,9 @@ saveStatusInfoNoCopy(OM_uint32 minor, char *message)
     p = GSSEAP_GETSPECIFIC(gssEapStatusInfoKey);
     for (; p != NULL; p = p->next) {
         if (p->code == minor) {
-            GSSEAP_FREE(p->message);
+            /* Set message in-place */
+            if (p->message != NULL)
+                GSSEAP_FREE(p->message);
             p->message = message;
             return;
         }
@@ -78,7 +87,8 @@ saveStatusInfoNoCopy(OM_uint32 minor, char *message)
 
     p = GSSEAP_CALLOC(1, sizeof(*p));
     if (p == NULL) {
-        GSSEAP_FREE(message);
+        if (message != NULL)
+            GSSEAP_FREE(message);
         return;
     }
 
@@ -151,6 +161,7 @@ gss_display_status(OM_uint32 *minor,
     if (errMsg == NULL) {
         GSSEAP_KRB_INIT(&krbContext);
 
+        /* Try the com_err message */
         errMsg = krb5_get_error_message(krbContext, status_value);
     }
 

diff --git a/mech_eap/export_sec_context.c b/mech_eap/export_sec_context.c
index 54cc3b9..8cda194 100644 (file)
--- a/mech_eap/export_sec_context.c
+++ b/mech_eap/export_sec_context.c
@@ -32,6 +32,9 @@
 
 #include "gssapiP_eap.h"
 
+/*
+ * Export a partially established acceptor context.
+ */
 static OM_uint32
 gssEapExportPartialContext(OM_uint32 *minor,
                            gss_ctx_id_t ctx,
@@ -101,8 +104,10 @@ gssEapExportSecContext(OM_uint32 *minor,
     unsigned char *p;
 
     if ((CTX_IS_INITIATOR(ctx) && !CTX_IS_ESTABLISHED(ctx)) ||
-        ctx->mechanismUsed == GSS_C_NO_OID)
+        ctx->mechanismUsed == GSS_C_NO_OID) {
+        *minor = GSSEAP_CONTEXT_INCOMPLETE;
         return GSS_S_NO_CONTEXT;
+    }
 
     key.length = KRB_KEY_LENGTH(&ctx->rfc3961Key);
     key.value  = KRB_KEY_DATA(&ctx->rfc3961Key);

diff --git a/mech_eap/gsseap_err.et b/mech_eap/gsseap_err.et
index 8d4d53f..0fb4407 100644 (file)
--- a/mech_eap/gsseap_err.et
+++ b/mech_eap/gsseap_err.et
@@ -66,7 +66,9 @@ error_code GSSEAP_ATTR_CONTEXT_FAILURE,     "Failed to initialise attribute cont
 error_code GSSEAP_BAD_CONTEXT_TOKEN,        "Context token is malformed or corrupt"
 error_code GSSEAP_MISSING_IOV,              "IOV is missing required buffer"
 error_code GSSEAP_BAD_STREAM_IOV,           "Stream IOV can only contain a single data buffer"
+error_code GSSEAP_BAD_PADDING_IOV,          "Padding IOV is not permitted for RFC 4121 tokens"
 error_code GSSEAP_BAD_PRF_KEY,              "PRF key usage type is unknown"
 error_code GSSEAP_BAD_ERROR_TOKEN,          "Error token is malformed or corrupt"
+error_code GSSEAP_BAD_WRAP_TOKEN,           "Bad RFC 4121 wrap or MIC token"
 
 end

diff --git a/mech_eap/init_sec_context.c b/mech_eap/init_sec_context.c
index fb34b36..f509327 100644 (file)
--- a/mech_eap/init_sec_context.c
+++ b/mech_eap/init_sec_context.c
@@ -249,6 +249,9 @@ peerConfigFree(OM_uint32 *minor,
     return GSS_S_COMPLETE;
 }
 
+/*
+ * Mark an initiator context as ready for cryptographic operations
+ */
 static OM_uint32
 initReady(OM_uint32 *minor, gss_ctx_id_t ctx, OM_uint32 reqFlags)
 {

diff --git a/mech_eap/inquire_mech_for_saslname.c b/mech_eap/inquire_mech_for_saslname.c
index 76ad620..ca54540 100644 (file)
--- a/mech_eap/inquire_mech_for_saslname.c
+++ b/mech_eap/inquire_mech_for_saslname.c
@@ -66,10 +66,12 @@ gss_inquire_saslname_for_mech(OM_uint32 *minor,
 
     if (sasl_mech_name != GSS_C_NO_BUFFER) {
         name = gssEapOidToSaslName(mech);
-        if (name == GSS_C_NO_BUFFER)
+        if (name == GSS_C_NO_BUFFER) {
+            *minor = GSSEAP_WRONG_MECH;
             major = GSS_S_BAD_MECH;
-        else
+        } else {
             major = duplicateBuffer(minor, name, sasl_mech_name);
+        }
     }
 
     return major;

diff --git a/mech_eap/inquire_names_for_mech.c b/mech_eap/inquire_names_for_mech.c
index 8ceaa89..4ca60a3 100644 (file)
--- a/mech_eap/inquire_names_for_mech.c
+++ b/mech_eap/inquire_names_for_mech.c
@@ -50,7 +50,7 @@ gss_inquire_names_for_mech(OM_uint32 *minor,
     size_t i;
 
     if (!gssEapIsMechanismOid(mechanism)) {
-        *minor = 0;
+        *minor = GSSEAP_WRONG_MECH;
         return GSS_S_BAD_MECH;
     }
 

diff --git a/mech_eap/inquire_saslname_for_mech.c b/mech_eap/inquire_saslname_for_mech.c
index 1565d66..64a33c3 100644 (file)
--- a/mech_eap/inquire_saslname_for_mech.c
+++ b/mech_eap/inquire_saslname_for_mech.c
@@ -38,8 +38,10 @@ gss_inquire_mech_for_saslname(OM_uint32 *minor,
                               gss_OID *mech_type)
 {
     *mech_type = gssEapSaslNameToOid(sasl_mech_name);
-    if (*mech_type == GSS_C_NO_OID)
+    if (*mech_type == GSS_C_NO_OID) {
+        *minor = GSSEAP_WRONG_MECH;
         return GSS_S_BAD_MECH;
+    }
 
     return GSS_S_COMPLETE;
 }

diff --git a/mech_eap/process_context_token.c b/mech_eap/process_context_token.c
index daba6f8..5bb978f 100644 (file)
--- a/mech_eap/process_context_token.c
+++ b/mech_eap/process_context_token.c
@@ -42,8 +42,10 @@ gss_process_context_token(OM_uint32 *minor,
 
     *minor = 0;
 
-    if (ctx == NULL)
+    if (ctx == NULL) {
+        *minor = EINVAL;
         return GSS_S_NO_CONTEXT;
+    }
 
     GSSEAP_MUTEX_LOCK(&ctx->mutex);
 

diff --git a/mech_eap/set_cred_option.c b/mech_eap/set_cred_option.c
index 95ec6a4..1854920 100644 (file)
--- a/mech_eap/set_cred_option.c
+++ b/mech_eap/set_cred_option.c
@@ -148,8 +148,10 @@ gssspi_set_cred_option(OM_uint32 *minor,
     gss_cred_id_t cred = *pCred;
     int i;
 
-    if (cred == GSS_C_NO_CREDENTIAL)
+    if (cred == GSS_C_NO_CREDENTIAL) {
+        *minor = EINVAL;
         return GSS_S_UNAVAILABLE;
+    }
 
     GSSEAP_MUTEX_LOCK(&cred->mutex);
 

diff --git a/mech_eap/unwrap_iov.c b/mech_eap/unwrap_iov.c
index a5a2c2f..10b74fe 100644 (file)
--- a/mech_eap/unwrap_iov.c
+++ b/mech_eap/unwrap_iov.c
@@ -94,8 +94,10 @@ unwrapToken(OM_uint32 *minor,
     assert(header != NULL);
 
     padding = gssEapLocateIov(iov, iov_count, GSS_IOV_BUFFER_TYPE_PADDING);
-    if (padding != NULL && padding->buffer.length != 0)
+    if (padding != NULL && padding->buffer.length != 0) {
+        *minor = GSSEAP_BAD_PADDING_IOV;
         return GSS_S_DEFECTIVE_TOKEN;
+    }
 
     trailer = gssEapLocateIov(iov, iov_count, GSS_IOV_BUFFER_TYPE_TRAILER);
 
@@ -115,11 +117,15 @@ unwrapToken(OM_uint32 *minor,
 
     ptr = (unsigned char *)header->buffer.value;
 
-    if (header->buffer.length < 16)
+    if (header->buffer.length < 16) {
+        *minor = GSSEAP_WRONG_SIZE;
         return GSS_S_DEFECTIVE_TOKEN;
+    }
 
-    if ((ptr[2] & flags) != flags)
+    if ((ptr[2] & flags) != flags) {
+        *minor = GSSEAP_BAD_DIRECTION;
         return GSS_S_BAD_SIG;
+    }
 
     if (toktype == TOK_TYPE_WRAP) {
         unsigned int krbTrailerLen;
@@ -184,7 +190,7 @@ unwrapToken(OM_uint32 *minor,
                 || althdr[2] != ptr[2]
                 || althdr[3] != ptr[3]
                 || memcmp(althdr + 8, ptr + 8, 8) != 0) {
-                *minor = 0;
+                *minor = GSSEAP_BAD_WRAP_TOKEN;
                 return GSS_S_BAD_SIG;
             }
         } else {
@@ -239,7 +245,7 @@ unwrapToken(OM_uint32 *minor,
     return code;
 
 defective:
-    *minor = 0;
+    *minor = GSSEAP_BAD_WRAP_TOKEN;
 
     return GSS_S_DEFECTIVE_TOKEN;
 }
@@ -462,8 +468,10 @@ gssEapUnwrapOrVerifyMIC(OM_uint32 *minor,
 {
     OM_uint32 major;
 
-    if (ctx->encryptionType == ENCTYPE_NULL)
+    if (ctx->encryptionType == ENCTYPE_NULL) {
+        *minor = GSSEAP_KEY_UNAVAILABLE;
         return GSS_S_UNAVAILABLE;
+    }
 
     if (gssEapLocateIov(iov, iov_count, GSS_IOV_BUFFER_TYPE_STREAM) != NULL) {
         major = unwrapStream(minor, ctx, conf_state, qop_state,

diff --git a/mech_eap/util_radius.cpp b/mech_eap/util_radius.cpp
index 1e93098..9136a5c 100644 (file)
--- a/mech_eap/util_radius.cpp
+++ b/mech_eap/util_radius.cpp
@@ -507,8 +507,12 @@ gssEapRadiusGetRawAvp(OM_uint32 *minor,
     uint32_t attr = VENDORATTR(vendor, attribute);
 
     *vp = pairfind(vps, attr);
+    if (*vp == NULL) {
+        *minor = GSSEAP_NO_SUCH_ATTR;
+        return GSS_S_UNAVAILABLE;
+    }
 
-    return (*vp == NULL) ? GSS_S_UNAVAILABLE : GSS_S_COMPLETE;
+    return GSS_S_COMPLETE;
 }
 
 OM_uint32
@@ -527,8 +531,10 @@ gssEapRadiusGetAvp(OM_uint32 *minor,
     buffer->value = NULL;
 
     vp = pairfind(vps, attr);
-    if (vp == NULL)
+    if (vp == NULL) {
+        *minor = GSSEAP_NO_SUCH_ATTR;
         return GSS_S_UNAVAILABLE;
+    }
 
     do {
         buffer->length += vp->length;

diff --git a/mech_eap/wrap_iov.c b/mech_eap/wrap_iov.c
index 8b8dc69..0b4fcd5 100644 (file)
--- a/mech_eap/wrap_iov.c
+++ b/mech_eap/wrap_iov.c
@@ -98,8 +98,10 @@ gssEapWrapOrGetMIC(OM_uint32 *minor,
     size_t dataLen, assocDataLen;
     krb5_context krbContext;
 
-    if (ctx->encryptionType == ENCTYPE_NULL)
+    if (ctx->encryptionType == ENCTYPE_NULL) {
+        *minor = GSSEAP_KEY_UNAVAILABLE;
         return GSS_S_UNAVAILABLE;
+    }
 
     GSSEAP_KRB_INIT(&krbContext);