gss: pass mechanism error tokens through SPNEGO
authorLuke Howard <lukeh@padl.com>
Tue, 21 Apr 2020 23:35:14 +0000 (09:35 +1000)
committerLuke Howard <lukeh@padl.com>
Fri, 24 Apr 2020 05:07:55 +0000 (15:07 +1000)
Fix for issue #486 based on a patch by Nico Williams.

A GSS-API acceptor can return an error token to be sent to the initiator. Our
SPNEGO implementation discarded these when sending a SPNEGO reject response.
This patch fixes the SPNEGO acceptor to convey those in the SPNEGO response.

The SPNEGO initiator is also updated to not bail out early on receiving a
SPNEGO reject response from the acceptor, but instead pass the response token
(if any) to gss_init_sec_context(). A reject response with no response token
will continue to return an error.

lib/gssapi/spnego/accept_sec_context.c
lib/gssapi/spnego/init_sec_context.c

index 89311a1e75c5bce6ca759fa9a07a9f8f5047373e..d2a8155fef9117b77d3ef3cd035bcc715635533b 100644 (file)
 
 static OM_uint32
 send_reject (OM_uint32 *minor_status,
+            gss_buffer_t mech_token,
             gss_buffer_t output_token)
 {
     NegotiationToken nt;
     size_t size;
+    heim_octet_string responseToken;
 
     nt.element = choice_NegotiationToken_negTokenResp;
 
@@ -50,6 +52,13 @@ send_reject (OM_uint32 *minor_status,
     *(nt.u.negTokenResp.negState)  = reject;
     nt.u.negTokenResp.supportedMech = NULL;
     nt.u.negTokenResp.responseToken = NULL;
+
+    if (mech_token != GSS_C_NO_BUFFER && mech_token->value != NULL) {
+       responseToken.length = mech_token->length;
+       responseToken.data   = mech_token->value;
+       nt.u.negTokenResp.responseToken = &responseToken;
+     } else
+       nt.u.negTokenResp.responseToken = NULL;
     nt.u.negTokenResp.mechListMIC   = NULL;
 
     ASN1_MALLOC_ENCODE(NegotiationToken,
@@ -513,7 +522,7 @@ acceptor_complete(OM_uint32 * minor_status,
            ret = _gss_spnego_verify_mechtypes_mic(minor_status, ctx, mic);
            if (ret) {
                if (*get_mic)
-                   send_reject(minor_status, output_token);
+                   send_reject(minor_status, GSS_C_NO_BUFFER, output_token);
                if (buf.value)
                    free(buf.value);
                return ret;
@@ -888,12 +897,11 @@ acceptor_continue
                              input_chan_bindings,
                              &obuf,
                              delegated_cred_handle);
-           if (ret == GSS_S_COMPLETE || ret == GSS_S_CONTINUE_NEEDED) {
-               mech_output_token = &obuf;
-           }
+           mech_output_token = &obuf;
            if (ret != GSS_S_COMPLETE && ret != GSS_S_CONTINUE_NEEDED) {
                free_NegotiationToken(&nt);
-               send_reject(&junk, output_token);
+               send_reject(&junk, mech_output_token, output_token);
+               gss_release_buffer(&junk, mech_output_token);
                HEIMDAL_MUTEX_unlock(&ctx->ctx_id_mutex);
                return ret;
            }
index c1b20a24c84a0a485a72f128d2a9b0d81f4145cd..12ec0ea41060c491d5f9d3c5c4616c4ed7eb71cb 100644 (file)
@@ -427,6 +427,7 @@ spnego_reply(OM_uint32 * minor_status,
     OM_uint32 ret, minor;
     NegotiationToken resp;
     gss_buffer_desc mech_output_token;
+    NegStateEnum negState;
 
     *minor_status = 0;
 
@@ -441,18 +442,21 @@ spnego_reply(OM_uint32 * minor_status,
     if (ret)
       return ret;
 
+    /* The SPNEGO token must be a negTokenResp */
     if (resp.element != choice_NegotiationToken_negTokenResp) {
        free_NegotiationToken(&resp);
        *minor_status = 0;
        return GSS_S_BAD_MECH;
     }
 
-    if (resp.u.negTokenResp.negState == NULL
-       || *(resp.u.negTokenResp.negState) == reject)
-    {
-       free_NegotiationToken(&resp);
-       return GSS_S_BAD_MECH;
-    }
+    /*
+     * When negState is absent, the actual state should be inferred from
+     * the state of the negotiated mechanism context. (RFC 4178 4.2.2.)
+     */
+    if (resp.u.negTokenResp.negState != NULL)
+       negState = *resp.u.negTokenResp.negState;
+    else
+       negState = accept_incomplete;
 
     /*
      * Pick up the mechanism that the acceptor selected, only pick up
@@ -528,7 +532,7 @@ spnego_reply(OM_uint32 * minor_status,
                                       "SPNEGO acceptor didn't send supportedMech");
     }
 
-    /* if a token (of non zero length), or no context, pass to underlaying mech */
+    /* if a token (of non zero length) pass to underlaying mech */
     if ((resp.u.negTokenResp.responseToken != NULL && resp.u.negTokenResp.responseToken->length) ||
        ctx->negotiated_ctx_id == GSS_C_NO_CONTEXT) {
        gss_buffer_desc mech_input_token;
@@ -567,9 +571,16 @@ spnego_reply(OM_uint32 * minor_status,
                                       &mech_output_token,
                                       &ctx->mech_flags,
                                       &ctx->mech_time_rec);
-           if (GSS_ERROR(ret))
+           if (GSS_ERROR(ret)) {
                gss_mg_collect_error(ctx->selected_mech_type, ret, minor);
+           }
        }
+       /*
+        * If the acceptor rejected, we're out even if the inner context is
+        * now complete. Note that the rejection is not integrity-protected.
+        */
+       if (negState == reject)
+           ret = GSS_S_BAD_MECH;
        if (GSS_ERROR(ret)) {
            free_NegotiationToken(&resp);
            *minor_status = minor;
@@ -578,11 +589,22 @@ spnego_reply(OM_uint32 * minor_status,
        if (ret == GSS_S_COMPLETE) {
            ctx->flags.open = 1;
        }
-    } else if (*resp.u.negTokenResp.negState == accept_completed) {
+    } else if (negState == reject) {
+       free_NegotiationToken(&resp);
+       return gss_mg_set_error_string(GSS_SPNEGO_MECHANISM,
+                                      GSS_S_BAD_MECH, (*minor_status = EPERM),
+                                      "SPNEGO acceptor rejected initiator token");
+    } else if (negState == accept_completed) {
+       /*
+        * Note that the accept_completed isn't integrity-protected, but
+        * ctx->maybe_open can only be true if the inner context is fully
+        * established.
+        */
        if (ctx->flags.maybe_open)
            ctx->flags.open = 1;
 
        if (!ctx->flags.open) {
+           free_NegotiationToken(&resp);
            return gss_mg_set_error_string(GSS_SPNEGO_MECHANISM,
                                           GSS_S_BAD_MECH, (*minor_status = EINVAL),
                                           "SPNEGO acceptor sent acceptor complete, "
@@ -590,7 +612,7 @@ spnego_reply(OM_uint32 * minor_status,
        }
     }
 
-    if (*resp.u.negTokenResp.negState == request_mic) {
+    if (negState == request_mic) {
        ctx->flags.peer_require_mic = 1;
     }
 
@@ -643,7 +665,7 @@ spnego_reply(OM_uint32 * minor_status,
 
     if (ctx->flags.open) {
 
-       if (*resp.u.negTokenResp.negState == accept_completed && ctx->flags.safe_omit) {
+       if (negState == accept_completed && ctx->flags.safe_omit) {
            ctx->initiator_state = step_completed;
            ret = GSS_S_COMPLETE;
        } else if (ctx->flags.require_mic != 0 && ctx->flags.verified_mic == 0) {
@@ -655,7 +677,7 @@ spnego_reply(OM_uint32 * minor_status,
        }
     }
 
-    if (*resp.u.negTokenResp.negState != accept_completed ||
+    if (negState != accept_completed ||
        ctx->initiator_state != step_completed ||
        mech_output_token.length)
     {