Subject: Invalid initial GSSAPI/SPNEGO token can cause segmentation error or assert failure
Date: Fri, 6 Mar 2009 10:35:57 -0000
From: "Richard Evans" <richard.evans@datanomic.com>
To: <krb5-bugs@mit.edu>

I've been testing a Java client using SPNEGO against Apache using
mod_auth_kerb.  Apache segfaults with this trace:

#0  0x006ffa25 in spnego_gss_accept_sec_context () from
/usr/lib/libgssapi_krb5.so.2
#1  0x006e3349 in gss_accept_sec_context () from
/usr/lib/libgssapi_krb5.so.2
#2  0x00929769 in kerb_authenticate_user (r=0xb85a1340) at
src/mod_auth_kerb.c:1390
...

The client code has sent slightly invalid ContextFlags for the reqFlags
field in the NegTokenInit (RFC 4178).

This is the sequence which causes the crash.  Code fragments are from
1.6.3.

In spnego_gss_accept_sec_context:


	if (*context_handle == GSS_C_NO_CONTEXT) {
		...
		ret = acc_ctx_new(minor_status, input_token,
				  context_handle, verifier_cred_handle,
				  &mechtok_in, &mic_in,
				  &negState, &return_token);
		if (ret != GSS_S_COMPLETE)
			goto cleanup;

The call to acc_ctx_new fails so the cleanup code is run:

cleanup:
	if (return_token != NO_TOKEN_SEND && return_token != CHECK_MIC)
{
		tmpret = make_spnego_tokenTarg_msg(negState,
sc->internal_mech,
						   &mechtok_out,
mic_out,
						   return_token,
						   output_token);

acc_ctx_new initialises return_token to ERROR_TOKEN_SEND and so the
cleanup code proceeds with the call to make_spnego_tokenTarg_msg.

At this point 'sc' has not been set, so it still NULL and the reference
to sc->internal_mech segfaults.

The acc_ctx_new call fails because get_req_flags rejects the ASN.1 bit
string sent by the client for the reqFlags.

I've tested with 1.7-alpha1 and this fails at the cleanup code with an
assertion failure:

		assert(sc != NULL);

This client code could cause any server using Kerberos/SPNEGO to fail.

Richard Evans

Subject: [krbdev.mit.edu #6402] Invalid initial GSSAPI/SPNEGO token can cause segmentation error or assert failure
Date: Thu, 12 Mar 2009 16:10:52 -0400
From: "Arlene Berry" <aberry@likewise.com>
To: <krb5-bugs@mit.edu>
RT-Send-Cc: 

This fixed 1.6.3 for us.  I changed the call so that it doesn't
dereference a NULL pointer and altered make_spnego_tokenTarg_msg to
allow for no mechanism since there may not be one when rejecting the
request.

--- spnego_mech.c	(revision xxxxx)
+++ spnego_mech.c	(working copy)
@@ -1269,7 +1269,8 @@
 	}
 cleanup:
 	if (return_token != NO_TOKEN_SEND && return_token != CHECK_MIC)
{
-		tmpret = make_spnego_tokenTarg_msg(negState,
sc->internal_mech,
+		tmpret = make_spnego_tokenTarg_msg(negState,
+                                      sc ? sc->internal_mech : NULL,
 						   &mechtok_out,
mic_out,
 						   return_token,
 						   output_token, 0);
@@ -2504,7 +2505,7 @@
 	 * If this is the initial token, include length of
 	 * mech_type and the negotiation result fields.
 	 */
-	if (sendtoken == INIT_TOKEN_SEND) {
+	if (sendtoken == INIT_TOKEN_SEND && mech_wanted) {
 		int mechlistTokenSize;
 		/*
 		 * 1 byte for the CONTEXT ID(0xa0),
@@ -2605,7 +2606,7 @@
 			goto errout;
 		}
 	}
-	if (sendtoken == INIT_TOKEN_SEND) {
+	if (sendtoken == INIT_TOKEN_SEND && mech_wanted) {
 		/*
 		 * Next, is the Supported MechType
 		 */

To: rt@krbdev.MIT.EDU
Subject: Re: [krbdev.mit.edu #6402] Invalid initial GSSAPI/SPNEGO token can cause segmentation error or assert failure
From: Tom Yu <tlyu@MIT.EDU>
Date: Thu, 12 Mar 2009 17:33:22 -0400
RT-Send-Cc: 

"" Arlene Berry " via RT" <rt-comment@krbdev.mit.edu> writes:

> This fixed 1.6.3 for us.  I changed the call so that it doesn't
> dereference a NULL pointer and altered make_spnego_tokenTarg_msg to
> allow for no mechanism since there may not be one when rejecting the
> request.

[...]

Actually, based on discussion on the krbdev list, I had come up with
this shorter patch.  Do you find any particular reasons to prefer one
over the other?

--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -1248,7 +1248,8 @@ spnego_gss_accept_sec_context(void *ct,
 				 &negState, &return_token);
 	}
 cleanup:
-	if (return_token != NO_TOKEN_SEND && return_token != CHECK_MIC) {
+	if (return_token == INIT_TOKEN_SEND ||
+	    return_token == CONT_TOKEN_SEND) {
 		tmpret = make_spnego_tokenTarg_msg(negState, sc->internal_mech,
 						   &mechtok_out, mic_out,
 						   return_token,

From: tlyu@mit.edu
Subject: SVN Commit


acc_ctx_new() can return an error condition without establishing a
SPNEGO context structure.  This can cause a null pointer dereference
in cleanup code in spnego_gss_accept_sec_context().

http://src.mit.edu/fisheye/changelog/krb5/?cs=22084
Commit By: tlyu
Revision: 22084
Changed Files:
U   trunk/src/lib/gssapi/spnego/spnego_mech.c

Committed fix.  Please test and review.  I accidentally created a new ticket instead
of updating
the existing one, but they are now merged.

From: tlyu@mit.edu
Subject: SVN Commit


pull up r22084 from trunk

acc_ctx_new() can return an error condition without establishing a
SPNEGO context structure.  This can cause a null pointer dereference
in cleanup code in spnego_gss_accept_sec_context().

http://src.mit.edu/fisheye/changelog/krb5/?cs=22099
Commit By: tlyu
Revision: 22099
Changed Files:
U   branches/krb5-1-7/src/lib/gssapi/spnego/spnego_mech.c

From: tlyu@mit.edu
Subject: SVN Commit


Apply revised patch from Apple that ensures that a REJECT token is
sent on error.

http://src.mit.edu/fisheye/changelog/krb5/?cs=22173
Commit By: tlyu
Revision: 22173
Changed Files:
U   trunk/src/lib/gssapi/spnego/spnego_mech.c

From: tlyu@mit.edu
Subject: SVN Commit


pull up r22173 from trunk

 ------------------------------------------------------------------------
 r22173 | tlyu | 2009-04-07 17:22:13 -0400 (Tue, 07 Apr 2009) | 4 lines
 Changed paths:
    M /trunk/src/lib/gssapi/spnego/spnego_mech.c

 ticket: 6417

 Apply revised patch from Apple that ensures that a REJECT token is
 sent on error.

http://src.mit.edu/fisheye/changelog/krb5/?cs=22222
Commit By: tlyu
Revision: 22222
Changed Files:
U   branches/krb5-1-7/src/lib/gssapi/spnego/spnego_mech.c