krb5 commit: Fix SPNEGO output parameter bugs

https://github.com/krb5/krb5/commit/24b844714dea3e47b17511746b5df5b6ddf13d43
commit 24b844714dea3e47b17511746b5df5b6ddf13d43
Author: Greg Hudson <[email protected]>
Date:   Wed Nov 6 23:02:51 2019 -0500

    Fix SPNEGO output parameter bugs
    
    When accepting, do not leak a name if the underlying mech reports a
    src_name twice.  Record mech_type and delegated_cred_handle and report
    them to the caller at the final SPNEGO step, not when the underlying
    mech reports them.
    
    When initiating or accepting, report ret_flags at every step, and
    filter out PROT_READY as required by RFC 4178 section 3.1.  Report a
    time_rec value at the final step even if we didn't call into the
    underlying mech, using a call to gss_context_time() if necessary.
    
    In the mechglue, initialize ret_flags and time_rec for both
    gss_initialize_sec_context() and gss_accept_sec_context().
    
    ticket: 8845

 src/lib/gssapi/mechglue/g_accept_sec_context.c |    6 ++
 src/lib/gssapi/mechglue/g_init_sec_context.c   |    6 ++
 src/lib/gssapi/spnego/gssapiP_spnego.h         |    1 +
 src/lib/gssapi/spnego/spnego_mech.c            |   85 +++++++++++++-----------
 4 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/src/lib/gssapi/mechglue/g_accept_sec_context.c b/src/lib/gssapi/mechglue/g_accept_sec_context.c
index 1a03cf4..8e63a9b 100644
--- a/src/lib/gssapi/mechglue/g_accept_sec_context.c
+++ b/src/lib/gssapi/mechglue/g_accept_sec_context.c
@@ -66,6 +66,12 @@ val_acc_sec_ctx_args(
 	output_token->value = NULL;
     }
 
+    if (ret_flags != NULL)
+	*ret_flags = 0;
+
+    if (time_rec != NULL)
+	*time_rec = 0;
+
     if (d_cred != NULL)
 	*d_cred = GSS_C_NO_CREDENTIAL;
 
diff --git a/src/lib/gssapi/mechglue/g_init_sec_context.c b/src/lib/gssapi/mechglue/g_init_sec_context.c
index e2df1ce..cf10192 100644
--- a/src/lib/gssapi/mechglue/g_init_sec_context.c
+++ b/src/lib/gssapi/mechglue/g_init_sec_context.c
@@ -63,6 +63,12 @@ val_init_sec_ctx_args(
 	output_token->value = NULL;
     }
 
+    if (ret_flags != NULL)
+	*ret_flags = 0;
+
+    if (time_rec != NULL)
+	*time_rec = 0;
+
     /* Validate arguments. */
 
     if (minor_status == NULL)
