From krb5-bugs-incoming-bounces@PCH.MIT.EDU  Mon Oct  1 17:32:12 2007
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (8.12.9) with
ESMTP
	id l91LWCHW014206; Mon, 1 Oct 2007 17:32:12 -0400 (EDT)
Received: from pch.mit.edu (pch.mit.edu [127.0.0.1])
	by pch.mit.edu (8.13.6/8.12.8) with ESMTP id l91LW7SQ006230;
	Mon, 1 Oct 2007 17:32:07 -0400
Received: from pacific-carrier-annex.mit.edu (PACIFIC-CARRIER-ANNEX.MIT.EDU
	[18.7.21.83])
	by pch.mit.edu (8.13.6/8.12.8) with ESMTP id l91JahSn005104
	for <krb5-bugs-incoming@PCH.mit.edu>; Mon, 1 Oct 2007 15:36:43 -0400
Received: from mit.edu (W92-130-BARRACUDA-2.MIT.EDU [18.7.21.223])
	by pacific-carrier-annex.mit.edu (8.13.6/8.9.2) with ESMTP id
	l91Jaam9025540
	for <krb5-bugs@mit.edu>; Mon, 1 Oct 2007 15:36:36 -0400 (EDT)
Received: from mx1.redhat.com (mx1.redhat.com [66.187.233.31])
	by mit.edu (Spam Firewall) with ESMTP id C1F6A82DCA6
	for <krb5-bugs@mit.edu>; Mon,  1 Oct 2007 15:36:35 -0400 (EDT)
Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com
	[172.16.52.254])
	by mx1.redhat.com (8.13.1/8.13.1) with ESMTP id l91JaZwv023621
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO)
	for <krb5-bugs@mit.edu>; Mon, 1 Oct 2007 15:36:35 -0400
Received: from blade.boston.redhat.com (blade.boston.redhat.com [172.16.80.50])
	by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l91JaXjV006047
	for <krb5-bugs@mit.edu>; Mon, 1 Oct 2007 15:36:34 -0400
Received: from blade.boston.redhat.com (localhost.localdomain [127.0.0.1])
	by blade.boston.redhat.com (8.14.1/8.14.1) with ESMTP id l91JaXcf021976
	for <krb5-bugs@mit.edu>; Mon, 1 Oct 2007 15:36:33 -0400
Received: (from nalin@localhost)
	by blade.boston.redhat.com (8.14.1/8.14.1/Submit) id l91JaXDM021975;
	Mon, 1 Oct 2007 15:36:33 -0400
Date: Mon, 1 Oct 2007 15:36:33 -0400
Message-Id: <200710011936.l91JaXDM021975@blade.boston.redhat.com>
To: krb5-bugs@mit.edu
From: nalin@redhat.com
X-send-pr-version: 3.99
X-Spam-Score: 1.50
X-Spam-Level: * (1.50)
X-Spam-Flag: NO
X-Scanned-By: MIMEDefang 2.42
X-Mailman-Approved-At: Mon, 01 Oct 2007 17:32:05 -0400
X-BeenThere: krb5-bugs-incoming@mailman.mit.edu
X-Mailman-Version: 2.1.6
Precedence: list
Reply-To: nalin@redhat.com
Sender: krb5-bugs-incoming-bounces@PCH.MIT.EDU
Errors-To: krb5-bugs-incoming-bounces@PCH.MIT.EDU


>Submitter-Id:	net
>Originator:
>Organization:
Nalin
>Confidential:	no
>Synopsis:	libgssapi mechglue doesn't always store delegated credentials
>Severity:	non-critical
>Priority:	medium
>Category:	krb5-libs
>Class:		sw-bug
>Release:	1.6.2
>Environment:

System: Linux blade.boston.redhat.com 2.6.23-0.211.rc8.git2.fc8 #1 SMP Thu Sep 27
18:21:00 EDT 2007 x86_64 x86_64 x86_64 GNU/Linux
Architecture: x86_64

>Description:
	The mechglue wrapper for gss_init_sec_context doesn't appear to be
	handling the ret_flags parameter correctly.
	* If the caller passes in NULL, the mechanism-specific function can't
	  store its own returned flags, so the wrapper can't dependably
	  be expected to store delegated credentials.
	* The wrapper also appears to be checking for the bit in ret_flags
	  (which is a pointer to a bitfield) incorrectly.
>How-To-Repeat:
	Using elinks to perform negotiate authentication to a patched
	mod_auth_kerb (both found in RHEL, if that helps), this happens
	pretty reliably.
>Fix:
	Proposed patch:

