krb5 commit: Unregister thread key in SPNEGO finalization

Greg Hudson ghudson at mit.edu
Tue Oct 20 19:27:49 EDT 2020


https://github.com/krb5/krb5/commit/07ff54d0bb85109df114612bbbfa6559f4a1e0cb
commit 07ff54d0bb85109df114612bbbfa6559f4a1e0cb
Author: Greg Hudson <ghudson at mit.edu>
Date:   Fri Oct 16 11:35:18 2020 -0400

    Unregister thread key in SPNEGO finalization
    
    Commit d160bc733a3dbeb6d84f4e175234ff18738d9f66 (ticket 7045) added a
    new thread key K5_KEY_GSS_SPNEGO_STATUS and registered it in SPNEGO
    library initialization, but neglected to unregister it in
    finalization.  As a result, loading, unloading, and reloading
    libgssapi_krb5 could throw an assertion failure if libkrb5support
    remained loaded.  Unregister the key in SPNEGO finalization and add a
    test case.
    
    Reported and investigated by Adam Dabrowski.
    
    ticket: 8614
    tags: pullup
    target_version: 1.18-next
    target_version: 1.17-next

 .gitignore                          |    1 +
 src/lib/gssapi/spnego/spnego_mech.c |    1 +
 src/tests/gssapi/Makefile.in        |   34 ++++++++------
 src/tests/gssapi/deps               |    2 +
 src/tests/gssapi/reload.c           |   83 +++++++++++++++++++++++++++++++++++
 5 files changed, 106 insertions(+), 15 deletions(-)

diff --git a/.gitignore b/.gitignore
index 6bcc57e..445c559 100644
--- a/.gitignore
+++ b/.gitignore
@@ -468,6 +468,7 @@ local.properties
 
 /src/tests/gssapi/ccinit
 /src/tests/gssapi/ccrefresh
+/src/tests/gssapi/reload
 /src/tests/gssapi/t_accname
 /src/tests/gssapi/t_add_cred
 /src/tests/gssapi/t_bindings
diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index 81d2400..d74dc2b 100644
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -325,6 +325,7 @@ int gss_spnegoint_lib_init(void)
 
 void gss_spnegoint_lib_fini(void)
 {
+	k5_key_delete(K5_KEY_GSS_SPNEGO_STATUS);
 }
 
 static OM_uint32
diff --git a/src/tests/gssapi/Makefile.in b/src/tests/gssapi/Makefile.in
index 68c132b..828c2b7 100644
--- a/src/tests/gssapi/Makefile.in
+++ b/src/tests/gssapi/Makefile.in
@@ -9,10 +9,10 @@ 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_add_cred.c $(srcdir)/t_bindings.c \
-	$(srcdir)/t_ccselect.c $(srcdir)/t_ciflags.c $(srcdir)/t_context.c \
-	$(srcdir)/t_credstore.c $(srcdir)/t_enctypes.c $(srcdir)/t_err.c \
-	$(srcdir)/t_export_cred.c $(srcdir)/t_export_name.c \
+	$(srcdir)/reload.c $(srcdir)/t_accname.c $(srcdir)/t_add_cred.c \
+	$(srcdir)/t_bindings.c $(srcdir)/t_ccselect.c $(srcdir)/t_ciflags.c \
+	$(srcdir)/t_context.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 \
@@ -21,12 +21,13 @@ 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_add_cred.o t_bindings.o \
-	t_ccselect.o t_ciflags.o t_context.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 reload.o t_accname.o t_add_cred.o \
+	t_bindings.o t_ccselect.o t_ciflags.o t_context.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)
@@ -37,11 +38,12 @@ all: ccinit ccrefresh t_accname t_add_cred t_bindings t_ccselect t_ciflags \
 	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
+check-unix: t_oid reload
 	$(RUN_TEST) ./t_invalid
 	$(RUN_TEST) ./t_oid
 	$(RUN_TEST) ./t_prf
 	$(RUN_TEST) ./t_imp_name
+	if [ -r $(TOPLIBD)/libgssapi_krb5.so ]; then $(RUN_TEST) ./reload; fi
 
 check-pytests: ccinit ccrefresh t_accname t_add_cred t_bindings t_ccselect \
 	t_ciflags t_context t_credstore t_enctypes t_err t_export_cred \
@@ -61,6 +63,8 @@ ccinit: ccinit.o $(KRB5_BASE_DEPLIBS)
 	$(CC_LINK) -o ccinit ccinit.o $(KRB5_BASE_LIBS)
 ccrefresh: ccrefresh.o $(KRB5_BASE_DEPLIBS)
 	$(CC_LINK) -o ccrefresh ccrefresh.o $(KRB5_BASE_LIBS)