diff --git a/src/lib/gssapi/spnego/gssapiP_spnego.h b/src/lib/gssapi/spnego/gssapiP_spnego.h
index 4ddee34..cad47ee 100644
--- a/src/lib/gssapi/spnego/gssapiP_spnego.h
+++ b/src/lib/gssapi/spnego/gssapiP_spnego.h
@@ -106,6 +106,7 @@ typedef struct {
 	OM_uint32 ctx_flags;
 	gss_name_t internal_name;
 	gss_OID actual_mech;
+	gss_cred_id_t deleg_cred;
 } spnego_gss_ctx_id_rec, *spnego_gss_ctx_id_t;
 
 /*
diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index c57d7d7..5f92cb6 100644
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -130,8 +130,7 @@ init_ctx_reselect(OM_uint32 *, spnego_gss_ctx_id_t, OM_uint32,
 static OM_uint32
 init_ctx_call_init(OM_uint32 *, spnego_gss_ctx_id_t, spnego_gss_cred_id_t,
 		   OM_uint32, gss_name_t, OM_uint32, OM_uint32, gss_buffer_t,
-		   gss_OID *, gss_buffer_t, OM_uint32 *, OM_uint32 *,
-		   send_token_flag *);
+		   gss_buffer_t, OM_uint32 *, send_token_flag *);
 
 static OM_uint32
 acc_ctx_new(OM_uint32 *, gss_buffer_t, spnego_gss_cred_id_t, gss_buffer_t *,
@@ -145,9 +144,8 @@ acc_ctx_vfy_oid(OM_uint32 *, spnego_gss_ctx_id_t, gss_OID,
 		OM_uint32 *, send_token_flag *);
 static OM_uint32
 acc_ctx_call_acc(OM_uint32 *, spnego_gss_ctx_id_t, spnego_gss_cred_id_t,
-		 gss_buffer_t, gss_OID *, gss_buffer_t,
-		 OM_uint32 *, OM_uint32 *, gss_cred_id_t *,
-		 OM_uint32 *, send_token_flag *);
+		 gss_buffer_t, gss_buffer_t, OM_uint32 *, OM_uint32 *,
+		 send_token_flag *);
 
 static gss_OID
 negotiate_mech(gss_OID_set, gss_OID_set, OM_uint32 *);
@@ -468,6 +466,7 @@ create_spnego_ctx(int initiate)
 	spnego_ctx->initiate = initiate;
 	spnego_ctx->internal_name = GSS_C_NO_NAME;
 	spnego_ctx->actual_mech = GSS_C_NO_OID;
+	spnego_ctx->deleg_cred = GSS_C_NO_CREDENTIAL;
 
 	return (spnego_ctx);
 }
@@ -900,9 +899,7 @@ init_ctx_call_init(OM_uint32 *minor_status,
 		   OM_uint32 req_flags,
 		   OM_uint32 time_req,
 		   gss_buffer_t mechtok_in,
-		   gss_OID *actual_mech,
 		   gss_buffer_t mechtok_out,
-		   OM_uint32 *ret_flags,
 		   OM_uint32 *time_rec,
 		   send_token_flag *send_token)
 {
@@ -938,8 +935,6 @@ init_ctx_call_init(OM_uint32 *minor_status,
 
 	if (ret == GSS_S_COMPLETE) {
 		sc->mech_complete = 1;
-		if (ret_flags != NULL)
-			*ret_flags = sc->ctx_flags;
 		/*
 		 * Microsoft SPNEGO implementations expect an even number of
 		 * token exchanges.  So if we're sending a final token, ask for
@@ -979,8 +974,8 @@ init_ctx_call_init(OM_uint32 *minor_status,
 		goto fail;
 	tmpret = init_ctx_call_init(&tmpmin, sc, spcred, acc_negState,
 				    target_name, req_flags, time_req,
-				    mechtok_in, actual_mech, mechtok_out,
-				    ret_flags, time_rec, send_token);
+				    mechtok_in, mechtok_out, time_rec,
+				    send_token);
 	if (HARD_ERROR(tmpret))
 		goto fail;
 	*minor_status = tmpmin;
@@ -1056,6 +1051,8 @@ spnego_gss_init_sec_context(
 
 	if (actual_mech != NULL)
 		*actual_mech = GSS_C_NO_OID;
+	if (time_rec != NULL)
+		*time_rec = 0;
 
 	/* Step 1: perform mechanism negotiation. */
 	spcred = (spnego_gss_cred_id_t)claimant_cred_handle;
