krb5 commit [krb5-1.17]: Unregister thread key in SPNEGO finalization

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/994f5f5107676aa127c680f91df3f682b89cfd88
commit 994f5f5107676aa127c680f91df3f682b89cfd88
Author: Greg Hudson <[email protected]>
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.
    
    (cherry picked from commit 07ff54d0bb85109df114612bbbfa6559f4a1e0cb)
    
    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        |   11 +++--
 src/tests/gssapi/deps               |    2 +
 src/tests/gssapi/reload.c           |   83 +++++++++++++++++++++++++++++++++++
 5 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index 9e0546d..5c2d5ea 100644
--- a/.gitignore
+++ b/.gitignore
@@ -465,6 +465,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_ccselect
diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c
index 038fdb6..24c94a7 100644
--- a/src/lib/gssapi/spnego/spnego_mech.c
+++ b/src/lib/gssapi/spnego/spnego_mech.c
@@ -326,6 +326,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 a7b8da4..378e492 100644
--- a/src/tests/gssapi/Makefile.in
+++ b/src/tests/gssapi/Makefile.in
@@ -8,7 +8,7 @@ LOCALINCLUDES = -I$(srcdir)/../../lib/gssapi/mechglue \
 	-I$(srcdir)/../../lib/gssapi/generic -I../../lib/gssapi/krb5 \
 	-I../../lib/gssapi/generic
 
-SRCS=	$(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c \
+SRCS=	$(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c $(srcdir)/reload.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 \
@@ -20,7 +20,7 @@ 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_ccselect.o \
+OBJS=	ccinit.o ccrefresh.o common.o reload.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 \
@@ -36,11 +36,12 @@ all: ccinit ccrefresh t_accname t_add_cred t_ccselect t_ciflags t_credstore \
 	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_ccselect t_ciflags \
 	t_credstore t_enctypes t_err t_export_cred t_export_name t_imp_cred \
@@ -58,6 +59,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)
@@ -114,7 +117,7 @@ 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_ccselect t_ciflags
+	$(RM) ccinit ccrefresh reload 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
diff --git a/src/tests/gssapi/deps b/src/tests/gssapi/deps
index c8905c9..2d50762 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;
+}
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.