Fix double free bug in policy, add support for assertions in message rules.

diff --git a/saml/binding/impl/SecurityPolicy.cpp b/saml/binding/impl/SecurityPolicy.cpp
index 83e7f68..ef98f7b 100644 (file)
--- a/saml/binding/impl/SecurityPolicy.cpp
+++ b/saml/binding/impl/SecurityPolicy.cpp
@@ -79,10 +79,8 @@ void SecurityPolicy::evaluate(const XMLObject& message, const GenericRequest* re
 
 void SecurityPolicy::setIssuer(saml2::Issuer* issuer)
 {
-    if (!getIssuerMatchingPolicy().issuerMatches(issuer, m_issuer)) {
-        delete issuer;
-        throw BindingException("A rule supplied an Issuer that conflicts with previous results.");
-    }
+    if (!getIssuerMatchingPolicy().issuerMatches(issuer, m_issuer))
+        throw SecurityPolicyException("A rule supplied an Issuer that conflicts with previous results.");
     
     delete m_issuer;
     m_issuer=issuer;
@@ -91,7 +89,7 @@ void SecurityPolicy::setIssuer(saml2::Issuer* issuer)
 void SecurityPolicy::setIssuerMetadata(const RoleDescriptor* issuerRole)
 {
     if (issuerRole && m_issuerRole && issuerRole!=m_issuerRole)
-        throw BindingException("A rule supplied a RoleDescriptor that conflicts with previous results.");
+        throw SecurityPolicyException("A rule supplied a RoleDescriptor that conflicts with previous results.");
     m_issuerRole=issuerRole;
 }
 

diff --git a/saml/exceptions.h b/saml/exceptions.h
index af90f02..d66caf5 100644 (file)
--- a/saml/exceptions.h
+++ b/saml/exceptions.h
@@ -33,6 +33,7 @@ namespace opensaml {
         class SAML_API RoleDescriptor;
     };
     
+    DECL_XMLTOOLING_EXCEPTION(SecurityPolicyException,SAML_EXCEPTIONAPI(SAML_API),opensaml,xmltooling::XMLToolingException,Exceptions in security policy processing);
     DECL_XMLTOOLING_EXCEPTION(BindingException,SAML_EXCEPTIONAPI(SAML_API),opensaml,xmltooling::XMLToolingException,Exceptions in SAML binding processing);
     DECL_XMLTOOLING_EXCEPTION(ProfileException,SAML_EXCEPTIONAPI(SAML_API),opensaml,xmltooling::ValidationException,Exceptions in SAML profile processing);
     DECL_XMLTOOLING_EXCEPTION(FatalProfileException,SAML_EXCEPTIONAPI(SAML_API),opensaml,ProfileException,Fatal exceptions in SAML profile processing);

diff --git a/saml/saml1/binding/impl/SAML1MessageRule.cpp b/saml/saml1/binding/impl/SAML1MessageRule.cpp
index 397cbde..ac5eb54 100644 (file)
--- a/saml/saml1/binding/impl/SAML1MessageRule.cpp
+++ b/saml/saml1/binding/impl/SAML1MessageRule.cpp
@@ -64,8 +64,9 @@ void SAML1MessageRule::evaluate(const XMLObject& message, const GenericRequest*
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);
 
-    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1P_NS)) {
-        log.debug("not a SAML 1.x protocol message");
+    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1P_NS) &&
+        !XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1_NS)) {
+        log.debug("not a SAML 1.x protocol message or assertion");
         return;
     }
 