@@ -1080,9 +1077,8 @@ spnego_gss_init_sec_context(
 	if (!spnego_ctx->mech_complete) {
 		ret = init_ctx_call_init(minor_status, spnego_ctx, spcred,
 					 acc_negState, target_name, req_flags,
-					 time_req, mechtok_in, actual_mech,
-					 &mechtok_out, ret_flags, time_rec,
-					 &send_token);
+					 time_req, mechtok_in, &mechtok_out,
+					 time_rec, &send_token);
 		if (ret != GSS_S_COMPLETE)
 			goto cleanup;
 
@@ -1106,6 +1102,9 @@ spnego_gss_init_sec_context(
 			goto cleanup;
 	}
 
+	if (ret_flags != NULL)
+		*ret_flags = spnego_ctx->ctx_flags & ~GSS_C_PROT_READY_FLAG;
+
 	ret = (send_token == NO_TOKEN_SEND || negState == ACCEPT_COMPLETE) ?
 		GSS_S_COMPLETE : GSS_S_CONTINUE_NEEDED;
 
@@ -1134,8 +1133,12 @@ cleanup:
 		spnego_ctx->opened = 1;
 		if (actual_mech != NULL)
 			*actual_mech = spnego_ctx->actual_mech;
-		if (ret_flags != NULL)
-			*ret_flags = spnego_ctx->ctx_flags;
+		/* Get an updated lifetime if we didn't call into the mech. */
+		if (time_rec != NULL && *time_rec == 0) {
+			(void) gss_context_time(&tmpmin,
+						spnego_ctx->ctx_handle,
+						time_rec);
+		}
 	} else if (ret != GSS_S_CONTINUE_NEEDED) {
 		if (spnego_ctx != NULL) {
 			gss_delete_sec_context(&tmpmin,
@@ -1533,12 +1536,10 @@ cleanup:
 static OM_uint32
 acc_ctx_call_acc(OM_uint32 *minor_status, spnego_gss_ctx_id_t sc,
 		 spnego_gss_cred_id_t spcred, gss_buffer_t mechtok_in,
-		 gss_OID *mech_type, gss_buffer_t mechtok_out,
-		 OM_uint32 *ret_flags, OM_uint32 *time_rec,
-		 gss_cred_id_t *delegated_cred_handle,
+		 gss_buffer_t mechtok_out, OM_uint32 *time_rec,
 		 OM_uint32 *negState, send_token_flag *tokflag)
 {
-	OM_uint32 ret;
+	OM_uint32 ret, tmpmin;
 	gss_OID_desc mechoid;
 	gss_cred_id_t mcred;
 
@@ -1558,17 +1559,13 @@ acc_ctx_call_acc(OM_uint32 *minor_status, spnego_gss_ctx_id_t sc,
 	}
 
 	mcred = (spcred == NULL) ? GSS_C_NO_CREDENTIAL : spcred->mcred;
-	ret = gss_accept_sec_context(minor_status,
-				     &sc->ctx_handle,
-				     mcred,
-				     mechtok_in,
-				     GSS_C_NO_CHANNEL_BINDINGS,
-				     &sc->internal_name,
-				     mech_type,
-				     mechtok_out,
-				     &sc->ctx_flags,
-				     time_rec,
-				     delegated_cred_handle);
+	(void) gss_release_name(&tmpmin, &sc->internal_name);
+	(void) gss_release_cred(&tmpmin, &sc->deleg_cred);
+	ret = gss_accept_sec_context(minor_status, &sc->ctx_handle, mcred,
+				     mechtok_in, GSS_C_NO_CHANNEL_BINDINGS,
+				     &sc->internal_name, &sc->actual_mech,
+				     mechtok_out, &sc->ctx_flags, time_rec,
+				     &sc->deleg_cred);
 	if (ret == GSS_S_COMPLETE) {
 #ifdef MS_BUG_TEST
 		/*
@@ -1585,8 +1582,6 @@ acc_ctx_call_acc(OM_uint32 *minor_status, spnego_gss_ctx_id_t sc,
 		}
 #endif
 		sc->mech_complete = 1;
-		if (ret_flags != NULL)
-			*ret_flags = sc->ctx_flags;
 
 		if (!sc->mic_reqd ||
 		    !(sc->ctx_flags & GSS_C_INTEG_FLAG)) {
@@ -1724,11 +1719,9 @@ spnego_gss_accept_sec_context(
 	 * whether it is the first round-trip.
 	 */
 	if (negState != REQUEST_MIC && mechtok_in != GSS_C_NO_BUFFER) {
-		ret = acc_ctx_call_acc(minor_status, sc, spcred,
-				       mechtok_in, mech_type, &mechtok_out,
-				       ret_flags, time_rec,
-				       delegated_cred_handle,
-				       &negState, &return_token);
+		ret = acc_ctx_call_acc(minor_status, sc, spcred, mechtok_in,
+				       &mechtok_out, time_rec, &negState,
+				       &return_token);
 	}
 
 	/* Step 3: process or generate the MIC, if the negotiated mech is
@@ -1741,6 +1734,10 @@ spnego_gss_accept_sec_context(
 				 sc, &mic_out,
 				 &negState, &return_token);
 	}
+
+	if (!HARD_ERROR(ret) && ret_flags != NULL)
+		*ret_flags = sc->ctx_flags & ~GSS_C_PROT_READY_FLAG;
+
 cleanup:
 	if (return_token == INIT_TOKEN_SEND && sendTokenInit) {
 		assert(sc != NULL);
@@ -1767,6 +1764,17 @@ cleanup:
 			*src_name = sc->internal_name;
 			sc->internal_name = GSS_C_NO_NAME;
 		}
+		if (mech_type != NULL)
+			*mech_type = sc->actual_mech;
+		/* Get an updated lifetime if we didn't call into the mech. */
+		if (time_rec != NULL && *time_rec == 0) {
+			(void) gss_context_time(&tmpmin, sc->ctx_handle,
+						time_rec);
+		}
+		if (delegated_cred_handle != NULL) {
+			*delegated_cred_handle = sc->deleg_cred;
+			sc->deleg_cred = GSS_C_NO_CREDENTIAL;
+		}
 	} else if (ret != GSS_S_CONTINUE_NEEDED) {
 		if (sc != NULL) {
 			gss_delete_sec_context(&tmpmin, &sc->ctx_handle,
@@ -3065,6 +3073,7 @@ release_spnego_ctx(spnego_gss_ctx_id_t *ctx)
 		(void) gss_release_oid_set(&minor_stat, &context->mech_set);
 
 		(void) gss_release_name(&minor_stat, &context->internal_name);
+		(void) gss_release_cred(&minor_stat, &context->deleg_cred);
 
 		free(context);
 		*ctx = NULL;