Ticket History #436: recvauth() frees caller's auth_context.

From root@melville.u.washington.edu Wed Jun 11 20:52:23 1997
Received: from MIT.EDU (PACIFIC-CARRIER-ANNEX.MIT.EDU [18.69.0.28]) by rt-11.MIT.EDU (8.7.5/8.7.3) with SMTP id UAA18026 for <bugs@RT-11.MIT.EDU>; Wed, 11 Jun 1997 20:52:23 -0400
Received: from melville.u.washington.edu by MIT.EDU with SMTP
id AA07338; Wed, 11 Jun 97 20:51:24 EDT
Received: (from root@localhost)
by melville.u.washington.edu (8.8.4+UW97.04/8.8.4+UW97.05)
id RAA18038; Wed, 11 Jun 1997 17:52:21 -0700
Message-Id: <199706120052.RAA18038@melville.u.washington.edu>
Date: Wed, 11 Jun 1997 17:52:21 -0700
From: donn@u.washington.edu
Reply-To: donn@u.washington.edu
To: krb5-bugs@MIT.EDU
Subject: recvauth() frees caller's auth_context.
X-Send-Pr-Version: 3.99

University of Washington, University Computing Services
UNIX
System: AIX melville 2 4 000010504900


recvauth() frees auth_context before returning, if an error occurred.
It does regardless of whether it's the caller's storage passed in as
a parameter, or storage allocated by recvauth() itself. In the latter
case it arguably makes sense. In the former it doesn't make sense,
at least not to me.
Allocate a krb5_auth_context object, with krb5_auth_con_init(), and
pass its address to recvauth(). Connect with a client whose server
principal was constructed using the local host instead of the service
host; this is an error and will result in ``Wrong principal in
request.'' Then, after return from recvauth(), free your auth_context
object with auth_con_free().
Context diff follows. I just saved the initial parameter value, and
only free the object if it was allocated locally.
--------
*** src/lib/krb5/krb/recvauth.c.dist Thu Nov 21 11:00:06 1996
--- src/lib/krb5/krb/recvauth.c Wed Jun 11 16:28:51 1997
***************
*** 35,47 ****
static char *sendauth_version = "KRB5_SENDAUTH_V1.0";

krb5_error_code
krb5_recvauth(context, auth_context,
/* IN */
fd, appl_version, server, flags, keytab,
/* OUT */
ticket)
krb5_context context;
! krb5_auth_context * auth_context;
krb5_pointer fd;
char * appl_version;
krb5_principal server;
--- 35,47 ----
static char *sendauth_version = "KRB5_SENDAUTH_V1.0";

krb5_error_code
krb5_recvauth(context, auth_context_arg,
/* IN */
fd, appl_version, server, flags, keytab,
/* OUT */
ticket)
krb5_context context;
! krb5_auth_context * auth_context_arg;
krb5_pointer fd;
char * appl_version;
krb5_principal server;
***************
*** 50,55 ****
--- 50,56 ----
krb5_ticket ** ticket;
{
krb5_auth_context new_auth_context;
+ krb5_auth_context *auth_context;
krb5_flags ap_option;
krb5_error_code retval, problem;
krb5_data inbuf;
***************
*** 138,143 ****
--- 139,145 ----
if ((retval = krb5_read_message(context, fd, &inbuf)))
return retval;

+ auth_context = auth_context_arg;
if (*auth_context == NULL) {
problem = krb5_auth_con_init(context, &new_auth_context);
*auth_context = new_auth_context;
***************
*** 229,235 ****
if (retval) {
if (rcache)
krb5_rc_close(context, rcache);
! krb5_auth_con_free(context, *auth_context);
}
return retval;
}
--- 231,238 ----
if (retval) {
if (rcache)
krb5_rc_close(context, rcache);
! if (auth_context != auth_context_arg)
! krb5_auth_con_free(context, *auth_context);
}
return retval;
}

From: Donn Cave <donn@u.washington.edu>
To: krb5-bugs@MIT.EDU, krb5-unassigned@RT-11.MIT.EDU
Cc: Subject: Re: krb5-libs/436: recvauth() frees caller's auth_context.
Date: Thu, 12 Jun 1997 17:21:47 -0800

