pull up r20559 from trunk
authorTom Yu <tlyu@mit.edu>
Fri, 25 Jul 2008 20:32:56 +0000 (20:32 +0000)
committerTom Yu <tlyu@mit.edu>
Fri, 25 Jul 2008 20:32:56 +0000 (20:32 +0000)
 r20559@cathode-dark-space:  jaltman | 2008-07-21 16:47:35 -0400
 ticket: 5442
 tags: pullup

 This patch addresses the issues raised in this ticket and ticket 5936.

 (a) In the case where 'cred_handle' != 'verifier_cred_handle'[1]
 krb5_gss_accept_sec_context() leaks the 'cred_handle' in the success
 case and the failure cases that result in returning from the function
 prior to reaching the end of the function.

 (b) The meaningful 'minor_status' return value is destroyed during the
 cleanup operations.

 The approach taken is to add a new 'exit:' label prior to the end of the
 function through which all function returns after reaching the 'fail:'
 label will goto.  After 'exit:', the 'cred_handle' will be released and
 if there is a krb5_context 'context' to be freed, the error info will be
 saved and krb5_free_context() will be called.

 In the success case, the krb5_context is saved in the gss context and we
 now set 'context' to NULL to prevent it from being freed.

 In order to preserve the minor_status return code, a 'tmp_minor_status'
 variable is added that is used after the 'fail:' label in calls to
 krb5_gss_delete_sec_context() and krb5_gss_release_cred().

 [1] If 'verifier_cred_handle' is non-NULL, then 'cred_handle' is set to
 the value of 'verifier_cred_handle'.

ticket: 5442

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/krb5-1-6@20581 dc483132-0cff-0310-8789-dd5450dbe970

src/lib/gssapi/krb5/accept_sec_context.c

index 7dcdd1a51a5b895f7e481ef4f6256f8a5b075606..bb0e2051a33573ff5e2a0ef813c6bd4157d52844 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000, 2004  by the Massachusetts Institute of Technology.
+ * Copyright 2000, 2004, 2008 by the Massachusetts Institute of Technology.
  * All Rights Reserved.
  *
  * Export of this software from the United States of America may
@@ -249,6 +249,7 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
    krb5_data option;
    const gss_OID_desc *mech_used = NULL;
    OM_uint32 major_status = GSS_S_FAILURE;
+   OM_uint32 tmp_minor_status;
    krb5_error krb_error_data;
    krb5_data scratch;
    gss_cred_id_t cred_handle = NULL;
@@ -903,13 +904,14 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
 
    if (!GSS_ERROR(major_status) && major_status != GSS_S_CONTINUE_NEEDED) {
        ctx->k5_context = context;
-       return(major_status);
+       context = NULL;
+       goto exit;
    }
 
    /* from here on is the real "fail" code */
 
    if (ctx)
-       (void) krb5_gss_delete_sec_context(minor_status, 
+       (void) krb5_gss_delete_sec_context(&tmp_minor_status, 
                                          (gss_ctx_id_t *) &ctx, NULL);
    if (deleg_cred) { /* free memory associated with the deleg credential */
        if (deleg_cred->ccache)
@@ -936,10 +938,9 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
    if (decode_req_message) {
        krb5_ap_req     * request;
           
-       if (decode_krb5_ap_req(&ap_req, &request)) {
-          krb5_free_context(context);
-          return (major_status);
-       }
+       if (decode_krb5_ap_req(&ap_req, &request))
+          goto exit;
+
        if (request->ap_options & AP_OPTS_MUTUAL_REQUIRED)
           gss_flags |= GSS_C_MUTUAL_FLAG;
        krb5_free_ap_req(context, request);
@@ -967,20 +968,16 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
        krb_error_data.server = cred->princ;
 
        code = krb5_mk_error(context, &krb_error_data, &scratch);
-       if (code) {
-          krb5_free_context(context);
-          return (major_status);
-       }
+       if (code)
+           goto exit;
 
        tmsglen = scratch.length;
        toktype = KG_TOK_CTX_ERROR;
 
        token.length = g_token_size(mech_used, tmsglen);
        token.value = (unsigned char *) xmalloc(token.length);
-       if (!token.value) {
-          krb5_free_context(context);
-          return (major_status);
-       }
+       if (!token.value) 
+           goto exit;
 
        ptr = token.value;
        g_make_token_header(mech_used, tmsglen, &ptr, toktype);
@@ -990,9 +987,13 @@ krb5_gss_accept_sec_context(minor_status, context_handle,
 
        *output_token = token;
    }
+
+  exit:
    if (!verifier_cred_handle && cred_handle) {
-          krb5_gss_release_cred(minor_status, &cred_handle);
+       krb5_gss_release_cred(&tmp_minor_status, &cred_handle);
+   }
+   if (context) {
+       krb5_free_context(context);
    }
-   krb5_free_context(context);
    return (major_status);
 }