From 9e5f5fd6b4d0dfd3cb062e98dcb087640bf82414 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Thu, 9 Nov 2006 03:12:15 +0000 Subject: [PATCH] Moved dest. check back to decoders, policy API changes. --- .cdtproject | 3 +- .gitignore | 1 + saml/Makefile.am | 8 +- saml/binding/MessageRoutingRule.h | 79 --------------- saml/binding/SecurityPolicy.h | 97 ++++++++++++++++-- saml/binding/SecurityPolicyRule.h | 19 +--- .../{MessageSigningRule.h => XMLSigningRule.h} | 8 +- saml/binding/impl/MessageRoutingRule.cpp | 111 --------------------- saml/binding/impl/SecurityPolicy.cpp | 15 +-- .../{MessageSigningRule.cpp => XMLSigningRule.cpp} | 14 +-- saml/saml.vcproj | 24 ++--- saml/saml1/binding/impl/SAML1POSTDecoder.cpp | 12 +++ saml/saml2/binding/impl/SAML2POSTDecoder.cpp | 29 +++++- saml/saml2/binding/impl/SAML2RedirectDecoder.cpp | 28 +++++- samltest/binding.h | 3 +- 15 files changed, 188 insertions(+), 263 deletions(-) delete mode 100644 saml/binding/MessageRoutingRule.h rename saml/binding/{MessageSigningRule.h => XMLSigningRule.h} (91%) delete mode 100644 saml/binding/impl/MessageRoutingRule.cpp rename saml/binding/impl/{MessageSigningRule.cpp => XMLSigningRule.cpp} (94%) diff --git a/.cdtproject b/.cdtproject index 0de9cbd..4dbb011 100644 --- a/.cdtproject +++ b/.cdtproject @@ -60,7 +60,7 @@ - + @@ -88,6 +88,7 @@ + diff --git a/.gitignore b/.gitignore index c958642..15ded4b 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,4 @@ /opensaml.spec /pkginfo /stamp-h1 +/.settings diff --git a/saml/Makefile.am b/saml/Makefile.am index 5556709..e840ce5 100644 --- a/saml/Makefile.am +++ b/saml/Makefile.am @@ -40,13 +40,12 @@ samlbindinclude_HEADERS = \ binding/MessageDecoder.h \ binding/MessageEncoder.h \ binding/MessageFlowRule.h \ - binding/MessageRoutingRule.h \ - binding/MessageSigningRule.h \ binding/SAMLArtifact.h \ binding/SecurityPolicy.h \ binding/SecurityPolicyRule.h \ binding/SimpleSigningRule.h \ - binding/URLEncoder.h + binding/URLEncoder.h \ + binding/XMLSigningRule.h encinclude_HEADERS = \ encryption/EncryptedKeyResolver.h @@ -110,12 +109,11 @@ libsaml_la_SOURCES = \ binding/impl/MessageDecoder.cpp \ binding/impl/MessageEncoder.cpp \ binding/impl/MessageFlowRule.cpp \ - binding/impl/MessageRoutingRule.cpp \ - binding/impl/MessageSigningRule.cpp \ binding/impl/SAMLArtifact.cpp \ binding/impl/SecurityPolicy.cpp \ binding/impl/SimpleSigningRule.cpp \ binding/impl/URLEncoder.cpp \ + binding/impl/XMLSigningRule.cpp \ saml1/core/impl/AssertionsImpl.cpp \ saml1/core/impl/AssertionsSchemaValidators.cpp \ saml1/core/impl/ProtocolsImpl.cpp \ diff --git a/saml/binding/MessageRoutingRule.h b/saml/binding/MessageRoutingRule.h deleted file mode 100644 index d5b6e2e..0000000 --- a/saml/binding/MessageRoutingRule.h +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright 2001-2006 Internet2 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @file saml/binding/MessageRoutingRule.h - * - * Routing rule that enforces message delivery to an intended destination - */ - -#include - - -namespace opensaml { - /** - * Routing rule that enforces message delivery to an intended destination - * - * Subclasses can provide support for additional message types - * by overriding the destination derivation method. - */ - class SAML_API MessageRoutingRule : public SecurityPolicyRule - { - public: - /** - * Constructor. - * - * If an XML attribute named mandatory is set to "true" or "1", then - * a destination address MUST be present in the message. - * - * @param e DOM tree to initialize rule - */ - MessageRoutingRule(const DOMElement* e); - virtual ~MessageRoutingRule() {} - - std::pair evaluate( - const GenericRequest& request, - const xmltooling::XMLObject& message, - const saml2md::MetadataProvider* metadataProvider, - const xmltooling::QName* role, - const TrustEngine* trustEngine - ) const; - - /** - * Controls whether rule insists on presence of destination address in - * the message. - * - * @param mandatory flag value to set - */ - void setMandatory(bool mandatory) { - m_mandatory = mandatory; - } - - protected: - /** - * Examines the message and/or its contents and extracts the destination - * address/URL, if specified. - * - * @param message message to examine - * @return the destination address/URL, or NULL - */ - virtual const XMLCh* getDestination(const xmltooling::XMLObject& message) const; - - private: - bool m_mandatory; - }; - -}; diff --git a/saml/binding/SecurityPolicy.h b/saml/binding/SecurityPolicy.h index 67259ca..81f990e 100644 --- a/saml/binding/SecurityPolicy.h +++ b/saml/binding/SecurityPolicy.h @@ -54,6 +54,21 @@ namespace opensaml { /** * Constructor for policy. * + * @param metadataProvider locked MetadataProvider instance + * @param role identifies the role (generally IdP or SP) of the policy peer + * @param trustEngine TrustEngine to authenticate policy peer + */ + SecurityPolicy( + const saml2md::MetadataProvider* metadataProvider=NULL, + const xmltooling::QName* role=NULL, + const TrustEngine* trustEngine=NULL + ) : m_issuer(NULL), m_issuerRole(NULL), m_matchingPolicy(NULL), m_metadata(metadataProvider), + m_role(role ? *role : xmltooling::QName()), m_trust(trustEngine) { + } + + /** + * Constructor for policy using existing rules. + * * @param rules reference to array of policy rules to use * @param metadataProvider locked MetadataProvider instance * @param role identifies the role (generally IdP or SP) of the policy peer @@ -64,9 +79,10 @@ namespace opensaml { const saml2md::MetadataProvider* metadataProvider=NULL, const xmltooling::QName* role=NULL, const TrustEngine* trustEngine=NULL - ) : m_issuer(NULL), m_issuerRole(NULL), m_rules(rules), m_metadata(metadataProvider), + ) : m_issuer(NULL), m_issuerRole(NULL), m_matchingPolicy(NULL), m_rules(rules), m_metadata(metadataProvider), m_role(role ? *role : xmltooling::QName()), m_trust(trustEngine) { } + virtual ~SecurityPolicy(); /** @@ -97,6 +113,33 @@ namespace opensaml { } /** + * Sets a locked MetadataProvider for the policy. + * + * @param metadata a locked MetadataProvider or NULL + */ + void setMetadataProvider(const saml2md::MetadataProvider* metadata) { + m_metadata = metadata; + } + + /** + * Sets a peer role element/type for to the policy. + * + * @param role the peer role element/type or NULL + */ + void setRole(const xmltooling::QName* role) { + m_role = (role ? *role : xmltooling::QName()); + } + + /** + * Sets a TrustEngine for the policy. + * + * @param trust a TrustEngine or NULL + */ + void setTrustEngine(const TrustEngine* trust) { + m_trust = trust; + } + + /** * Evaluates the rule against the given request and message, * possibly populating issuer information in the policy object. * @@ -140,25 +183,59 @@ namespace opensaml { * @param issuerRole metadata for the role the issuer is operating in */ void setIssuerMetadata(const saml2md::RoleDescriptor* issuerRole); + + /** Allows override of rules for comparing saml2:Issuer information. */ + class SAML_API IssuerMatchingPolicy { + MAKE_NONCOPYABLE(IssuerMatchingPolicy); + public: + IssuerMatchingPolicy() {} + virtual ~IssuerMatchingPolicy() {} + + /** + * Returns true iff the two operands "match". Applications can override this method to + * support non-standard issuer matching for complex policies. + * + *

