https://issues.shibboleth.net/jira/browse/SSPCPP-264

diff --git a/configs/shibboleth2.xml b/configs/shibboleth2.xml
index d935c7f..8ed5938 100644 (file)
--- a/configs/shibboleth2.xml
+++ b/configs/shibboleth2.xml
@@ -227,7 +227,7 @@
         <AttributeExtractor type="XML" validate="true" path="attribute-map.xml"/>
         
         <!-- Use a SAML query if no attributes are supplied during SSO. -->
-        <AttributeResolver type="Query"/>
+        <AttributeResolver type="Query" subjectMatch="true"/>
 
         <!-- Default filtering policy for recognized attributes, lets other data pass. -->
         <AttributeFilter type="XML" validate="true" path="attribute-policy.xml"/>

diff --git a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
index 5948b5c..f080088 100644 (file)
--- a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
+++ b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp
@@ -205,6 +205,7 @@ namespace shibsp {
 
         Category& m_log;
         string m_policyId;
+        bool m_subjectMatch;
         vector<AttributeDesignator*> m_SAML1Designators;
         vector<saml2::Attribute*> m_SAML2Designators;
     };
@@ -214,20 +215,24 @@ namespace shibsp {
         return new QueryResolver(e);
     }
 
-    static const XMLCh _policyId[] = UNICODE_LITERAL_8(p,o,l,i,c,y,I,d);
+    static const XMLCh policyId[] =     UNICODE_LITERAL_8(p,o,l,i,c,y,I,d);
+    static const XMLCh subjectMatch[] = UNICODE_LITERAL_12(s,u,b,j,e,c,t,M,a,t,c,h);
 };
 
-QueryResolver::QueryResolver(const DOMElement* e) : m_log(Category::getInstance(SHIBSP_LOGCAT".AttributeResolver.Query"))
+QueryResolver::QueryResolver(const DOMElement* e) : m_log(Category::getInstance(SHIBSP_LOGCAT".AttributeResolver.Query")), m_subjectMatch(false)
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("QueryResolver");
 #endif
 
-    const XMLCh* pid = e ? e->getAttributeNS(NULL, _policyId) : NULL;
+    const XMLCh* pid = e ? e->getAttributeNS(NULL, policyId) : NULL;
     if (pid && *pid) {
         auto_ptr_char temp(pid);
         m_policyId = temp.get();
     }
