krb5 commit [krb5-1.16]: Fix memory leak in gss_add_cred() creation case

Greg Hudson ghudson at mit.edu
Tue Oct 30 12:25:49 EDT 2018


https://github.com/krb5/krb5/commit/a2dfb0be1c78ba9d6fef1e37b9c6e5be5787d31a
commit a2dfb0be1c78ba9d6fef1e37b9c6e5be5787d31a
Author: Greg Hudson <ghudson at mit.edu>
Date:   Thu Sep 13 11:29:46 2018 -0400

    Fix memory leak in gss_add_cred() creation case
    
    If gss_add_cred() is called with no input_cred_handle, it creates a
    new credential with one element.  At the end of the function, use the
    created credential as the output container, rather than creating a
    second one and leaking the first.
    
    Add a test program for gss_add_cred() and run it.
    
    (cherry picked from commit 9e32161dc307a323fd36fd59e252583fe7b90526)
    
    ticket: 8729
    version_fixed: 1.16.2

 .gitignore                               |    1 +
 src/lib/gssapi/mechglue/g_acquire_cred.c |    3 +
 src/tests/gssapi/Makefile.in             |   48 ++++++++-------
 src/tests/gssapi/t_add_cred.c            |   98 ++++++++++++++++++++++++++++++
 src/tests/gssapi/t_gssapi.py             |    6 +-
 5 files changed, 131 insertions(+), 25 deletions(-)

diff --git a/.gitignore b/.gitignore
index c13b5e3..f96ff9f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -423,6 +423,7 @@ local.properties
 /src/tests/gssapi/ccinit
 /src/tests/gssapi/ccrefresh
 /src/tests/gssapi/t_accname
+/src/tests/gssapi/t_add_cred
 /src/tests/gssapi/t_ccselect
 /src/tests/gssapi/t_ciflags
 /src/tests/gssapi/t_credstore
