Skip to content

Commit

Permalink
gss: pass mechanism error tokens through SPNEGO
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@lhoward
lhoward committed Apr 24, 2020
1 parent dfb1e6f commit 8d19f3f
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 17 deletions.
18 changes: 13 additions & 5 deletions lib/gssapi/spnego/accept_sec_context.c
View file
Expand Up @@ -35,10 +35,12 @@

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;

Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
46 changes: 34 additions & 12 deletions lib/gssapi/spnego/init_sec_context.c
View file
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand 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;
Expand Down Expand Up @@ -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;
Expand All @@ -578,19 +589,30 @@ 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, "
"but we are not complete yet");
}
}

if (*resp.u.negTokenResp.negState == request_mic) {
if (negState == request_mic) {
ctx->flags.peer_require_mic = 1;
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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)
{
Expand Down

0 comments on commit 8d19f3f

Please sign in to comment.