The default implementation does a basic comparison of the XML content, treating + * an unsupplied Format as an "entityID". + * + * @param issuer1 the first Issuer to match + * @param issuer2 the second Issuer to match + * @return true iff the operands match + */ + virtual bool issuerMatches(const saml2::Issuer* issuer1, const saml2::Issuer* issuer2) const; + }; - protected: /** - * Returns true iff the two operands "match". Applications can override this method to - * support non-standard issuer matching for complex policies. + * Returns the IssuerMatchingPolicy in effect. * - *

The default implementation does a basic comparison of the XML content, treating - * an unsupplied Format as "entityID". + * @return the effective IssuerMatchingPolicy + */ + const IssuerMatchingPolicy* getIssuerMatchingPolicy() const { + return m_matchingPolicy ? m_matchingPolicy : &m_defaultMatching; + } + + /** + * Sets the IssuerMatchingPolicy in effect. Setting no policy will + * cause the simple, default approach to be used. * - * @param issuer1 the first Issuer to match - * @param issuer2 the second Issuer to match - * @return true iff the operands match + *

The matching object will be freed by the SecurityPolicy. + * + * @param matchingPolicy the IssuerMatchingPolicy to use */ - virtual bool issuerMatches(const saml2::Issuer* issuer1, const saml2::Issuer* issuer2) const; + void getIssuerMatchingPolicy(IssuerMatchingPolicy* matchingPolicy) { + delete m_matchingPolicy; + m_matchingPolicy = matchingPolicy; + } + + protected: + /** A shared matching object that just supports the default matching rules. */ + static IssuerMatchingPolicy m_defaultMatching; private: saml2::Issuer* m_issuer; const saml2md::RoleDescriptor* m_issuerRole; + IssuerMatchingPolicy* m_matchingPolicy; std::vector m_rules; const saml2md::MetadataProvider* m_metadata; xmltooling::QName m_role; diff --git a/saml/binding/SecurityPolicyRule.h b/saml/binding/SecurityPolicyRule.h index 668d097..0222017 100644 --- a/saml/binding/SecurityPolicyRule.h +++ b/saml/binding/SecurityPolicyRule.h @@ -93,13 +93,12 @@ namespace opensaml { #define MESSAGEFLOW_POLICY_RULE "org.opensaml.binding.MessageFlowRule" /** - * SecurityPolicyRule for ensuring messages are delivered to the right place. + * SecurityPolicyRule for protocol message "blob" signing. * - *

Enforcement is mandatory and the message must be explicitly addressed, - * unless a "mandatory" XML attribute is set to "0" or "false" when instantiating - * the policy rule. + * Allows the message issuer to be authenticated using a non-XML digital signature + * over the message body. The transport layer is not considered. */ - #define MESSAGEROUTING_POLICY_RULE "org.opensaml.binding.MessageRoutingRule" + #define SIMPLESIGNING_POLICY_RULE "org.opensaml.binding.SimpleSigningRule" /** * SecurityPolicyRule for protocol message XML signing. @@ -107,15 +106,7 @@ namespace opensaml { * Allows the message issuer to be authenticated using an XML digital signature * over the message. The transport layer is not considered. */ - #define MESSAGESIGNING_POLICY_RULE "org.opensaml.binding.MessageSigningRule" - - /** - * SecurityPolicyRule for protocol message "blob" signing. - * - * Allows the message issuer to be authenticated using a non-XML digital signature - * over the message body. The transport layer is not considered. - */ - #define SIMPLESIGNING_POLICY_RULE "org.opensaml.binding.SimpleSigningRule" + #define XMLSIGNING_POLICY_RULE "org.opensaml.binding.XMLSigningRule" }; #endif /* __saml_secrule_h__ */ diff --git a/saml/binding/MessageSigningRule.h b/saml/binding/XMLSigningRule.h similarity index 91% rename from saml/binding/MessageSigningRule.h rename to saml/binding/XMLSigningRule.h index 1fa0581..f4ff85e 100644 --- a/saml/binding/MessageSigningRule.h +++ b/saml/binding/XMLSigningRule.h @@ -15,7 +15,7 @@ */ /** - * @file saml/binding/MessageSigningRule.h + * @file saml/binding/XMLSigningRule.h * * XML Signature checking SecurityPolicyRule */ @@ -30,11 +30,11 @@ namespace opensaml { * Subclasses can provide support for additional message types * by overriding the issuer derivation method. */ - class SAML_API MessageSigningRule : public SecurityPolicyRule + class SAML_API XMLSigningRule : public SecurityPolicyRule { public: - MessageSigningRule(const DOMElement* e) {} - virtual ~MessageSigningRule() {} + XMLSigningRule(const DOMElement* e) {} + virtual ~XMLSigningRule() {} std::pair evaluate( const GenericRequest& request, diff --git a/saml/binding/impl/MessageRoutingRule.cpp b/saml/binding/impl/MessageRoutingRule.cpp deleted file mode 100644 index 60b297f..0000000 --- a/saml/binding/impl/MessageRoutingRule.cpp +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright 2001-2006 Internet2 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * MessageRoutingRule.cpp - * - * XML Signature checking SecurityPolicyRule - */ - -#include "internal.h" -#include "exceptions.h" -#include "binding/HTTPRequest.h" -#include "binding/MessageRoutingRule.h" -#include "saml1/core/Protocols.h" -#include "saml2/core/Protocols.h" - -#include -#include -#include - -using namespace opensaml; -using namespace xmltooling; -using namespace log4cpp; -using namespace std; - -namespace opensaml { - SecurityPolicyRule* SAML_DLLLOCAL MessageRoutingRuleFactory(const DOMElement* const & e) - { - return new MessageRoutingRule(e); - } -}; - -static const XMLCh mandatory[] = UNICODE_LITERAL_9(m,a,n,d,a,t,o,r,y); - -MessageRoutingRule::MessageRoutingRule(const DOMElement* e) : m_mandatory(false) -{ - if (e) { - const XMLCh* attr = e->getAttributeNS(NULL, mandatory); - if (attr && (*attr==chLatin_t || *attr==chDigit_1)) - m_mandatory = true; - } -} - -pair MessageRoutingRule::evaluate( - const GenericRequest& request, - const XMLObject& message, - const saml2md::MetadataProvider* metadataProvider, - const QName* role, - const opensaml::TrustEngine* trustEngine - ) const -{ - Category& log=Category::getInstance(SAML_LOGCAT".SecurityPolicyRule.MessageRouting"); - log.debug("evaluating message routing policy"); - - try { - const char* to = dynamic_cast(request).getRequestURL(); - if (!to || !*to) { - if (m_mandatory) - throw BindingException("Unable to determine delivery location."); - log.debug("unable to determine delivery location, ignoring message"); - return pair(NULL,NULL); - } - auto_ptr_char dest(getDestination(message)); - if (dest.get() && *dest.get()) { - if (!XMLString::equals(to, dest.get())) { - log.error("Message intended for (%s), but delivered to (%s)", dest.get(), to); - throw BindingException("Message delivered to incorrect address."); - } - } - else if (m_mandatory) - throw BindingException("Message did not contain intended address."); - } - catch (bad_cast&) { - throw BindingException("Message was not of a recognized type."); - } - return pair(NULL,NULL); -} - -const XMLCh* MessageRoutingRule::getDestination(const XMLObject& message) const -{ - // We just let any bad casts throw here. - - // Shortcuts some of the casting. - const XMLCh* ns = message.getElementQName().getNamespaceURI(); - if (ns) { - if (XMLString::equals(ns, samlconstants::SAML20P_NS)) { - const saml2p::StatusResponseType* response = dynamic_cast(&message); - if (response) - return response->getDestination(); - return dynamic_cast(message).getDestination(); - } - else if (XMLString::equals(ns, samlconstants::SAML1P_NS)) { - // Should be a samlp:Response, at least in OpenSAML. - return dynamic_cast(message).getRecipient(); - } - } - return NULL; -} diff --git a/saml/binding/impl/SecurityPolicy.cpp b/saml/binding/impl/SecurityPolicy.cpp index 882dfac..437241a 100644 --- a/saml/binding/impl/SecurityPolicy.cpp +++ b/saml/binding/impl/SecurityPolicy.cpp @@ -33,22 +33,23 @@ using namespace std; namespace opensaml { SAML_DLLLOCAL PluginManager::Factory MessageFlowRuleFactory; - SAML_DLLLOCAL PluginManager::Factory MessageRoutingRuleFactory; - SAML_DLLLOCAL PluginManager::Factory MessageSigningRuleFactory; SAML_DLLLOCAL PluginManager::Factory SimpleSigningRuleFactory; + SAML_DLLLOCAL PluginManager::Factory XMLSigningRuleFactory; }; void SAML_API opensaml::registerSecurityPolicyRules() { SAMLConfig& conf=SAMLConfig::getConfig(); conf.SecurityPolicyRuleManager.registerFactory(MESSAGEFLOW_POLICY_RULE, MessageFlowRuleFactory); - conf.SecurityPolicyRuleManager.registerFactory(MESSAGEROUTING_POLICY_RULE, MessageRoutingRuleFactory); - conf.SecurityPolicyRuleManager.registerFactory(MESSAGESIGNING_POLICY_RULE, MessageSigningRuleFactory); conf.SecurityPolicyRuleManager.registerFactory(SIMPLESIGNING_POLICY_RULE, SimpleSigningRuleFactory); + conf.SecurityPolicyRuleManager.registerFactory(XMLSIGNING_POLICY_RULE, XMLSigningRuleFactory); } +SecurityPolicy::IssuerMatchingPolicy SecurityPolicy::m_defaultMatching; + SecurityPolicy::~SecurityPolicy() { + delete m_matchingPolicy; delete m_issuer; } @@ -62,7 +63,7 @@ void SecurityPolicy::evaluate(const GenericRequest& request, const XMLObject& me // Make sure returned issuer doesn't conflict. if (ident.first) { - if (!issuerMatches(ident.first, m_issuer)) { + if (!(m_matchingPolicy ? m_matchingPolicy : &m_defaultMatching)->issuerMatches(ident.first, m_issuer)) { delete ident.first; throw BindingException("Policy rules returned differing Issuers."); } @@ -80,7 +81,7 @@ void SecurityPolicy::evaluate(const GenericRequest& request, const XMLObject& me void SecurityPolicy::setIssuer(saml2::Issuer* issuer) { - if (!issuerMatches(issuer, m_issuer)) { + if (!((m_matchingPolicy ? m_matchingPolicy : &m_defaultMatching))->issuerMatches(issuer, m_issuer)) { delete issuer; throw BindingException("Externally provided Issuer conflicts with policy results."); } @@ -96,7 +97,7 @@ void SecurityPolicy::setIssuerMetadata(const RoleDescriptor* issuerRole) m_issuerRole=issuerRole; } -bool SecurityPolicy::issuerMatches(const Issuer* issuer1, const Issuer* issuer2) const +bool SecurityPolicy::IssuerMatchingPolicy::issuerMatches(const Issuer* issuer1, const Issuer* issuer2) const { // NULL matches anything for the purposes of this interface. if (!issuer1 || !issuer2) diff --git a/saml/binding/impl/MessageSigningRule.cpp b/saml/binding/impl/XMLSigningRule.cpp similarity index 94% rename from saml/binding/impl/MessageSigningRule.cpp rename to saml/binding/impl/XMLSigningRule.cpp index 6b9eedf..cad0e6d 100644 --- a/saml/binding/impl/MessageSigningRule.cpp +++ b/saml/binding/impl/XMLSigningRule.cpp @@ -15,7 +15,7 @@ */ /** - * MessageSigningRule.cpp + * XMLSigningRule.cpp * * XML Signature checking SecurityPolicyRule */ @@ -23,7 +23,7 @@ #include "internal.h" #include "exceptions.h" #include "RootObject.h" -#include "binding/MessageSigningRule.h" +#include "binding/XMLSigningRule.h" #include "saml1/core/Assertions.h" #include "saml1/core/Protocols.h" #include "saml2/core/Protocols.h" @@ -42,13 +42,13 @@ using namespace log4cpp; using namespace std; namespace opensaml { - SecurityPolicyRule* SAML_DLLLOCAL MessageSigningRuleFactory(const DOMElement* const & e) + SecurityPolicyRule* SAML_DLLLOCAL XMLSigningRuleFactory(const DOMElement* const & e) { - return new MessageSigningRule(e); + return new XMLSigningRule(e); } }; -pair MessageSigningRule::evaluate( +pair XMLSigningRule::evaluate( const GenericRequest& request, const XMLObject& message, const MetadataProvider* metadataProvider, @@ -56,7 +56,7 @@ pair MessageSigningRule::evaluate const opensaml::TrustEngine* trustEngine ) const { - Category& log=Category::getInstance(SAML_LOGCAT".SecurityPolicyRule.MessageSigning"); + Category& log=Category::getInstance(SAML_LOGCAT".SecurityPolicyRule.XMLSigning"); log.debug("evaluating message signing policy"); pair ret = pair(NULL,NULL); @@ -118,7 +118,7 @@ pair MessageSigningRule::evaluate return ret; } -pair MessageSigningRule::getIssuerAndProtocol(const XMLObject& message) const +pair XMLSigningRule::getIssuerAndProtocol(const XMLObject& message) const { // We just let any bad casts throw here. diff --git a/saml/saml.vcproj b/saml/saml.vcproj index 39548a4..d6ca290 100644 --- a/saml/saml.vcproj +++ b/saml/saml.vcproj @@ -486,14 +486,6 @@ > - - - - @@ -509,6 +501,10 @@ RelativePath=".\binding\impl\URLEncoder.cpp" > + + - - - - @@ -832,6 +820,10 @@ RelativePath=".\binding\URLEncoder.h" > + + getRecipient()); + const char* recipient2 = httpRequest->getRequestURL(); + if (!recipient.get() || !*(recipient.get())) { + log.error("response missing Recipient attribute"); + throw BindingException("SAML response did not contain Recipient attribute identifying intended destination."); + } + else if (!recipient2 || !*recipient2 || strcmp(recipient.get(),recipient2)) { + log.error("POST targeted at (%s), but delivered to (%s)", recipient.get(), recipient2 ? recipient2 : "none"); + throw BindingException("SAML message delivered with POST to incorrect server URL."); + } + // Run through the policy. policy.evaluate(genericRequest, *response); } diff --git a/saml/saml2/binding/impl/SAML2POSTDecoder.cpp b/saml/saml2/binding/impl/SAML2POSTDecoder.cpp index 3d4cb42..f216a8d 100644 --- a/saml/saml2/binding/impl/SAML2POSTDecoder.cpp +++ b/saml/saml2/binding/impl/SAML2POSTDecoder.cpp @@ -100,14 +100,35 @@ saml2::RootObject* SAML2POSTDecoder::decode( auto_ptr xmlObject(XMLObjectBuilder::buildOneFromElement(doc->getDocumentElement(), true)); janitor.release(); - saml2::RootObject* root = dynamic_cast(xmlObject.get()); - if (!root) - throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); + saml2::RootObject* root = NULL; + StatusResponseType* response = NULL; + RequestAbstractType* request = dynamic_cast(xmlObject.get()); + if (!request) { + response = dynamic_cast(xmlObject.get()); + if (!response) + throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); + root = static_cast(response); + } + else { + root = static_cast(request); + } try { if (!m_validate) SchemaValidators.validate(xmlObject.get()); + // Check destination URL. + auto_ptr_char dest(request ? request->getDestination() : response->getDestination()); + const char* dest2 = httpRequest->getRequestURL(); + if ((root->getSignature() || httpRequest->getParameter("Signature")) && !dest.get() || !*(dest.get())) { + log.error("signed SAML message missing Destination attribute"); + throw BindingException("Signed SAML message missing Destination attribute identifying intended destination."); + } + else if (dest.get() && (!dest2 || !*dest2 || strcmp(dest.get(),dest2))) { + log.error("POST targeted at (%s), but delivered to (%s)", dest.get(), dest2 ? dest2 : "none"); + throw BindingException("SAML message delivered with POST to incorrect server URL."); + } + // Run through the policy. policy.evaluate(genericRequest, *root); } @@ -119,7 +140,7 @@ saml2::RootObject* SAML2POSTDecoder::decode( const Issuer* claimedIssuer = root->getIssuer(); if (!claimedIssuer) { // Check for assertions. - const Response* assbag = dynamic_cast(root); + const Response* assbag = dynamic_cast(response); if (assbag) { const vector& assertions=assbag->getAssertions(); if (!assertions.empty()) diff --git a/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp b/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp index 9efd6e4..4ea64ff 100644 --- a/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp +++ b/saml/saml2/binding/impl/SAML2RedirectDecoder.cpp @@ -114,14 +114,36 @@ XMLObject* SAML2RedirectDecoder::decode( auto_ptr xmlObject(XMLObjectBuilder::buildOneFromElement(doc->getDocumentElement(), true)); janitor.release(); - saml2::RootObject* root = dynamic_cast(xmlObject.get()); - if (!root) - throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); + saml2::RootObject* root = NULL; + StatusResponseType* response = NULL; + RequestAbstractType* request = dynamic_cast(xmlObject.get()); + if (!request) { + response = dynamic_cast(xmlObject.get()); + if (!response) + throw BindingException("XML content for SAML 2.0 HTTP-POST Decoder must be a SAML 2.0 protocol message."); + root = static_cast(response); + } + else { + root = static_cast(request); + } + try { if (!m_validate) SchemaValidators.validate(xmlObject.get()); + // Check destination URL. + auto_ptr_char dest(request ? request->getDestination() : response->getDestination()); + const char* dest2 = httpRequest->getRequestURL(); + if ((root->getSignature() || httpRequest->getParameter("Signature")) && !dest.get() || !*(dest.get())) { + log.error("signed SAML message missing Destination attribute"); + throw BindingException("Signed SAML message missing Destination attribute identifying intended destination."); + } + else if (dest.get() && (!dest2 || !*dest2 || strcmp(dest.get(),dest2))) { + log.error("Redirect targeted at (%s), but delivered to (%s)", dest.get(), dest2 ? dest2 : "none"); + throw BindingException("SAML message delivered with Redirect to incorrect server URL."); + } + // Run through the policy. policy.evaluate(genericRequest, *root); } diff --git a/samltest/binding.h b/samltest/binding.h index d92280f..aa0bff9 100644 --- a/samltest/binding.h +++ b/samltest/binding.h @@ -78,9 +78,8 @@ public: m_trust = SAMLConfig::getConfig().TrustEngineManager.newPlugin(EXPLICIT_KEY_SAMLTRUSTENGINE, NULL); m_rules.push_back(SAMLConfig::getConfig().SecurityPolicyRuleManager.newPlugin(MESSAGEFLOW_POLICY_RULE,NULL)); - m_rules.push_back(SAMLConfig::getConfig().SecurityPolicyRuleManager.newPlugin(MESSAGEROUTING_POLICY_RULE,NULL)); - m_rules.push_back(SAMLConfig::getConfig().SecurityPolicyRuleManager.newPlugin(MESSAGESIGNING_POLICY_RULE,NULL)); m_rules.push_back(SAMLConfig::getConfig().SecurityPolicyRuleManager.newPlugin(SIMPLESIGNING_POLICY_RULE,NULL)); + m_rules.push_back(SAMLConfig::getConfig().SecurityPolicyRuleManager.newPlugin(XMLSIGNING_POLICY_RULE,NULL)); } catch (XMLToolingException& ex) { TS_TRACE(ex.what()); -- 2.1.4