+reload: reload.o
+	$(CC_LINK) -o $@ reload.o $(DL_LIB)
 t_accname: t_accname.o $(COMMON_DEPS)
 	$(CC_LINK) -o $@ t_accname.o $(COMMON_LIBS)
 t_add_cred: t_add_cred.o $(COMMON_DEPS)
@@ -121,9 +125,9 @@ t_srcattrs: t_srcattrs.o $(COMMON_DEPS)
 	$(CC_LINK) -o $@ t_srcattrs.o $(COMMON_LIBS)
 
 clean:
-	$(RM) ccinit ccrefresh t_accname t_add_cred t_bindings t_ccselect
-	$(RM) t_ciflags t_context t_credstore t_enctypes t_err t_export_cred
-	$(RM) t_export_name t_gssexts t_imp_cred t_imp_name t_invalid
-	$(RM) t_inq_cred t_inq_ctx t_inq_mechs_name t_iov t_lifetime
+	$(RM) ccinit ccrefresh reload t_accname t_add_cred t_bindings
+	$(RM) t_ccselect t_ciflags t_context t_credstore t_enctypes t_err
+	$(RM) t_export_cred t_export_name t_gssexts t_imp_cred t_imp_name
+	$(RM) t_invalid t_inq_cred t_inq_ctx t_inq_mechs_name t_iov t_lifetime
 	$(RM) t_namingexts t_oid t_pcontok t_prf t_s4u t_s4u2proxy_krb5
 	$(RM) t_saslname t_spnego t_srcattrs
diff --git a/src/tests/gssapi/deps b/src/tests/gssapi/deps
index 73e4d9a..ca1d6e2 100644
--- a/src/tests/gssapi/deps
+++ b/src/tests/gssapi/deps
@@ -25,6 +25,8 @@ $(OUTPRE)common.$(OBJEXT): $(BUILDTOP)/include/gssapi/gssapi.h \
   $(BUILDTOP)/include/gssapi/gssapi_ext.h $(BUILDTOP)/include/gssapi/gssapi_krb5.h \
   $(BUILDTOP)/include/krb5/krb5.h $(COM_ERR_DEPS) $(top_srcdir)/include/krb5.h \
   common.c common.h
+$(OUTPRE)reload.$(OBJEXT): $(BUILDTOP)/include/gssapi/gssapi.h \
+  reload.c
 $(OUTPRE)t_accname.$(OBJEXT): $(BUILDTOP)/include/gssapi/gssapi.h \
   $(BUILDTOP)/include/gssapi/gssapi_ext.h $(BUILDTOP)/include/gssapi/gssapi_krb5.h \
   $(BUILDTOP)/include/krb5/krb5.h $(COM_ERR_DEPS) $(top_srcdir)/include/krb5.h \
diff --git a/src/tests/gssapi/reload.c b/src/tests/gssapi/reload.c
new file mode 100644
index 0000000..4fe3565
--- /dev/null
+++ b/src/tests/gssapi/reload.c
@@ -0,0 +1,83 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* tests/gssapi/reload.c - test loading libgssapi_krb5 twice */
+/*
+ * Copyright (C) 2020 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 is a regression test for ticket #8614.  It ensures that libgssapi_krb5
+ * can be loaded multiple times in the same process when libkrb5support is held
+ * open by another library.
+ */
+
+#include <gssapi/gssapi.h>
+#include <stdio.h>
+#include <dlfcn.h>
+#include <assert.h>
+
+/* Load libgssapi_krb5, briefly use it (to force the initializer to run), and
+ * close it. */
+static void
+load_gssapi(void)
+{
+    void *gssapi;
+    OM_uint32 (*indmechs)(OM_uint32 *, gss_OID_set *);
+    OM_uint32 (*reloidset)(OM_uint32 *, gss_OID_set *);
+    OM_uint32 major, minor;
+    gss_OID_set mechs;
+
+    gssapi = dlopen("libgssapi_krb5.so", RTLD_NOW | RTLD_LOCAL);
+    assert(gssapi != NULL);
+    indmechs = dlsym(gssapi, "gss_indicate_mechs");
+    reloidset = dlsym(gssapi, "gss_release_oid_set");
+    assert(indmechs != NULL && reloidset != NULL);
+    major = (*indmechs)(&minor, &mechs);
+    assert(major == 0);
+    (*reloidset)(&minor, &mechs);
+    dlclose(gssapi);
+}
+
+int
+main()
+{
+    void *support;
+
+    /* Hold open libkrb5support to ensure that thread-local state remains */
+    support = dlopen("libkrb5support.so", RTLD_NOW | RTLD_LOCAL);
+    if (support == NULL) {
+        fprintf(stderr, "Error loading libkrb5support: %s\n", dlerror());
+        return 1;
+    }
+
+    load_gssapi();
+    load_gssapi();
+
+    dlclose(support);
+    return 0;
+}


More information about the cvs-krb5 mailing list