diff --git a/src/lib/gssapi/mechglue/g_acquire_cred.c b/src/lib/gssapi/mechglue/g_acquire_cred.c
index 9bd500b..5e82495 100644
--- a/src/lib/gssapi/mechglue/g_acquire_cred.c
+++ b/src/lib/gssapi/mechglue/g_acquire_cred.c
@@ -517,6 +517,9 @@ gss_add_cred_from(minor_status, input_cred_handle,
 	free(union_cred->mechs_array);
 	free(union_cred->cred_array);
 	new_union_cred = union_cred;
+    } else if (input_cred_handle == GSS_C_NO_CREDENTIAL) {
+	new_union_cred = union_cred;
+	*output_cred_handle = (gss_cred_id_t)new_union_cred;
     } else {
 	new_union_cred = malloc(sizeof (gss_union_cred_desc));
 	if (new_union_cred == NULL) {
diff --git a/src/tests/gssapi/Makefile.in b/src/tests/gssapi/Makefile.in
index 604f926..2463c23 100644
--- a/src/tests/gssapi/Makefile.in
+++ b/src/tests/gssapi/Makefile.in
@@ -9,9 +9,9 @@ LOCALINCLUDES = -I$(srcdir)/../../lib/gssapi/mechglue \
 	-I../../lib/gssapi/generic
 
 SRCS=	$(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c \
-	$(srcdir)/t_accname.c $(srcdir)/t_ccselect.c $(srcdir)/t_ciflags.c \
-	$(srcdir)/t_credstore.c $(srcdir)/t_enctypes.c $(srcdir)/t_err.c \
-	$(srcdir)/t_export_cred.c $(srcdir)/t_export_name.c \
+	$(srcdir)/t_accname.c $(srcdir)/t_add_cred.c $(srcdir)/t_ccselect.c \
+	$(srcdir)/t_ciflags.c $(srcdir)/t_credstore.c $(srcdir)/t_enctypes.c \
+	$(srcdir)/t_err.c $(srcdir)/t_export_cred.c $(srcdir)/t_export_name.c \
 	$(srcdir)/t_gssexts.c $(srcdir)/t_imp_cred.c $(srcdir)/t_imp_name.c \
 	$(srcdir)/t_invalid.c $(srcdir)/t_inq_cred.c $(srcdir)/t_inq_ctx.c \
 	$(srcdir)/t_inq_mechs_name.c $(srcdir)/t_iov.c \
@@ -20,31 +20,31 @@ SRCS=	$(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c \
 	$(srcdir)/t_s4u2proxy_krb5.c $(srcdir)/t_saslname.c \
 	$(srcdir)/t_spnego.c $(srcdir)/t_srcattrs.c
 
-OBJS=	ccinit.o ccrefresh.o common.o t_accname.o t_ccselect.o t_ciflags.o \
-	t_credstore.o t_enctypes.o t_err.o t_export_cred.o t_export_name.o \
-	t_gssexts.o t_imp_cred.o t_imp_name.o t_invalid.o t_inq_cred.o \
-	t_inq_ctx.o t_inq_mechs_name.o t_iov.o t_lifetime.o t_namingexts.o \
-	t_oid.o t_pcontok.o t_prf.o t_s4u.o t_s4u2proxy_krb5.o t_saslname.o \
-	t_spnego.o t_srcattrs.o
+OBJS=	ccinit.o ccrefresh.o common.o t_accname.o t_add_cred.o t_ccselect.o \
+	t_ciflags.o t_credstore.o t_enctypes.o t_err.o t_export_cred.o \
+	t_export_name.o t_gssexts.o t_imp_cred.o t_imp_name.o t_invalid.o \
+	t_inq_cred.o t_inq_ctx.o t_inq_mechs_name.o t_iov.o t_lifetime.o \
+	t_namingexts.o t_oid.o t_pcontok.o t_prf.o t_s4u.o t_s4u2proxy_krb5.o \
+	t_saslname.o t_spnego.o t_srcattrs.o
 
 COMMON_DEPS= common.o $(GSS_DEPLIBS) $(KRB5_BASE_DEPLIBS)
 COMMON_LIBS= common.o $(GSS_LIBS) $(KRB5_BASE_LIBS)
 
-all: ccinit ccrefresh t_accname t_ccselect t_ciflags t_credstore t_enctypes \
-	t_err t_export_cred t_export_name t_gssexts t_imp_cred t_imp_name \
-	t_invalid t_inq_cred t_inq_ctx t_inq_mechs_name t_iov t_lifetime \
-	t_namingexts t_oid t_pcontok t_prf t_s4u t_s4u2proxy_krb5 t_saslname \
-	t_spnego t_srcattrs
+all: ccinit ccrefresh t_accname t_add_cred t_ccselect t_ciflags t_credstore \
+	t_enctypes t_err t_export_cred t_export_name t_gssexts t_imp_cred \
+	t_imp_name t_invalid t_inq_cred t_inq_ctx t_inq_mechs_name t_iov \
+	t_lifetime t_namingexts t_oid t_pcontok t_prf t_s4u t_s4u2proxy_krb5 \
+	t_saslname t_spnego t_srcattrs
 
 check-unix: t_oid
 	$(RUN_TEST) ./t_invalid
 	$(RUN_TEST) ./t_oid
 	$(RUN_TEST) ./t_prf
 
-check-pytests: ccinit ccrefresh t_accname t_ccselect t_ciflags t_credstore \
-	t_enctypes t_err t_export_cred t_export_name t_imp_cred t_inq_cred \
-	t_inq_ctx t_inq_mechs_name t_iov t_lifetime t_pcontok t_s4u \
-	t_s4u2proxy_krb5 t_spnego t_srcattrs
+check-pytests: ccinit ccrefresh t_accname t_add_cred t_ccselect t_ciflags \
+	t_credstore t_enctypes t_err t_export_cred t_export_name t_imp_cred \
+	t_inq_cred t_inq_ctx t_inq_mechs_name t_iov t_lifetime t_pcontok \
+	t_s4u t_s4u2proxy_krb5 t_spnego t_srcattrs
 	$(RUNPYTEST) $(srcdir)/t_gssapi.py $(PYTESTFLAGS)
 	$(RUNPYTEST) $(srcdir)/t_ccselect.py $(PYTESTFLAGS)
 	$(RUNPYTEST) $(srcdir)/t_client_keytab.py $(PYTESTFLAGS)
@@ -59,6 +59,8 @@ ccrefresh: ccrefresh.o $(KRB5_BASE_DEPLIBS)
 	$(CC_LINK) -o ccrefresh ccrefresh.o $(KRB5_BASE_LIBS)
 t_accname: t_accname.o $(COMMON_DEPS)
 	$(CC_LINK) -o $@ t_accname.o $(COMMON_LIBS)
+t_add_cred: t_add_cred.o $(COMMON_DEPS)
+	$(CC_LINK) -o $@ t_add_cred.o $(COMMON_LIBS)
 t_ccselect: t_ccselect.o $(COMMON_DEPS)
 	$(CC_LINK) -o $@ t_ccselect.o $(COMMON_LIBS)
 t_ciflags: t_ciflags.o $(COMMON_DEPS)
@@ -111,8 +113,8 @@ t_srcattrs: t_srcattrs.o $(COMMON_DEPS)
 	$(CC_LINK) -o $@ t_srcattrs.o $(COMMON_LIBS)
 
 clean:
-	$(RM) ccinit ccrefresh t_accname t_ccselect t_ciflags t_credstore
-	$(RM) t_enctypes t_err t_export_cred t_export_name t_gssexts t_imp_cred
-	$(RM) t_imp_name t_invalid t_inq_cred t_inq_ctx t_inq_mechs_name t_iov
-	$(RM) t_lifetime t_namingexts t_oid t_pcontok t_prf t_s4u
-	$(RM) t_s4u2proxy_krb5 t_saslname t_spnego t_srcattrs
+	$(RM) ccinit ccrefresh t_accname t_add_cred t_ccselect t_ciflags
+	$(RM) t_credstore t_enctypes t_err t_export_cred t_export_name
+	$(RM) t_gssexts t_imp_cred t_imp_name t_invalid t_inq_cred t_inq_ctx
+	$(RM) t_inq_mechs_name t_iov t_lifetime t_namingexts t_oid t_pcontok
+	$(RM) t_prf t_s4u t_s4u2proxy_krb5 t_saslname t_spnego t_srcattrs
diff --git a/src/tests/gssapi/t_add_cred.c b/src/tests/gssapi/t_add_cred.c
new file mode 100644
index 0000000..d59fde9
--- /dev/null
+++ b/src/tests/gssapi/t_add_cred.c
@@ -0,0 +1,98 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* tests/gssapi/t_add_cred.c - gss_add_cred() tests */
+/*
+ * Copyright (C) 2018 by the Massachusetts Institute of Technology.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * This program tests the mechglue behavior of gss_add_cred().  It relies on a
+ * krb5 keytab and credentials being present so that initiator and acceptor
+ * credentials can be acquired, but does not use them to initiate or accept any
+ * requests.
+ */
+
+#include <stdio.h>
+#include <assert.h>
+
+#include "common.h"
+
+int
+main()
+{
+    OM_uint32 minor, major;
+    gss_cred_id_t cred1;
+    gss_cred_usage_t usage;
+
+    /* Check that we get the expected error if we pass neither an input nor an
+     * output cred handle. */
+    major = gss_add_cred(&minor, GSS_C_NO_CREDENTIAL, GSS_C_NO_NAME,
+                         &mech_krb5, GSS_C_INITIATE, GSS_C_INDEFINITE,
+                         GSS_C_INDEFINITE, NULL, NULL, NULL, NULL);
+    assert(major == (GSS_S_CALL_INACCESSIBLE_WRITE | GSS_S_NO_CRED));
+
+    /* Create cred1 with a krb5 initiator cred by passing an output handle but
+     * no input handle. */
+    major = gss_add_cred(&minor, GSS_C_NO_CREDENTIAL, GSS_C_NO_NAME,
+                         &mech_krb5, GSS_C_INITIATE, GSS_C_INDEFINITE,
+                         GSS_C_INDEFINITE, &cred1, NULL, NULL, NULL);
+    assert(major == GSS_S_COMPLETE);
+
+    /* Verify that cred1 has the expected mechanism creds. */
+    major = gss_inquire_cred_by_mech(&minor, cred1, &mech_krb5, NULL, NULL,
+                                     NULL, &usage);
+    assert(major == GSS_S_COMPLETE && usage == GSS_C_INITIATE);
+    major = gss_inquire_cred_by_mech(&minor, cred1, &mech_iakerb, NULL, NULL,
+                                     NULL, &usage);
+    assert(major == GSS_S_NO_CRED);
+
+    /* Check that we get the expected error if we try to add another krb5 mech
+     * cred to cred1. */
+    major = gss_add_cred(&minor, cred1, GSS_C_NO_NAME, &mech_krb5,
+                         GSS_C_INITIATE, GSS_C_INDEFINITE, GSS_C_INDEFINITE,
+                         NULL, NULL, NULL, NULL);
+    assert(major == GSS_S_DUPLICATE_ELEMENT);
+
+    /* Add an IAKERB acceptor mech cred to cred1. */
+    major = gss_add_cred(&minor, cred1, GSS_C_NO_NAME, &mech_iakerb,
+                         GSS_C_ACCEPT, GSS_C_INDEFINITE, GSS_C_INDEFINITE,
+                         NULL, NULL, NULL, NULL);
+    assert(major == GSS_S_COMPLETE);
+
+    /* Verify cred1 mechanism creds. */
+    major = gss_inquire_cred_by_mech(&minor, cred1, &mech_krb5, NULL, NULL,
+                                     NULL, &usage);
+    assert(major == GSS_S_COMPLETE && usage == GSS_C_INITIATE);
+    major = gss_inquire_cred_by_mech(&minor, cred1, &mech_iakerb, NULL, NULL,
+                                     NULL, &usage);
+    assert(major == GSS_S_COMPLETE && usage == GSS_C_ACCEPT);
+
+    gss_release_cred(&minor, &cred1);
+
+    return 0;
+}
diff --git a/src/tests/gssapi/t_gssapi.py b/src/tests/gssapi/t_gssapi.py
index 6da5fce..077d266 100755
--- a/src/tests/gssapi/t_gssapi.py
+++ b/src/tests/gssapi/t_gssapi.py
@@ -9,9 +9,11 @@ for realm in multipass_realms():
     realm.run(['./t_iov', '-s', 'p:' + realm.host_princ])
     realm.run(['./t_pcontok', 'p:' + realm.host_princ])
 
-### Test acceptor name behavior.
-
+# Test gss_add_cred().
 realm = K5Realm()
+realm.run(['./t_add_cred'])
+
+### Test acceptor name behavior.
 
 # Create some host-based principals and put most of them into the
 # keytab.  Rename one principal so that the keytab name matches the


More information about the cvs-krb5 mailing list