From: Dan Breslau <dbreslau@painless-security.com>
Date: Fri, 21 Jul 2017 13:59:04 +0000 (-0400)
Subject: Fix for https://bugs.launchpad.net/moonshot/+bug/1705714 (Infinite loop in tids exit... 
X-Git-Tag: moonshot-1.0.6-centos6~8
X-Git-Url: http://www.project-moonshot.org/gitweb/?p=mech_eap.git;a=commitdiff_plain;h=e741515228135ff735175923eb0ec09a817ee4e4

Fix for https://bugs.launchpad.net/moonshot/+bug/1705714 (Infinite loop in tids exit handlers)

Refactored the C++ static object whose destructor was unloading the libraries.
The shibresolver library is now unloaded by a "smaller" static object that is
created after shibresolver is loaded. Creating that object late means that it
will be destroyed earlier, before the static objects in the log4shib library
are destroyed.
---

diff --git a/mech_eap/eap_mech.c b/mech_eap/eap_mech.c
index c88ecf6..403e2f8 100644
--- a/mech_eap/eap_mech.c
+++ b/mech_eap/eap_mech.c
@@ -205,6 +205,7 @@ gssEapInitiatorInit(OM_uint32 *minor)
 void
 gssEapFinalize(void)
 {
+    wpa_printf(MSG_INFO, "### gssEapFinalize()");
     eap_peer_unregister_methods();
 }
 
diff --git a/mech_eap/util_attr.cpp b/mech_eap/util_attr.cpp
index beb283c..1931efe 100644
--- a/mech_eap/util_attr.cpp
+++ b/mech_eap/util_attr.cpp
@@ -64,9 +64,14 @@ GSSEAP_ONCE_CALLBACK(gssEapAttrProvidersInitInternal)
     gssEapLocalAttrProviderInit(&minor);
 #endif
 #ifdef HAVE_OPENSAML
+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Calling gssEapSamlAttrProvidersInit()");
     major = gssEapSamlAttrProvidersInit(&minor);
-    if (GSS_ERROR(major))
+    if (GSS_ERROR(major)) {
+        wpa_printf(MSG_ERROR, "### gssEapAttrProvidersInitInternal(): Error returned from gssEapSamlAttrProvidersInit; major code is %08X; minor is %08X", major, minor);
         goto cleanup;
+    }
+#else
+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Don't have OpenSAML; not calling gssEapSamlAttrProvidersInit()");
 #endif
 
 cleanup:
@@ -74,6 +79,7 @@ cleanup:
     GSSEAP_ASSERT(major == GSS_S_COMPLETE);
 #endif
 
+    wpa_printf(MSG_INFO, "### gssEapAttrProvidersInitInternal(): Setting gssEapAttrProvidersInitStatus to %08X", major);
     gssEapAttrProvidersInitStatus = major;
 
     GSSEAP_ONCE_LEAVE;
@@ -90,16 +96,25 @@ gssEapAttrProvidersInit(OM_uint32 *minor)
     return gssEapAttrProvidersInitStatus;
 }
 
+
 namespace {
+
     class finalize_class {
     public:
+
+        finalize_class() {
+            wpa_printf(MSG_INFO, "### finalize_class::finalize_class(): Constructing");
+        }
+
       ~finalize_class()
 	    {
 		OM_uint32 minor = 0;
+
+        wpa_printf(MSG_INFO, "### ~finalize_class::~finalize_class() : initStatus=%08x", gssEapAttrProvidersInitStatus);
+
 		if (gssEapAttrProvidersInitStatus == GSS_S_COMPLETE) {
-#ifdef HAVE_SHIBRESOLVER
-		    gssEapLocalAttrProviderFinalize(&minor);
-#endif
+            wpa_printf(MSG_INFO, "### ~finalize_class::~finalize_class() : really finalizing");
+
 #ifdef HAVE_OPENSAML
 		    gssEapSamlAttrProvidersFinalize(&minor);
 #endif
diff --git a/mech_eap/util_shib.cpp b/mech_eap/util_shib.cpp
index 7b62484..4ba70e0 100644
--- a/mech_eap/util_shib.cpp
+++ b/mech_eap/util_shib.cpp
@@ -78,6 +78,58 @@ using namespace opensaml;
 using namespace xercesc;
 #endif
 
+
+namespace {
+
+
+    class ShibFinalizer {
+    public:
+
+        static bool isShibInitialized() {return shibInitialized;}
+        static void createSingleton();
+
+    private:
+        ShibFinalizer(): is_extra(false) {
+            if (shibInitialized) {
+                // This should never, ever happen. Initialization is (supposed to be)
+                // funneled through a single thread, so there should be no race
+                // conditions here. And only this class sets this flag, and there's
+                // only a single instance of this class.
+                wpa_printf(MSG_ERROR, "### ShibFinalizer::ShibFinalizer(): Attempt to construct an extraneous instance!");
+                is_extra = true;
+            }
+            else {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::ShibFinalizer(): Constructing");
+                shibInitialized = true;
+            }
+        }
+
+        ~ShibFinalizer() {
+            if (!is_extra) {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): Destructing");
+                gss_eap_shib_attr_provider::finalize();
+                shibInitialized = false;
+            }
+            else {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): This was an extraneous instance; not destructing anything.");
+            }
+        }
+
+        bool is_extra;
+        static bool shibInitialized;
+    };
+}
+
+
+bool ShibFinalizer::shibInitialized = false;
+
+void ShibFinalizer::createSingleton() {
+    // This object's constructor is invoked on the first call to this method.
+    // At exit, its destructor will terminate Shibboleth.
+    static ShibFinalizer finalizer;
+}
+
+
 gss_eap_shib_attr_provider::gss_eap_shib_attr_provider(void)
 {
     m_initialized = false;
@@ -463,13 +515,22 @@ gss_eap_shib_attr_provider::init(void)
 {
     bool ret = false;
 
+    if (ShibFinalizer::isShibInitialized()) {
+        wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::init(): ShibResolver library is already initialized; ignoring.");
+        return true;
+    }
+
+    wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::init(): Initializing ShibResolver library");
+
     try {
         ret = ShibbolethResolver::init();
     } catch (exception &e) {
     }
 
-    if (ret)
+    if (ret) {
+        ShibFinalizer::createSingleton();
         gss_eap_attr_ctx::registerProvider(ATTR_TYPE_LOCAL, createAttrContext);
+    }
 
     return ret;
 }
@@ -477,6 +538,7 @@ gss_eap_shib_attr_provider::init(void)
 void
 gss_eap_shib_attr_provider::finalize(void)
 {
+    wpa_printf(MSG_INFO, "### gss_eap_shib_attr_provider::finalize(): calling ShibbolethResolver::term()");
     gss_eap_attr_ctx::unregisterProvider(ATTR_TYPE_LOCAL);
     ShibbolethResolver::term();
 }
@@ -534,6 +596,7 @@ gss_eap_shib_attr_provider::duplicateAttributes(const vector <Attribute *>src)
     return dst;
 }
 
+
 OM_uint32
 gssEapLocalAttrProviderInit(OM_uint32 *minor)
 {
@@ -541,14 +604,6 @@ gssEapLocalAttrProviderInit(OM_uint32 *minor)
         *minor = GSSEAP_SHIB_INIT_FAILURE;
         return GSS_S_FAILURE;
     }
-    return GSS_S_COMPLETE;
-}
-
-OM_uint32
-gssEapLocalAttrProviderFinalize(OM_uint32 *minor)
-{
-    gss_eap_shib_attr_provider::finalize();
 
-    *minor = 0;
     return GSS_S_COMPLETE;
 }