+    pid = e ? e->getAttributeNS(NULL, subjectMatch) : NULL;
+    if (pid && (*pid == chLatin_t || *pid == chDigit_1))
+        m_subjectMatch = true;
 
     DOMElement* child = XMLHelper::getFirstChildElement(e);
     while (child) {
@@ -374,7 +379,24 @@ bool QueryResolver::SAML1Query(QueryContext& ctx) const
         AttributeExtractor* extractor = application.getAttributeExtractor();
         if (extractor) {
             Locker extlocker(extractor);
-            extractor->extractAttributes(application, AA, *newtoken, ctx.getResolvedAttributes());
+            const vector<saml1::AttributeStatement*>& statements = const_cast<const saml1::Assertion*>(newtoken)->getAttributeStatements();
+            for (vector<saml1::AttributeStatement*>::const_iterator s = statements.begin(); s!=statements.end(); ++s) {
+                if (m_subjectMatch) {
+                    // Check for subject match.
+                    const NameIdentifier* respName = (*s)->getSubject() ? (*s)->getSubject()->getNameIdentifier() : NULL;
+                    if (!respName || !XMLString::equals(respName->getName(), ctx.getNameID()->getName()) ||
+                        !XMLString::equals(respName->getFormat(), ctx.getNameID()->getFormat()) ||
+                        !XMLString::equals(respName->getNameQualifier(), ctx.getNameID()->getNameQualifier())) {
+                        if (respName)
+                            m_log.warnStream() << "ignoring AttributeStatement without strongly matching NameIdentifier in Subject: " <<
+                                *respName << logging::eol;
+                        else
+                            m_log.warn("ignoring AttributeStatement without NameIdentifier in Subject");
+                        continue;
+                    }
+                }
+                extractor->extractAttributes(application, AA, *(*s), ctx.getResolvedAttributes());
+            }
         }
 
         AttributeFilter* filter = application.getAttributeFilter();
@@ -437,7 +459,6 @@ bool QueryResolver::SAML2Query(QueryContext& ctx) const
             // Encrypt the NameID?
             if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) {
                 auto_ptr<EncryptedID> encrypted(EncryptedIDBuilder::buildEncryptedID());
-                MetadataCredentialCriteria mcc(*AA);
                 encrypted->encrypt(
                     *ctx.getNameID(),
                     *(application.getMetadataProvider()),
@@ -509,12 +530,7 @@ bool QueryResolver::SAML2Query(QueryContext& ctx) const
         // Attempt to decrypt it.
         try {
             Locker credlocker(cr);
-            auto_ptr<MetadataCredentialCriteria> mcc(
-                policy.getIssuerMetadata() ? new MetadataCredentialCriteria(*policy.getIssuerMetadata()) : NULL
-                );
-            auto_ptr<XMLObject> tokenwrapper(
-                encassertions.front()->decrypt(*cr, application.getRelyingParty(ctx.getEntityDescriptor())->getXMLString("entityID").second, mcc.get())
-                );
+            auto_ptr<XMLObject> tokenwrapper(encassertions.front()->decrypt(*cr, relyingParty->getXMLString("entityID").second, &mcc));
             newtoken = dynamic_cast<saml2::Assertion*>(tokenwrapper.get());
             if (newtoken) {
                 tokenwrapper.release();
@@ -559,6 +575,48 @@ bool QueryResolver::SAML2Query(QueryContext& ctx) const
         // Now we can check the security status of the policy.
         if (!policy.isAuthenticated())
             throw SecurityPolicyException("Security of SAML 2.0 query result not established.");
+
+        if (m_subjectMatch) {
+            // Check for subject match.
+            bool ownedName = false;
+            NameID* respName = newtoken->getSubject() ? newtoken->getSubject()->getNameID() : NULL;
+            if (!respName) {
+                // Check for encryption.
+                EncryptedID* encname = newtoken->getSubject() ? newtoken->getSubject()->getEncryptedID() : NULL;
+                if (encname) {
+                    CredentialResolver* cr=application.getCredentialResolver();
+                    if (!cr)
+                        m_log.warn("found EncryptedID, but no CredentialResolver was available");
+                    else {
+                        Locker credlocker(cr);
+                        auto_ptr<XMLObject> decryptedID(encname->decrypt(*cr, relyingParty->getXMLString("entityID").second, &mcc));
+                        respName = dynamic_cast<NameID*>(decryptedID.get());
+                        if (respName) {
+                            ownedName = true;
+                            decryptedID.release();
+                            if (m_log.isDebugEnabled())
+                                m_log.debugStream() << "decrypted NameID: " << *respName << logging::eol;
+                        }
+                    }
+                }
+            }
+
+            auto_ptr<NameID> nameIDwrapper(ownedName ? respName : NULL);
+
+            if (!respName || !XMLString::equals(respName->getName(), ctx.getNameID()->getName()) ||
+                !XMLString::equals(respName->getFormat(), ctx.getNameID()->getFormat()) ||
+                !XMLString::equals(respName->getNameQualifier(), ctx.getNameID()->getNameQualifier()) ||
+                !XMLString::equals(respName->getSPNameQualifier(), ctx.getNameID()->getSPNameQualifier())) {
+                if (respName)
+                    m_log.warnStream() << "ignoring Assertion without strongly matching NameID in Subject: " <<
+                        *respName << logging::eol;
+                else
+                    m_log.warn("ignoring Assertion without NameID in Subject");
+                if (!wrapper.get())
+                    delete newtoken;
+                return true;
+            }
+        }
     }
     catch (exception& ex) {
         m_log.error("assertion failed policy validation: %s", ex.what());
@@ -619,9 +677,11 @@ void QueryResolver::resolveAttributes(ResolutionContext& ctx) const
             m_log.debug("attempting SAML 1.x attribute query");
             SAML1Query(qctx);
         }
-        else
-            m_log.warn("SSO protocol does not allow for attribute query");
+        else {
+            m_log.info("SSO protocol does not allow for attribute query");
+        }
     }
-    else
+    else {
         m_log.warn("can't attempt attribute query, either no NameID or no metadata to use");
+    }
 }

diff --git a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
index 1773d9b..f3d9f79 100644 (file)
--- a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
+++ b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp
@@ -178,6 +178,7 @@ namespace shibsp {
 
         Category& m_log;
         string m_policyId;
+        bool m_subjectMatch;
         vector<string> m_attributeIds;
         xstring m_format;
         MetadataProvider* m_metadata;
@@ -197,12 +198,13 @@ namespace shibsp {
     static const XMLCh format[] =               UNICODE_LITERAL_6(f,o,r,m,a,t);
     static const XMLCh _MetadataProvider[] =    UNICODE_LITERAL_16(M,e,t,a,d,a,t,a,P,r,o,v,i,d,e,r);
     static const XMLCh policyId[] =             UNICODE_LITERAL_8(p,o,l,i,c,y,I,d);
+    static const XMLCh subjectMatch[] =         UNICODE_LITERAL_12(s,u,b,j,e,c,t,M,a,t,c,h);
     static const XMLCh _TrustEngine[] =         UNICODE_LITERAL_11(T,r,u,s,t,E,n,g,i,n,e);
     static const XMLCh _type[] =                UNICODE_LITERAL_4(t,y,p,e);
 };
 
 SimpleAggregationResolver::SimpleAggregationResolver(const DOMElement* e)
-    : m_log(Category::getInstance(SHIBSP_LOGCAT".AttributeResolver.SimpleAggregation")), m_metadata(NULL), m_trust(NULL)
+    : m_log(Category::getInstance(SHIBSP_LOGCAT".AttributeResolver.SimpleAggregation")), m_subjectMatch(false), m_metadata(NULL), m_trust(NULL)
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("SimpleAggregationResolver");
@@ -214,6 +216,10 @@ SimpleAggregationResolver::SimpleAggregationResolver(const DOMElement* e)
         m_policyId = temp.get();
     }
 
+    pid = e ? e->getAttributeNS(NULL, subjectMatch) : NULL;
+    if (pid && (*pid == chLatin_t || *pid == chDigit_1))
+        m_subjectMatch = true;
+
     pid = e ? e->getAttributeNS(NULL, attributeId) : NULL;
     if (pid && *pid) {
         char* dup = XMLString::transcode(pid);
@@ -235,7 +241,6 @@ SimpleAggregationResolver::SimpleAggregationResolver(const DOMElement* e)
         pid = e->getAttributeNS(NULL, format);
         if (pid && *pid)
             m_format = pid;
-
     }
 
     DOMElement* child = XMLHelper::getFirstChildElement(e, _MetadataProvider);
@@ -295,7 +300,6 @@ SimpleAggregationResolver::SimpleAggregationResolver(const DOMElement* e)
         }
         child = XMLHelper::getNextSiblingElement(child);
     }