Index: src/lib/gssapi/mechglue/g_accept_sec_context.c
===================================================================
--- src/lib/gssapi/mechglue/g_accept_sec_context.c	(revision 20038)
+++ src/lib/gssapi/mechglue/g_accept_sec_context.c	(working copy)
@@ -112,6 +112,7 @@

 {
     OM_uint32		status, temp_status, temp_minor_status;
+    OM_uint32		temp_ret_flags = 0;
     gss_union_ctx_id_t	union_ctx_id;
     gss_union_cred_t	union_cred;
     gss_cred_id_t	input_cred_handle = GSS_C_NO_CREDENTIAL;
@@ -202,7 +203,7 @@
 						  &internal_name,
 						  mech_type,
 						  output_token,
-						  ret_flags,
+						  &temp_ret_flags,
 						  time_rec,
 					d_cred ? &tmp_d_cred : NULL);

@@ -248,7 +249,7 @@
 	    }

 	    /* Ensure we're returning correct creds format */
-	    if ((ret_flags && GSS_C_DELEG_FLAG) &&
+	    if ((temp_ret_flags & GSS_C_DELEG_FLAG) &&
 		tmp_d_cred != GSS_C_NO_CREDENTIAL) {
 		gss_union_cred_t d_u_cred = NULL;

@@ -335,6 +336,8 @@
 	    if (src_name == NULL && tmp_src_name != NULL)
 		(void) gss_release_name(&temp_minor_status,
 					&tmp_src_name);
+	    if (ret_flags != NULL)
+		*ret_flags = temp_ret_flags;
 	    return	(status);
     } else {

Date: Mon, 1 Oct 2007 17:35:29 -0400
From: Nalin Dahyabhai <nalin@redhat.com>
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #5802] AutoReply: libgssapi mechglue doesn't always store delegated credentials
RT-Send-Cc: 

On second thought, the proposed change will always store delegated
credentials, even when the calling application didn't pass in ret_flags,
and therefore might not care.  A replacement for just the second hunk:

> @@ -202,7 +203,7 @@
>  						  &internal_name,
>  						  mech_type,
>  						  output_token,
> -						  ret_flags,
> +						  ret_flags ? &temp_ret_flags : NULL,
>  						  time_rec,
>  					d_cred ? &tmp_d_cred : NULL);
>

To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #5802] AutoReply: libgssapi mechglue doesn't always store delegated credentials
From: Tom Yu <tlyu@MIT.EDU>
Date: Tue, 02 Oct 2007 18:17:52 -0400
RT-Send-Cc: 

Is the application passing in non-null deleg_cred_handle but null
ret_flags?  I would suspect that the right thing to do is to actually
have accept_sec_context() fill in the cred handle but skip storing the
flags.

For these reasons I think the first patch is probably right.

From: tlyu@mit.edu
Subject: SVN Commit

Apply patch from Nalin Dahyabhai to correctly handle delegated
credentials if ret_flags is null.


Commit By: tlyu



Revision: 20082
Changed Files:
_U  trunk/
U   trunk/src/lib/gssapi/mechglue/g_accept_sec_context.c

From: tlyu@mit.edu
Subject: SVN Commit

pull up r20082 from trunk

 r20082@cathode-dark-space:  tlyu | 2007-10-02 20:17:27 -0400
 ticket: 5802
 target_version: 1.6.3
 tags: pullup

 Apply patch from Nalin Dahyabhai to correctly handle delegated
 credentials if ret_flags is null.




Commit By: tlyu



Revision: 20083
Changed Files:
_U  branches/krb5-1-6/
U   branches/krb5-1-6/src/lib/gssapi/mechglue/g_accept_sec_context.c

Date: Wed, 3 Oct 2007 08:24:32 -0400
From: Nalin Dahyabhai <nalin@redhat.com>
To: Tom Yu via RT <rt-comment@krbdev.mit.edu>
Subject: Re: [krbdev.mit.edu #5802] AutoReply: libgssapi mechglue doesn't always store delegated credentials
RT-Send-Cc: 

On Tue, Oct 02, 2007 at 06:18:05PM -0400, Tom Yu via RT wrote:
> Is the application passing in non-null deleg_cred_handle but null
> ret_flags?

It was (mod_auth_kerb).

>            I would suspect that the right thing to do is to actually
> have accept_sec_context() fill in the cred handle but skip storing the
> flags.
>
> For these reasons I think the first patch is probably right.

Makes sense.  I was figuring that a null ret_flags might be taken as a
hint that the calling application wouldn't "know" that it needed to
dispose of delegated credentials, so we'd be leaking memory in some
cases, but I can believe that that's a problem with the application.

Thanks,

Nalin