Refactor export reentrancy fix to be less intrusive
authorLuke Howard <lukeh@padl.com>
Mon, 28 Mar 2011 22:06:48 +0000 (09:06 +1100)
committerLuke Howard <lukeh@padl.com>
Mon, 28 Mar 2011 22:06:48 +0000 (09:06 +1100)
mech_eap/export_sec_context.c
mech_eap/gssapiP_eap.h
mech_eap/util.h
mech_eap/util_attr.cpp
mech_eap/util_attr.h
mech_eap/util_name.c
mech_eap/util_reauth.c
mech_eap/util_shib.cpp
mech_eap/util_shib.h

index b242086..43f3f28 100644 (file)
@@ -102,8 +102,7 @@ cleanup:
 OM_uint32
 gssEapExportSecContext(OM_uint32 *minor,
                        gss_ctx_id_t ctx,
-                       gss_buffer_t token,
-                       OM_uint32 flags)
+                       gss_buffer_t token)
 {
     OM_uint32 major, tmpMinor;
     size_t length;
@@ -123,13 +122,9 @@ gssEapExportSecContext(OM_uint32 *minor,
     key.value  = KRB_KEY_DATA(&ctx->rfc3961Key);
 
     if (ctx->initiatorName != GSS_C_NO_NAME) {
-        OM_uint32 nameFlags = EXPORT_NAME_FLAG_COMPOSITE;
-
-        if (flags & EXPORT_CTX_FLAG_DISABLE_LOCAL_ATTRS)
-            nameFlags |= EXPORT_NAME_FLAG_DISABLE_LOCAL_ATTRS;
-
         major = gssEapExportNameInternal(minor, ctx->initiatorName,
-                                         &initiatorName, nameFlags);
+                                         &initiatorName,
+                                         EXPORT_NAME_FLAG_COMPOSITE);
         if (GSS_ERROR(major))
             goto cleanup;
     }
@@ -233,7 +228,7 @@ gss_export_sec_context(OM_uint32 *minor,
 
     GSSEAP_MUTEX_LOCK(&ctx->mutex);
 
-    major = gssEapExportSecContext(minor, ctx, interprocess_token, 0);
+    major = gssEapExportSecContext(minor, ctx, interprocess_token);
     if (GSS_ERROR(major)) {
         GSSEAP_MUTEX_UNLOCK(&ctx->mutex);
         return major;
index 635c155..a45ebd5 100644 (file)
@@ -263,13 +263,10 @@ gssEapSaveStatusInfo(OM_uint32 minor, const char *format, ...);
                                          (err) <= GSSEAP_RADIUS_PROT_FAILURE)
 
 /* export_sec_context.c */
-#define EXPORT_CTX_FLAG_DISABLE_LOCAL_ATTRS     0x1
-
 OM_uint32
 gssEapExportSecContext(OM_uint32 *minor,
                        gss_ctx_id_t ctx,
-                       gss_buffer_t token,
-                       OM_uint32 flags);
+                       gss_buffer_t token);
 
 
 #ifdef __cplusplus
index 9ca587b..1c1b585 100644 (file)
@@ -455,7 +455,6 @@ gssEapSaslNameToOid(const gss_buffer_t name);
 /* util_name.c */
 #define EXPORT_NAME_FLAG_OID                    0x1
 #define EXPORT_NAME_FLAG_COMPOSITE              0x2
-#define EXPORT_NAME_FLAG_DISABLE_LOCAL_ATTRS    0x4
 
 OM_uint32 gssEapAllocName(OM_uint32 *minor, gss_name_t *pName);
 OM_uint32 gssEapReleaseName(OM_uint32 *minor, gss_name_t *pName);
index f2e7661..282d3ac 100644 (file)
@@ -339,7 +339,7 @@ gss_eap_attr_ctx::initWithJsonObject(JSONObject &obj)
 }
 
 JSONObject