| Thank you very much for your problem report.
| It has the internal identification `krb5-libs/436'.
| The individual assigned to look at your
| report is: krb5-unassigned.

Dear mr. or ms. krb5-unassigned:

I'm sorry to say that the patch I suggested wasn't quite right.
In the cleanup clause in krb5_recvauth(), I suggested that the
krb5_auth_con_free() be conditional on whether the auth_context
object had been allocated by krb5_recvauth() itself.

The krb5_rc_close() should go either along with that test,
and only happen after local allocation of auth_context, or
auth_context->rcache should be cleared after close.

I have to clean this up for a Python module that supports an auth_context
Python object class and can call krb5_recvauth(). In a reasonably
straightforward C application, it may be no big deal to accommodate
quirks like this, but the translation between C and Python's reference
based object system is less flexible in this way. So I have my own
parallel version of krb5_recvauth(), for now.

Donn Cave, University Computing Services, University of Washington
donn@u.washington.edu

Responsible-Changed-From-To: krb5-unassigned->tlyu
Responsible-Changed-By: tlyu
Responsible-Changed-When: Thu Feb 19 18:41:24 1998
Responsible-Changed-Why:
I'm dealing now.

State-Changed-From-To: open-feedback
State-Changed-By: tlyu
State-Changed-When: Thu Feb 19 18:41:40 1998
State-Changed-Why:
Should be fixed now...



From: Tom Yu <tlyu@MIT.EDU>
To: donn@u.washington.edu
Cc: krb5-bugs@MIT.EDU
Subject: Re: krb5-libs/436: recvauth() frees caller's auth_context.
Date: Thu, 19 Feb 1998 19:05:44 -0500

Sorry for not getting back to you earlier... it does appear that we
have some problems in the cleanup logic. Thanks for pointing that
out. Please let me know if the following patch works for you.

---Tom

--- recvauth.c 1997/02/06 02:28:43 5.30
+++ recvauth.c 1998/02/20 00:03:04
@@ -58,6 +58,7 @@
krb5_octet response;
krb5_data null_server;
int need_error_free = 0;
+ int local_rcache = 0, local_authcon = 0;

/*
* Zero out problem variable. If problem is set at the end of
@@ -141,8 +142,10 @@
if (*auth_context == NULL) {
problem = krb5_auth_con_init(context, &new_auth_context);
*auth_context = new_auth_context;
+ local_authcon = 1;
}
- if ((!problem) && ((*auth_context)->rcache == NULL)) {
+ krb5_auth_con_getrcache(context, *auth_context, &rcache);
+ if ((!problem) && rcache == NULL) {
/*
* Setup the replay cache.
*/
@@ -156,6 +159,7 @@
}
if (!problem)
problem = krb5_auth_con_setrcache(context, *auth_context, rcache);
+ local_rcache = 1;
}
if (!problem) {
problem = krb5_rd_req(context, auth_context, &inbuf, server,
@@ -227,9 +231,12 @@

cleanup:;
if (retval) {
- if (rcache)
+ if (local_authcon) {
+ krb5_auth_con_free(context, *auth_context);
+ } else if (local_rcache && rcache != NULL) {
krb5_rc_close(context, rcache);
- krb5_auth_con_free(context, *auth_context);
+ krb5_auth_con_setrcache(context, *auth_context, NULL);
+ }
}
return retval;
}

From: Donn Cave <donn@u.washington.edu>
To: Tom Yu <tlyu@MIT.EDU>
Cc: krb5-bugs@MIT.EDU
Subject: Re: krb5-libs/436: recvauth() frees caller's auth_context.
Date: Fri, 20 Feb 1998 10:12:28 -0800 (PST)

On Thu, 19 Feb 1998, Tom Yu wrote:
| Sorry for not getting back to you earlier... it does appear that we
| have some problems in the cleanup logic. Thanks for pointing that
| out. Please let me know if the following patch works for you.

It appears I can no longer duplicate the original problem, but
for what it's worth, your revision does work OK. Thanks,

Donn Cave, University Computing Services, University of Washington
donn@u.washington.edu

State-Changed-From-To: feedback-closed
State-Changed-By: tlyu
State-Changed-When: Sun Feb 22 21:34:43 1998
State-Changed-Why:

Fixed.