From: cantor Date: Mon, 17 Sep 2007 02:18:25 +0000 (+0000) Subject: Refactor assertion extraction into handlers. X-Git-Tag: 2.4~748 X-Git-Url: http://www.project-moonshot.org/gitweb/?p=shibboleth%2Fsp.git;a=commitdiff_plain;h=86b94e5088c87e59c1a786c0de796c15653fcaea Refactor assertion extraction into handlers. Fix some sloppy code related to policy reuse. git-svn-id: https://svn.middleware.georgetown.edu/cpp-sp/trunk@2489 cb58f699-b61c-0410-a6fe-9272a202ed29 --- diff --git a/adfs/adfs.cpp b/adfs/adfs.cpp index 6fc91a4..116fa02 100644 --- a/adfs/adfs.cpp +++ b/adfs/adfs.cpp @@ -82,6 +82,12 @@ namespace { virtual ~ADFSDecoder() {} XMLObject* decode(string& relayState, const GenericRequest& genericRequest, SecurityPolicy& policy) const; + + protected: + void extractMessageDetails( + const XMLObject& message, const GenericRequest& req, const XMLCh* protocol, SecurityPolicy& policy + ) const { + } }; MessageDecoder* ADFSDecoderFactory(const pair& p) @@ -514,12 +520,15 @@ string ADFSConsumer::implementProtocol( if (!token || !token->getSignature()) throw FatalProfileException("Incoming message did not contain a signed SAML 1.1 assertion."); - // Run the policy over the assertion. Handles issuer consistency, replay, freshness, - // and signature verification, assuming the relevant rules are configured. - policy.evaluate(*token, NULL, m_protocol.get()); + // Extract message and issuer details from assertion. + extractMessageDetails(*token, m_protocol.get(), policy); + + // Run the policy over the assertion. Handles replay, freshness, and + // signature verification, assuming the relevant rules are configured. + policy.evaluate(*token); // If no security is in place now, we kick it. - if (!policy.isSecure()) + if (!policy.isAuthenticated()) throw SecurityPolicyException("Unable to establish security of incoming assertion."); // Now do profile and core semantic validation to ensure we can use it for SSO. diff --git a/configs/shibboleth2.xml.in b/configs/shibboleth2.xml.in index 2a74d20..6842035 100644 --- a/configs/shibboleth2.xml.in +++ b/configs/shibboleth2.xml.in @@ -270,8 +270,6 @@ chunkedEncoding="false" connectTimeout="15" timeout="30" > - - diff --git a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp index 7a9988e..2cf113e 100644 --- a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp +++ b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp @@ -338,9 +338,19 @@ bool QueryResolver::SAML1Query(QueryContext& ctx) const } try { + // We're going to insist that the assertion issuer is the same as the peer. + // Reset the policy's message bits and extract them from the assertion. + policy.reset(true); + policy.setMessageID(newtoken->getAssertionID()); + policy.setIssueInstant(newtoken->getIssueInstantEpoch()); + policy.setIssuer(newtoken->getIssuer()); policy.evaluate(*newtoken); - if (!policy.isSecure()) + + // Now we can check the security status of the policy. + if (!policy.isAuthenticated()) throw SecurityPolicyException("Security of SAML 1.x query result not established."); + + // Lastly, check it over. saml1::AssertionValidator tokval(application.getAudiences(), time(NULL)); tokval.validateAssertion(*newtoken); } @@ -481,9 +491,19 @@ bool QueryResolver::SAML2Query(QueryContext& ctx) const } try { + // We're going to insist that the assertion issuer is the same as the peer. + // Reset the policy's message bits and extract them from the assertion. + policy.reset(true); + policy.setMessageID(newtoken->getID()); + policy.setIssueInstant(newtoken->getIssueInstantEpoch()); + policy.setIssuer(newtoken->getIssuer()); policy.evaluate(*newtoken); - if (!policy.isSecure()) + + // Now we can check the security status of the policy. + if (!policy.isAuthenticated()) throw SecurityPolicyException("Security of SAML 2.0 query result not established."); + + // Lastly, check it over. saml2::AssertionValidator tokval(application.getAudiences(), time(NULL)); tokval.validateAssertion(*newtoken); } diff --git a/shibsp/binding/impl/ArtifactResolver.cpp b/shibsp/binding/impl/ArtifactResolver.cpp index b62e3b6..6888d16 100644 --- a/shibsp/binding/impl/ArtifactResolver.cpp +++ b/shibsp/binding/impl/ArtifactResolver.cpp @@ -93,6 +93,8 @@ saml1p::Response* ArtifactResolver::resolve( throw BindingException("Identity provider returned a SAML error in response to artifact(s)."); } + // The SOAP client handles policy evaluation against the SOAP and Response layer, + // but no security checking is done here. return response; } @@ -149,5 +151,8 @@ ArtifactResponse* ArtifactResolver::resolve( BindingException ex("Identity provider returned a SAML error in response to artifact."); annotateException(&ex, &ssoDescriptor, response->getStatus()); // rethrow } + + // The SOAP client handles policy evaluation against the SOAP and Response layer, + // but no security checking is done here. return response; } diff --git a/shibsp/handler/AssertionConsumerService.h b/shibsp/handler/AssertionConsumerService.h index 90d363d..b8154b9 100644 --- a/shibsp/handler/AssertionConsumerService.h +++ b/shibsp/handler/AssertionConsumerService.h @@ -85,6 +85,17 @@ namespace shibsp { const xmltooling::XMLObject& xmlObject ) const=0; + /** + * Extracts policy-relevant assertion details. + * + * @param assertion the incoming assertion + * @param protocol the protocol family in use + * @param policy SecurityPolicy to provide various components and track message data + */ + virtual void extractMessageDetails( + const opensaml::Assertion& assertion, const XMLCh* protocol, opensaml::SecurityPolicy& policy + ) const; + /** * Attempt SSO-initiated attribute resolution using the supplied information, * including NameID and token extraction and filtering followed by diff --git a/shibsp/handler/impl/AssertionConsumerService.cpp b/shibsp/handler/impl/AssertionConsumerService.cpp index 41610d7..0a220f7 100644 --- a/shibsp/handler/impl/AssertionConsumerService.cpp +++ b/shibsp/handler/impl/AssertionConsumerService.cpp @@ -40,6 +40,8 @@ # include # include using namespace samlconstants; +using opensaml::saml2md::EntityDescriptor; +using opensaml::saml2md::IDPSSODescriptor; #else # include "lite/CommonDomainCookie.h" #endif @@ -384,6 +386,45 @@ ResolutionContext* AssertionConsumerService::resolveAttributes( return new DummyContext(resolvedAttributes); return NULL; } + +void AssertionConsumerService::extractMessageDetails(const Assertion& assertion, const XMLCh* protocol, opensaml::SecurityPolicy& policy) const +{ + policy.setMessageID(assertion.getID()); + policy.setIssueInstant(assertion.getIssueInstantEpoch()); + + if (XMLString::equals(assertion.getElementQName().getNamespaceURI(), samlconstants::SAML20P_NS)) { + const saml2::Assertion* a2 = dynamic_cast(&assertion); + if (a2) { + m_log.debug("extracting issuer from SAML 2.0 assertion"); + policy.setIssuer(a2->getIssuer()); + } + } + else { + const saml1::Assertion* a1 = dynamic_cast(&assertion); + if (a1) { + m_log.debug("extracting issuer from SAML 1.x assertion"); + policy.setIssuer(a1->getIssuer()); + } + } + + if (policy.getIssuer() && !policy.getIssuerMetadata() && policy.getMetadataProvider()) { + m_log.debug("searching metadata for assertion issuer..."); + const EntityDescriptor* entity = policy.getMetadataProvider()->getEntityDescriptor(policy.getIssuer()->getName()); + if (entity) { + m_log.debug("matched assertion issuer against metadata, searching for applicable role..."); + const IDPSSODescriptor* idp=entity->getIDPSSODescriptor(protocol); + if (idp) + policy.setIssuerMetadata(idp); + else if (m_log.isWarnEnabled()) + m_log.warn("unable to find compatible IdP role in metadata"); + } + else if (m_log.isWarnEnabled()) { + auto_ptr_char iname(policy.getIssuer()->getName()); + m_log.warn("no metadata found, can't establish identity of issuer (%s)", iname.get()); + } + } +} + #endif void AssertionConsumerService::maintainHistory(SPRequest& request, const char* entityID, const char* cookieProps) const diff --git a/shibsp/handler/impl/SAML1Consumer.cpp b/shibsp/handler/impl/SAML1Consumer.cpp index 1e06560..d9cb975 100644 --- a/shibsp/handler/impl/SAML1Consumer.cpp +++ b/shibsp/handler/impl/SAML1Consumer.cpp @@ -110,15 +110,15 @@ string SAML1Consumer::implementProtocol( // the focus here is on the assertion content. For SAML 1.x POST, // all the security comes from the protocol layer, and signing // the assertion isn't sufficient. So we can check the policy - // object now and bail if it's not a secure message. - if (m_post && !policy.isSecure()) { + // object now and bail if it's not a secured message. + if (m_post && !policy.isAuthenticated()) { if (policy.getIssuer() && !policy.getIssuerMetadata()) throw MetadataException("Security of SAML 1.x SSO POST response not established."); throw SecurityPolicyException("Security of SAML 1.x SSO POST response not established."); } // Remember whether we already established trust. - bool alreadySecured = policy.isSecure(); + bool alreadySecured = policy.isAuthenticated(); const Response* response = dynamic_cast(&xmlObject); if (!response) @@ -128,6 +128,8 @@ string SAML1Consumer::implementProtocol( if (assertions.empty()) throw FatalProfileException("Incoming message contained no SAML assertions."); + pair minor = response->getMinorVersion(); + // Maintain list of "legit" tokens to feed to SP subsystems. const AuthenticationStatement* ssoStatement=NULL; vector tokens; @@ -153,14 +155,20 @@ string SAML1Consumer::implementProtocol( try { // We clear the security flag, so we can tell whether the token was secured on its own. - policy.setSecure(false); - - // Run the policy over the assertion. Handles issuer consistency, replay, freshness, - // and signature verification, assuming the relevant rules are configured. + policy.setAuthenticated(false); + policy.reset(true); + + // Extract message bits and re-verify Issuer information. + extractMessageDetails( + *(*a), (minor.first && minor.second==0) ? samlconstants::SAML10_PROTOCOL_ENUM : samlconstants::SAML11_PROTOCOL_ENUM, policy + ); + + // Run the policy over the assertion. Handles replay, freshness, and + // signature verification, assuming the relevant rules are configured. policy.evaluate(*(*a)); // If no security is in place now, we kick it. - if (!alreadySecured && !policy.isSecure()) { + if (!alreadySecured && !policy.isAuthenticated()) { m_log.warn("unable to establish security of assertion"); badtokens.push_back(*a); continue; diff --git a/shibsp/handler/impl/SAML2ArtifactResolution.cpp b/shibsp/handler/impl/SAML2ArtifactResolution.cpp index 6a1097b..72ebae6 100644 --- a/shibsp/handler/impl/SAML2ArtifactResolution.cpp +++ b/shibsp/handler/impl/SAML2ArtifactResolution.cpp @@ -283,7 +283,7 @@ pair SAML2ArtifactResolution::processMessage(const Application& appli auto_ptr artobj(SAMLArtifact::parse(artifact.get())); auto_ptr payload(artmap->retrieveContent(artobj.get(), issuer.get())); - if (!policy.isSecure()) { + if (!policy.isAuthenticated()) { m_log.error("request for artifact was unauthenticated, purging the artifact mapping"); return samlError(application, *req, httpResponse, StatusCode::REQUESTER, StatusCode::AUTHN_FAILED, "Unable to authenticate request."); } diff --git a/shibsp/handler/impl/SAML2Consumer.cpp b/shibsp/handler/impl/SAML2Consumer.cpp index 534544d..9b1c3ec 100644 --- a/shibsp/handler/impl/SAML2Consumer.cpp +++ b/shibsp/handler/impl/SAML2Consumer.cpp @@ -99,7 +99,7 @@ string SAML2Consumer::implementProtocol( // Remember whether we already established trust. // None of the SAML 2 bindings require security at the protocol layer. - bool alreadySecured = policy.isSecure(); + bool alreadySecured = policy.isAuthenticated(); // Check for errors...this will throw if it's not a successful message. checkError(&xmlObject); @@ -146,14 +146,18 @@ string SAML2Consumer::implementProtocol( try { // We clear the security flag, so we can tell whether the token was secured on its own. - policy.setSecure(false); - - // Run the policy over the assertion. Handles issuer consistency, replay, freshness, - // and signature verification, assuming the relevant rules are configured. + policy.setAuthenticated(false); + policy.reset(true); + + // Extract message bits and re-verify Issuer information. + extractMessageDetails(*(*a), samlconstants::SAML20P_NS, policy); + + // Run the policy over the assertion. Handles replay, freshness, and + // signature verification, assuming the relevant rules are configured. policy.evaluate(*(*a)); // If no security is in place now, we kick it. - if (!alreadySecured && !policy.isSecure()) { + if (!alreadySecured && !policy.isAuthenticated()) { m_log.warn("unable to establish security of assertion"); badtokens.push_back(*a); continue; @@ -229,15 +233,21 @@ string SAML2Consumer::implementProtocol( try { // We clear the security flag, so we can tell whether the token was secured on its own. - policy.setSecure(false); - - // Run the policy over the assertion. Handles issuer consistency, replay, freshness, - // and signature verification, assuming the relevant rules are configured. + policy.setAuthenticated(false); + policy.reset(true); + + // Extract message bits and re-verify Issuer information. + extractMessageDetails(*decrypted, samlconstants::SAML20P_NS, policy); + + // Run the policy over the assertion. Handles replay, freshness, and + // signature verification, assuming the relevant rules are configured. // We have to marshall the object first to ensure signatures can be checked. + if (!decrypted->getDOM()) + decrypted->marshall(); policy.evaluate(*decrypted); // If no security is in place now, we kick it. - if (!alreadySecured && !policy.isSecure()) { + if (!alreadySecured && !policy.isAuthenticated()) { m_log.warn("unable to establish security of assertion"); badtokens.push_back(decrypted); continue; diff --git a/shibsp/handler/impl/SAML2Logout.cpp b/shibsp/handler/impl/SAML2Logout.cpp index aae1079..4674931 100644 --- a/shibsp/handler/impl/SAML2Logout.cpp +++ b/shibsp/handler/impl/SAML2Logout.cpp @@ -329,7 +329,7 @@ pair SAML2Logout::doRequest( auto_ptr msg(m_decoder->decode(relayState, request, policy)); const LogoutRequest* logoutRequest = dynamic_cast(msg.get()); if (logoutRequest) { - if (!policy.isSecure()) + if (!policy.isAuthenticated()) throw SecurityPolicyException("Security of LogoutRequest not established."); // Message from IdP to logout one or more sessions. @@ -491,7 +491,7 @@ pair SAML2Logout::doRequest( // A LogoutResponse completes an SP-initiated logout sequence. const LogoutResponse* logoutResponse = dynamic_cast(msg.get()); if (logoutResponse) { - if (!policy.isSecure()) { + if (!policy.isAuthenticated()) { SecurityPolicyException ex("Security of LogoutResponse not established."); if (policy.getIssuerMetadata()) annotateException(&ex, policy.getIssuerMetadata()); // throws it