-gss_eap_attr_ctx::jsonRepresentation(uint32_t flags) const
+gss_eap_attr_ctx::jsonRepresentation(void) const
 {
     JSONObject obj, sources;
     unsigned int i;
@@ -351,10 +351,6 @@ gss_eap_attr_ctx::jsonRepresentation(uint32_t flags) const
         gss_eap_attr_provider *provider;
         const char *key;
 
-        if (i == ATTR_TYPE_LOCAL &&
-            (flags & ATTR_FLAG_DISABLE_LOCAL))
-            continue; /* reentrancy workaround */
-
         provider = m_providers[i];
         if (provider == NULL)
             continue; /* provider not initialised */
@@ -626,13 +622,12 @@ gss_eap_attr_ctx::releaseAnyNameMapping(gss_buffer_t type_id,
  * Export attribute context to buffer
  */
 void
-gss_eap_attr_ctx::exportToBuffer(gss_buffer_t buffer,
-                                 uint32_t flags) const
+gss_eap_attr_ctx::exportToBuffer(gss_buffer_t buffer) const
 {
     OM_uint32 minor;
     char *s;
 
-    JSONObject obj = jsonRepresentation(flags);
+    JSONObject obj = jsonRepresentation();
 
 #if 0
     obj.dump(stdout, JSON_INDENT(3));
@@ -973,8 +968,7 @@ gssEapSetNameAttribute(OM_uint32 *minor,
 OM_uint32
 gssEapExportAttrContext(OM_uint32 *minor,
                         gss_name_t name,
-                        gss_buffer_t buffer,
-                        OM_uint32 flags)
+                        gss_buffer_t buffer)
 {
     if (name->attrCtx == NULL) {
         buffer->length = 0;
@@ -987,7 +981,7 @@ gssEapExportAttrContext(OM_uint32 *minor,
         return GSS_S_UNAVAILABLE;
 
     try {
-        name->attrCtx->exportToBuffer(buffer, flags);
+        name->attrCtx->exportToBuffer(buffer);
     } catch (std::exception &e) {
         return name->attrCtx->mapException(minor, e);
     }
index bdc8766..e391df1 100644 (file)
@@ -59,6 +59,8 @@ typedef bool
 #define ATTR_TYPE_MIN               ATTR_TYPE_RADIUS
 #define ATTR_TYPE_MAX               ATTR_TYPE_LOCAL
 
+#define ATTR_FLAG_DISABLE_LOCAL     0x00000001
+
 /*
  * Attribute provider: this represents a source of attributes derived
  * from the security context.
@@ -207,8 +209,7 @@ public:
     void releaseAnyNameMapping(gss_buffer_t type_id,
                                gss_any_t input) const;
 
-    void exportToBuffer(gss_buffer_t buffer,
-                        uint32_t flags) const;
+    void exportToBuffer(gss_buffer_t buffer) const;
     bool initFromBuffer(const gss_buffer_t buffer);
 
     static std::string
@@ -254,7 +255,7 @@ private:
     gss_buffer_desc attributeTypeToPrefix(unsigned int type) const;
 
     bool initWithJsonObject(JSONObject &object);
-    JSONObject jsonRepresentation(uint32_t flags = 0) const;
+    JSONObject jsonRepresentation(void) const;
 
     gss_eap_attr_provider *getPrimaryProvider(void) const;
 
@@ -302,8 +303,6 @@ struct gss_eap_attr_ctx;
 extern "C" {
 #endif
 
-#define ATTR_FLAG_DISABLE_LOCAL     0x00000001
-
 /*
  * C wrappers for attribute context functions. These match their
  * GSS naming extension equivalents. The caller is required to
@@ -349,8 +348,7 @@ gssEapSetNameAttribute(OM_uint32 *minor,
 OM_uint32
 gssEapExportAttrContext(OM_uint32 *minor,
                         gss_name_t name,
-                        gss_buffer_t buffer,
-                        OM_uint32 flags);
+                        gss_buffer_t buffer);
 
 OM_uint32
 gssEapImportAttrContext(OM_uint32 *minor,
index e8c0d66..aeef333 100644 (file)
@@ -551,12 +551,7 @@ gssEapExportNameInternal(OM_uint32 *minor,
     }
     exportedNameLen += 4 + nameBuf.length;
     if (flags & EXPORT_NAME_FLAG_COMPOSITE) {
-        OM_uint32 attrFlags = 0;
-
-        if (flags & EXPORT_NAME_FLAG_DISABLE_LOCAL_ATTRS)
-            attrFlags |= ATTR_FLAG_DISABLE_LOCAL;
-
-        major = gssEapExportAttrContext(minor, name, &attrs, attrFlags);
+        major = gssEapExportAttrContext(minor, name, &attrs);
         if (GSS_ERROR(major))
             goto cleanup;
         exportedNameLen += attrs.length;
index 631a70f..465bb16 100644 (file)
@@ -146,7 +146,7 @@ freezeAttrContext(OM_uint32 *minor,
 
     GSSEAP_KRB_INIT(&krbContext);
 
-    major = gssEapExportAttrContext(minor, initiatorName, &attrBuf, 0);
+    major = gssEapExportAttrContext(minor, initiatorName, &attrBuf);
     if (GSS_ERROR(major))
         return major;
 
index 4358578..2f6e54d 100644 (file)
@@ -70,6 +70,7 @@ using namespace std;
 
 gss_eap_shib_attr_provider::gss_eap_shib_attr_provider(void)
 {
+    m_initialized = false;
     m_authenticated = false;
 }
 
@@ -99,6 +100,8 @@ gss_eap_shib_attr_provider::initFromExistingContext(const gss_eap_attr_ctx *mana
         m_authenticated = shib->authenticated();
     }
 
+    m_initialized = true;
+
     return true;
 }
 
@@ -136,15 +139,12 @@ gss_eap_shib_attr_provider::initFromGssContext(const gss_eap_attr_ctx *manager,
     }
 #endif
 
-    major = gssEapExportSecContext(&minor, gssCtx, &exportedCtx,
-                                   EXPORT_CTX_FLAG_DISABLE_LOCAL_ATTRS);
+    major = gssEapExportSecContext(&minor, gssCtx, &exportedCtx);
     if (major == GSS_S_COMPLETE) {
         resolver->addToken(&exportedCtx);
         gss_release_buffer(&minor, &exportedCtx);
     }
 
-    m_authenticated = true;
-
     if (saml != NULL && saml->getAssertion() != NULL) {
         resolver->addToken(saml->getAssertion());
         m_authenticated = saml->authenticated();
@@ -160,6 +160,8 @@ gss_eap_shib_attr_provider::initFromGssContext(const gss_eap_attr_ctx *manager,
 #endif
     }
 
+    m_initialized = true;
+
     return true;
 }
 
@@ -168,6 +170,8 @@ gss_eap_shib_attr_provider::getAttributeIndex(const gss_buffer_t attr) const
 {
     int i = 0;
 
+    assert(m_initialized);
+
     for (vector<Attribute *>::const_iterator a = m_attributes.begin();
          a != m_attributes.end();
          ++a)
@@ -194,6 +198,8 @@ gss_eap_shib_attr_provider::setAttribute(int complete GSSEAP_UNUSED,
     vector <string> ids(1, attrStr);
     SimpleAttribute *a = new SimpleAttribute(ids);
 
+    assert(m_initialized);
+
     if (value->length != 0) {
         string valueStr((char *)value->value, value->length);
 
@@ -211,6 +217,8 @@ gss_eap_shib_attr_provider::deleteAttribute(const gss_buffer_t attr)
 {
     int i;
 
+    assert(m_initialized);
+
     i = getAttributeIndex(attr);
     if (i >= 0)
         m_attributes.erase(m_attributes.begin() + i);
@@ -224,6 +232,8 @@ bool
 gss_eap_shib_attr_provider::getAttributeTypes(gss_eap_attr_enumeration_cb addAttribute,
                                               void *data) const
 {
+    assert(m_initialized);
+
     for (vector<Attribute*>::const_iterator a = m_attributes.begin();
         a != m_attributes.end();
         ++a)
@@ -245,6 +255,8 @@ gss_eap_shib_attr_provider::getAttribute(const gss_buffer_t attr) const
 {
     const Attribute *ret = NULL;
 
+    assert(m_initialized);
+
     for (vector<Attribute *>::const_iterator a = m_attributes.begin();
          a != m_attributes.end();
          ++a)
@@ -277,6 +289,8 @@ gss_eap_shib_attr_provider::getAttribute(const gss_buffer_t attr,
     gss_buffer_desc buf;
     int nvalues, i = *more;
 
+    assert(m_initialized);
+
     *more = 0;
 
     shibAttr = getAttribute(attr);
@@ -318,6 +332,8 @@ gss_eap_shib_attr_provider::mapToAny(int authenticated,
 {
     gss_any_t output;
 
+    assert(m_initialized);
+
     if (authenticated && !m_authenticated)
         return (gss_any_t)NULL;
 
@@ -332,6 +348,8 @@ void
 gss_eap_shib_attr_provider::releaseAnyNameMapping(gss_buffer_t type_id GSSEAP_UNUSED,
                                                   gss_any_t input) const
 {
+    assert(m_initialized);
+
     vector <Attribute *> *v = ((vector <Attribute *> *)input);
     delete v;
 }
@@ -353,7 +371,8 @@ gss_eap_shib_attr_provider::jsonRepresentation(void) const
 {
     JSONObject obj;
 
-    obj.set("authenticated", m_authenticated);
+    if (m_initialized == false)
+        return obj; /* don't export incomplete context */
 
     JSONObject attrs = JSONObject::array();
 
@@ -366,6 +385,8 @@ gss_eap_shib_attr_provider::jsonRepresentation(void) const
 
     obj.set("attributes", attrs);
 
+    obj.set("authenticated", m_authenticated);
+
     return obj;
 }
 
@@ -379,8 +400,6 @@ gss_eap_shib_attr_provider::initWithJsonObject(const gss_eap_attr_ctx *ctx,
     assert(m_authenticated == false);
     assert(m_attributes.size() == 0);
 
-    m_authenticated = obj["authenticated"].integer();
-
     JSONObject attrs = obj["attributes"];
     size_t nelems = attrs.size();
 
@@ -390,6 +409,9 @@ gss_eap_shib_attr_provider::initWithJsonObject(const gss_eap_attr_ctx *ctx,
         m_attributes.push_back(attribute);
     }
 
+    m_authenticated = obj["authenticated"].integer();
+    m_initialized = true;
+
     return true;
 }
 
index 7fc9e6d..8080e0c 100644 (file)
@@ -104,16 +104,11 @@ private:
 
     bool authenticated(void) const { return m_authenticated; }
 
-    friend bool
-    addRadiusAttribute(const gss_eap_attr_provider *source,
-                       const gss_buffer_t attribute,
-                       void *data);
-
+    bool m_initialized;
+    bool m_authenticated;
     std::vector<shibsp::Attribute *> m_attributes;
-    int m_authenticated;
 };
 
-
 extern "C" {
 #endif