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

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]> 
https://github.com/krb5/krb5/commit/e058595730c57032b84bacbb00806a95fd5eb470
commit e058595730c57032b84bacbb00806a95fd5eb470
Author: Greg Hudson <[email protected]>
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.15.4

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

diff --git a/.gitignore b/.gitignore
index f9e8512..979e3fb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -422,6 +422,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 6c14642..afea18c 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 \
@@ -19,28 +19,28 @@ SRCS=	$(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c \
 	$(srcdir)/t_prf.c $(srcdir)/t_s4u.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_namingexts.o t_oid.o \
-	t_pcontok.o t_prf.o t_s4u.o t_s4u2proxy_krb5.o t_saslname.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_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_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_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 \
+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_pcontok t_s4u t_s4u2proxy_krb5 \
 	t_spnego t_srcattrs
@@ -58,6 +58,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)
@@ -108,8 +110,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_namingexts t_oid t_pcontok t_prf t_s4u t_s4u2proxy_krb5
-	$(RM) 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_namingexts t_oid t_pcontok t_prf t_s4u
+	$(RM) 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 e23c936..5d452eb 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.