-
 }
 
 bool SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const char* entityID, const NameID* name) const
@@ -353,7 +357,6 @@ bool SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
             // Encrypt the NameID?
             if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) {
                 auto_ptr<EncryptedID> encrypted(EncryptedIDBuilder::buildEncryptedID());
-                MetadataCredentialCriteria mcc(*AA);
                 encrypted->encrypt(
                     *name,
                     *(policy.getMetadataProvider()),
@@ -407,7 +410,8 @@ bool SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
     const vector<saml2::Assertion*>& assertions = const_cast<const saml2p::Response*>(response)->getAssertions();
     if (assertions.empty()) {
         // Check for encryption.
-        const vector<saml2::EncryptedAssertion*>& encassertions = const_cast<const saml2p::Response*>(response)->getEncryptedAssertions();
+        const vector<saml2::EncryptedAssertion*>& encassertions =
+            const_cast<const saml2p::Response*>(response)->getEncryptedAssertions();
         if (encassertions.empty()) {
             m_log.warn("response from attribute authority was empty");
             return true;
@@ -425,12 +429,7 @@ bool SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
         // Attempt to decrypt it.
         try {
             Locker credlocker(cr);
-            auto_ptr<MetadataCredentialCriteria> mcc(
-                policy.getIssuerMetadata() ? new MetadataCredentialCriteria(*policy.getIssuerMetadata()) : NULL
-                );
-            auto_ptr<XMLObject> tokenwrapper(
-                encassertions.front()->decrypt(*cr, relyingParty->getXMLString("entityID").second, mcc.get())
-                );
+            auto_ptr<XMLObject> tokenwrapper(encassertions.front()->decrypt(*cr, relyingParty->getXMLString("entityID").second, &mcc));
             newtoken = dynamic_cast<saml2::Assertion*>(tokenwrapper.get());
             if (newtoken) {
                 tokenwrapper.release();
@@ -475,6 +474,48 @@ bool SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha
         // Now we can check the security status of the policy.
         if (!policy.isAuthenticated())
             throw SecurityPolicyException("Security of SAML 2.0 query result not established.");
+
+        if (m_subjectMatch) {
+            // Check for subject match.
+            bool ownedName = false;
+            NameID* respName = newtoken->getSubject() ? newtoken->getSubject()->getNameID() : NULL;
+            if (!respName) {
+                // Check for encryption.
+                EncryptedID* encname = newtoken->getSubject() ? newtoken->getSubject()->getEncryptedID() : NULL;
+                if (encname) {
+                    CredentialResolver* cr=application.getCredentialResolver();
+                    if (!cr)
+                        m_log.warn("found EncryptedID, but no CredentialResolver was available");
+                    else {
+                        Locker credlocker(cr);
+                        auto_ptr<XMLObject> decryptedID(encname->decrypt(*cr, relyingParty->getXMLString("entityID").second, &mcc));
+                        respName = dynamic_cast<NameID*>(decryptedID.get());
+                        if (respName) {
+                            ownedName = true;
+                            decryptedID.release();
+                            if (m_log.isDebugEnabled())
+                                m_log.debugStream() << "decrypted NameID: " << *respName << logging::eol;
+                        }
+                    }
+                }
+            }
+
+            auto_ptr<NameID> nameIDwrapper(ownedName ? respName : NULL);
+
+            if (!respName || !XMLString::equals(respName->getName(), name->getName()) ||
+                !XMLString::equals(respName->getFormat(), name->getFormat()) ||
+                !XMLString::equals(respName->getNameQualifier(), name->getNameQualifier()) ||
+                !XMLString::equals(respName->getSPNameQualifier(), name->getSPNameQualifier())) {
+                if (respName)
+                    m_log.warnStream() << "ignoring Assertion without strongly matching NameID in Subject: " <<
+                        *respName << logging::eol;
+                else
+                    m_log.warn("ignoring Assertion without NameID in Subject");
+                if (!wrapper.get())
+                    delete newtoken;
+                return true;
+            }
+        }
     }
     catch (exception& ex) {
         m_log.error("assertion failed policy validation: %s", ex.what());