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 ; 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 >Number: 436 >Category: krb5-libs >Synopsis: recvauth() frees caller's auth_context. >Confidential: no >Severity: serious >Priority: medium >Responsible: tlyu >State: closed >Class: sw-bug >Submitter-Id: unknown >Arrival-Date: Wed Jun 11 20:53:01 EDT 1997 >Last-Modified: Sun Feb 22 21:34:59 EST 1998 >Originator: Donn Cave >Organization: University of Washington, University Computing Services >Release: 1.0pl1 >Environment: UNIX System: AIX melville 2 4 000010504900 >Description: 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. >How-To-Repeat: 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(). >Fix: 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; } >Audit-Trail: From: Donn Cave 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 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 To: Tom Yu 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. >Unformatted: