Fix for https://bugs.launchpad.net/moonshot/+bug/1705714 (Infinite loop in tids exit... dbreslau/1705714_infinite_loop_at_exit
authorDan Breslau <dbreslau@painless-security.com>
Fri, 21 Jul 2017 13:59:04 +0000 (09:59 -0400)
committerDan Breslau <dbreslau@painless-security.com>
Fri, 21 Jul 2017 13:59:04 +0000 (09:59 -0400)
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.

mech_eap/eap_mech.c
mech_eap/util_attr.cpp
mech_eap/util_shib.cpp

index c88ecf6..403e2f8 100644 (file)
@@ -205,6 +205,7 @@ gssEapInitiatorInit(OM_uint32 *minor)
 void
 gssEapFinalize(void)
 {
+    wpa_printf(MSG_INFO, "### gssEapFinalize()");
     eap_peer_unregister_methods();
 }
 
index beb283c..1931efe 100644 (file)
@@ -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
index 7b62484..6c5566c 100644 (file)
@@ -78,6 +78,33 @@ using namespace opensaml;
 using namespace xercesc;
 #endif
 
+
+namespace {
+    bool shibInitialized;
+
+    class ShibFinalizer {
+    public:
+
+        ShibFinalizer() {
+            wpa_printf(MSG_INFO, "### ShibFinalizer::ShibFinalizer(): Constructing");
+
+            // Category::getInstance(MECHEAP_LOGCAT).debug("log4shib is now initialized");
+        }
+
+        ~ShibFinalizer() {
+            if (shibInitialized) {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): Destructing");
+                gss_eap_shib_attr_provider::finalize();
+            }
+            else {
+                wpa_printf(MSG_INFO, "### ShibFinalizer::~ShibFinalizer(): Shib is not initialized.");
+            }
+        }
+
+    };
+}
+
+
 gss_eap_shib_attr_provider::gss_eap_shib_attr_provider(void)
 {
     m_initialized = false;
@@ -477,6 +504,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,21 +562,40 @@ gss_eap_shib_attr_provider::duplicateAttributes(const vector <Attribute *>src)
     return dst;
 }
 
+static void createShibFinalizer() {
+    // This object's constructor is invoked on the first call to this method.
+    // At exit, its destructor will terminate Shibboleth.
+    static ShibFinalizer finalizer;
+}
+
 OM_uint32
 gssEapLocalAttrProviderInit(OM_uint32 *minor)
 {
+    if (shibInitialized) {
+        wpa_printf(MSG_INFO, "### gssEapLocalAttrProviderInit(): ShibResolver is already initialized.");
+        return GSS_S_COMPLETE;
+    }
+
+    wpa_printf(MSG_INFO, "### gssEapLocalAttrProviderInit(): Initializing ShibResolver");
     if (!gss_eap_shib_attr_provider::init()) {
         *minor = GSSEAP_SHIB_INIT_FAILURE;
         return GSS_S_FAILURE;
     }
-    return GSS_S_COMPLETE;
-}
+    shibInitialized = true;
 
-OM_uint32
-gssEapLocalAttrProviderFinalize(OM_uint32 *minor)
-{
-    gss_eap_shib_attr_provider::finalize();
+    // Construct a static object that, since it's being initialized after the ShibResolver,
+    // will be destroyed before static objects in the ShibResolver are destroyed.
+    // This will let us terminate the ShibResolver before its static objects are destructed.
+    createShibFinalizer();
 
-    *minor = 0;
     return GSS_S_COMPLETE;
 }
+
+// static OM_uint32
+// gssEapLocalAttrProviderFinalize(OM_uint32 *minor)
+// {
+//     gss_eap_shib_attr_provider::finalize();
+
+//     *minor = 0;
+//     return GSS_S_COMPLETE;
+// }