krb5 commit: 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/07ff54d0bb85109df114612bbbfa6559f4a1e0cb
commit 07ff54d0bb85109df114612bbbfa6559f4a1e0cb
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.
    
    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;
+}
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.