@@ -77,23 +78,33 @@ void SAML1MessageRule::evaluate(const XMLObject& message, const GenericRequest*
 
         log.debug("extracting issuer from message");
 
-        // Only samlp:Response is known to carry issuer (via payload) in standard SAML 1.x.
         const XMLCh* protocol = NULL;
-        if (XMLString::equals(q.getLocalPart(), Response::LOCAL_NAME)) {
+        const saml1::Assertion* a = NULL;
+
+        // Handle assertions directly.
+        if (XMLString::equals(q.getLocalPart(), saml1::Assertion::LOCAL_NAME))
+            a = dynamic_cast<const saml1::Assertion*>(&samlRoot);
+            
+        // Only samlp:Response is known to carry issuer (via payload) in standard SAML 1.x.
+        if (!a && XMLString::equals(q.getLocalPart(), Response::LOCAL_NAME)) {
             // Should be a samlp:Response.
             const vector<saml1::Assertion*>& assertions = dynamic_cast<const saml1p::Response&>(samlRoot).getAssertions();
-            if (!assertions.empty()) {
-                const saml1::Assertion* a = assertions.front();
-                if (a->getIssuer()) {
-                    auto_ptr<saml2::Issuer> issuer(saml2::IssuerBuilder::buildIssuer());
-                    issuer->setName(a->getIssuer());
-                    policy.setIssuer(issuer.get());
-                    issuer.release();   // owned by policy now
-                    pair<bool,int> minor = a->getMinorVersion();
-                    protocol = (minor.first && minor.second==0) ?
-                        samlconstants::SAML10_PROTOCOL_ENUM : samlconstants::SAML11_PROTOCOL_ENUM;
-                }
+            if (!assertions.empty())
+                a = assertions.front();
+        }
+
+        if (a && a->getIssuer()) {
+            if (!policy.getIssuer() || policy.getIssuer()->getFormat() ||
+                    !XMLString::equals(policy.getIssuer()->getName(), a->getIssuer())) {
+                // We either have a conflict, or a first-time set of Issuer.
+                auto_ptr<saml2::Issuer> issuer(saml2::IssuerBuilder::buildIssuer());
+                issuer->setName(a->getIssuer());
+                policy.setIssuer(issuer.get());
+                issuer.release();   // owned by policy now
             }
+            pair<bool,int> minor = a->getMinorVersion();
+            protocol = (minor.first && minor.second==0) ?
+                samlconstants::SAML10_PROTOCOL_ENUM : samlconstants::SAML11_PROTOCOL_ENUM;
         }
         
         if (!protocol) {
@@ -106,6 +117,11 @@ void SAML1MessageRule::evaluate(const XMLObject& message, const GenericRequest*
             log.debug("message from (%s)", iname.get());
         }
         
+        if (policy.getIssuerMetadata()) {
+            log.debug("metadata for issuer already set, leaving in place");
+            return;
+        }
+        
         if (policy.getMetadataProvider() && policy.getRole()) {
             log.debug("searching metadata for message issuer...");
             const EntityDescriptor* entity = policy.getMetadataProvider()->getEntityDescriptor(policy.getIssuer()->getName());

diff --git a/saml/saml2/binding/impl/SAML2MessageRule.cpp b/saml/saml2/binding/impl/SAML2MessageRule.cpp
index c9bb283..c1e632a 100644 (file)
--- a/saml/saml2/binding/impl/SAML2MessageRule.cpp
+++ b/saml/saml2/binding/impl/SAML2MessageRule.cpp
@@ -61,8 +61,9 @@ void SAML2MessageRule::evaluate(const XMLObject& message, const GenericRequest*
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);
     
-    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20P_NS)) {
-        log.debug("not a SAML 2.0 protocol message");
+    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20P_NS)&&
+        !XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20_NS)) {
+        log.debug("not a SAML 2.0 protocol message or assertion");
         return;
     }
 
@@ -101,6 +102,11 @@ void SAML2MessageRule::evaluate(const XMLObject& message, const GenericRequest*
             log.debug("message from (%s)", iname.get());
         }
 
+        if (policy.getIssuerMetadata()) {
+            log.debug("metadata for issuer already set, leaving in place");
+            return;
+        }
+
         if (policy.getMetadataProvider() && policy.getRole()) {
             if (policy.getIssuer()->getFormat() && !XMLString::equals(policy.getIssuer()->getFormat(), saml2::NameIDType::ENTITY)) {
                 log.warn("non-system entity issuer, skipping metadata lookup");