From bce5046d6d2fd42e4b2b270bb2db557acfd4581c Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Thu, 5 Oct 2006 19:30:39 +0000 Subject: [PATCH] Unit test for 1.x POST binding, plus fixes. --- .cdtproject | 3 +- saml/saml1/binding/impl/SAML1POSTDecoder.cpp | 57 +++++++-- samltest/Makefile.am | 2 + samltest/binding.h | 101 +++++++++++++++ samltest/data/binding/ExampleMetadataProvider.xml | 2 + samltest/data/binding/example-metadata.xml | 92 ++++++++++++++ samltest/data/saml1/binding/SAML1Response.xml | 10 ++ samltest/internal.h | 3 +- samltest/saml1/binding/SAML1POSTTest.h | 145 ++++++++++++++++++++++ samltest/samltest.h | 5 +- samltest/samltest.vcproj | 50 +++++++- 11 files changed, 451 insertions(+), 19 deletions(-) create mode 100644 samltest/binding.h create mode 100644 samltest/data/binding/ExampleMetadataProvider.xml create mode 100644 samltest/data/binding/example-metadata.xml create mode 100644 samltest/data/saml1/binding/SAML1Response.xml create mode 100644 samltest/saml1/binding/SAML1POSTTest.h diff --git a/.cdtproject b/.cdtproject index 7827afa..0361b54 100644 --- a/.cdtproject +++ b/.cdtproject @@ -81,11 +81,12 @@ - + + diff --git a/saml/saml1/binding/impl/SAML1POSTDecoder.cpp b/saml/saml1/binding/impl/SAML1POSTDecoder.cpp index b99ab68..181d6f9 100644 --- a/saml/saml1/binding/impl/SAML1POSTDecoder.cpp +++ b/saml/saml1/binding/impl/SAML1POSTDecoder.cpp @@ -105,54 +105,84 @@ Response* SAML1POSTDecoder::decode( // Check recipient URL. auto_ptr_char recipient(response->getRecipient()); const char* recipient2 = httpRequest.getRequestURL(); - if (!recipient2 || !*recipient2 || strcmp(recipient.get(),recipient2)) { + 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."); } + // Check freshness. time_t now = time(NULL); if (response->getIssueInstant()->getEpoch() < now-(2*XMLToolingConfig::getConfig().clock_skew_secs)) throw BindingException("Detected expired POST profile response."); - + + // Check replay. ReplayCache* replayCache = SAMLConfig::getConfig().getReplayCache(); if (replayCache) { auto_ptr_char id(response->getResponseID()); - if (!replayCache->check("SAML1POST", id.get(), response->getIssueInstant()->getEpoch() + (2*XMLToolingConfig::getConfig().clock_skew_secs))) + if (!replayCache->check("SAML1POST", id.get(), response->getIssueInstant()->getEpoch() + (2*XMLToolingConfig::getConfig().clock_skew_secs))) { + log.error("replay detected of response ID (%s)", id.get()); throw BindingException("Rejecting replayed response ID ($1).", params(1,id.get())); + } } else log.warn("replay cache was not provided, this is a serious security risk!"); + /* For SAML 1, the issuer can only be established from any assertions in the message. + * Generally, errors aren't delivered like this, so there should be one. + * The Issuer attribute is matched against metadata, and then trust checking can be + * applied. + */ issuer = NULL; issuerTrusted = false; log.debug("attempting to establish issuer and integrity of message..."); const vector& assertions=const_cast(response)->getAssertions(); if (!assertions.empty()) { + log.debug("searching metadata for assertion issuer..."); const EntityDescriptor* provider= metadataProvider ? metadataProvider->getEntityDescriptor(assertions.front()->getIssuer()) : NULL; if (provider) { + log.debug("matched assertion issuer against metadata, searching for applicable role..."); pair minor = response->getMinorVersion(); issuer=provider->getRoleDescriptor( *role, (minor.first && minor.second==0) ? SAMLConstants::SAML10_PROTOCOL_ENUM : SAMLConstants::SAML11_PROTOCOL_ENUM ); - if (issuer && trustEngine && response->getSignature()) { - issuerTrusted = static_cast(trustEngine)->validate( - *(response->getSignature()), *issuer, metadataProvider->getKeyResolver() + if (issuer) { + if (trustEngine && response->getSignature()) { + issuerTrusted = static_cast(trustEngine)->validate( + *(response->getSignature()), *issuer, metadataProvider->getKeyResolver() + ); + if (!issuerTrusted) + log.error("unable to verify signature on message with supplied trust engine"); + } + else { + log.warn("unable to verify integrity of the message, leaving untrusted"); + } + } + else { + log.warn( + "unable to find compatible SAML 1.%d role (%s) in metadata", + (minor.first && minor.second==0) ? 0 : 1, + role->toString().c_str() ); - if (!issuerTrusted) - log.error("signature on message could not be verified by supplied trust engine"); } if (log.isDebugEnabled()) { auto_ptr_char iname(assertions.front()->getIssuer()); log.debug("message from (%s), integrity %sverified", iname.get(), issuerTrusted ? "" : "NOT "); } } - else - log.warn("no metadata provider supplied, can't establish identity of issuer"); + else { + auto_ptr_char temp(assertions.front()->getIssuer()); + log.warn("no metadata found, can't establish identity of issuer (%s)", temp.get()); + } } - else + else { log.warn("no assertions found, can't establish identity of issuer"); + } } catch (XMLToolingException& ex) { // Check for an Issuer. @@ -169,10 +199,11 @@ Response* SAML1POSTDecoder::decode( const EntityDescriptor* provider=metadataProvider->getEntityDescriptor(assertions.front()->getIssuer(),false); if (provider) { pair minor = response->getMinorVersion(); - const IDPSSODescriptor* role=provider->getIDPSSODescriptor( + issuer=provider->getRoleDescriptor( + *role, (minor.first && minor.second==0) ? SAMLConstants::SAML10_PROTOCOL_ENUM : SAMLConstants::SAML11_PROTOCOL_ENUM ); - if (role) annotateException(&ex,role); // throws it + if (issuer) annotateException(&ex,issuer); // throws it annotateException(&ex,provider); // throws it } } diff --git a/samltest/Makefile.am b/samltest/Makefile.am index 50e65a2..f4f7be6 100644 --- a/samltest/Makefile.am +++ b/samltest/Makefile.am @@ -20,6 +20,7 @@ samltest_h = \ signature/SAML2AssertionTest.h \ security/AbstractPKIXTrustEngineTest.h \ security/ExplicitKeyTrustEngineTest.h \ + saml1/binding/impl/SAML1POSTTest.h \ saml1/core/impl/ActionTest.h \ saml1/core/impl/AdviceTest.h \ saml1/core/impl/AssertionIDReferenceTest.h \ @@ -90,6 +91,7 @@ samltest_h = \ saml2/metadata/FilesystemMetadataProviderTest.h noinst_HEADERS = \ + binding.h \ internal.h \ signature/SAMLSignatureTestBase.h diff --git a/samltest/binding.h b/samltest/binding.h new file mode 100644 index 0000000..474e882 --- /dev/null +++ b/samltest/binding.h @@ -0,0 +1,101 @@ +/* + * 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. + */ + +#include "internal.h" + +#include +#include +#include +#include +#include + +using namespace saml2md; +using namespace xmlsignature; + +class SAMLBindingBaseTestCase : public MessageDecoder::HTTPRequest +{ +protected: + CredentialResolver* m_creds; + MetadataProvider* m_metadata; + opensaml::X509TrustEngine* m_trust; + map m_fields; + +public: + void setUp() { + m_creds=NULL; + m_metadata=NULL; + m_trust=NULL; + m_fields.clear(); + + try { + string config = data_path + "binding/ExampleMetadataProvider.xml"; + ifstream in(config.c_str()); + DOMDocument* doc=XMLToolingConfig::getConfig().getParser().parse(in); + XercesJanitor janitor(doc); + + auto_ptr_XMLCh path("path"); + string s = data_path + "binding/example-metadata.xml"; + auto_ptr_XMLCh file(s.c_str()); + doc->getDocumentElement()->setAttributeNS(NULL,path.get(),file.get()); + + m_metadata = SAMLConfig::getConfig().MetadataProviderManager.newPlugin( + FILESYSTEM_METADATA_PROVIDER,doc->getDocumentElement() + ); + m_metadata->init(); + + config = data_path + "FilesystemCredentialResolver.xml"; + ifstream in2(config.c_str()); + DOMDocument* doc2=XMLToolingConfig::getConfig().getParser().parse(in2); + XercesJanitor janitor2(doc2); + m_creds = XMLToolingConfig::getConfig().CredentialResolverManager.newPlugin( + FILESYSTEM_CREDENTIAL_RESOLVER,doc2->getDocumentElement() + ); + + m_trust = dynamic_cast( + SAMLConfig::getConfig().TrustEngineManager.newPlugin(EXPLICIT_KEY_SAMLTRUSTENGINE, NULL) + ); + } + catch (XMLToolingException& ex) { + TS_TRACE(ex.what()); + tearDown(); + throw; + } + + } + + void tearDown() { + delete m_creds; + delete m_metadata; + delete m_trust; + m_creds=NULL; + m_metadata=NULL; + m_trust=NULL; + m_fields.clear(); + } + + const char* getParameter(const char* name) const { + map::const_iterator i=m_fields.find(name); + return i==m_fields.end() ? NULL : i->second.c_str(); + } + + vector::size_type getParameters(const char* name, vector& values) const { + values.clear(); + map::const_iterator i=m_fields.find(name); + if (i!=m_fields.end()) + values.push_back(i->second.c_str()); + return values.size(); + } +}; diff --git a/samltest/data/binding/ExampleMetadataProvider.xml b/samltest/data/binding/ExampleMetadataProvider.xml new file mode 100644 index 0000000..c296c1b --- /dev/null +++ b/samltest/data/binding/ExampleMetadataProvider.xml @@ -0,0 +1,2 @@ + + diff --git a/samltest/data/binding/example-metadata.xml b/samltest/data/binding/example-metadata.xml new file mode 100644 index 0000000..c67283b --- /dev/null +++ b/samltest/data/binding/example-metadata.xml @@ -0,0 +1,92 @@ + + + + + + + + + + MIICjzCCAfigAwIBAgIJAKk8t1hYcMkhMA0GCSqGSIb3DQEBBAUAMDoxCzAJBgNV + BAYTAlVTMRIwEAYDVQQKEwlJbnRlcm5ldDIxFzAVBgNVBAMTDnNwLmV4YW1wbGUu + b3JnMB4XDTA1MDYyMDE1NDgzNFoXDTMyMTEwNTE1NDgzNFowOjELMAkGA1UEBhMC + VVMxEjAQBgNVBAoTCUludGVybmV0MjEXMBUGA1UEAxMOc3AuZXhhbXBsZS5vcmcw + gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANlZ1L1mKzYbUVKiMQLhZlfGDyYa + /jjCiaXP0WhLNgvJpOTeajvsrApYNnFX5MLNzuC3NeQIjXUNLN2Yo2MCSthBIOL5 + qE5dka4z9W9zytoflW1LmJ8vXpx8Ay/meG4z//J5iCpYVEquA0xl28HUIlownZUF + 7w7bx0cF/02qrR23AgMBAAGjgZwwgZkwHQYDVR0OBBYEFJZiO1qsyAyc3HwMlL9p + JpN6fbGwMGoGA1UdIwRjMGGAFJZiO1qsyAyc3HwMlL9pJpN6fbGwoT6kPDA6MQsw + CQYDVQQGEwJVUzESMBAGA1UEChMJSW50ZXJuZXQyMRcwFQYDVQQDEw5zcC5leGFt + cGxlLm9yZ4IJAKk8t1hYcMkhMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQAD + gYEAMFq/UeSQyngE0GpZueyD2UW0M358uhseYOgGEIfm+qXIFQF6MYwNoX7WFzhC + LJZ2E6mEvZZFHCHUtl7mGDvsRwgZ85YCtRbvleEpqfgNQToto9pLYe+X6vvH9Z6p + gmYsTmak+kxO93JprrOd9xp8aZPMEprL7VCdrhbZEfyYER0= + + + + + + + + + + Example Identity Provider + Identities 'R' Us + http://idp.example.org/ + + + Technical Support + support@idp.example.org + + + + + + + + + + + + MIICjzCCAfigAwIBAgIJAKk8t1hYcMkhMA0GCSqGSIb3DQEBBAUAMDoxCzAJBgNV + BAYTAlVTMRIwEAYDVQQKEwlJbnRlcm5ldDIxFzAVBgNVBAMTDnNwLmV4YW1wbGUu + b3JnMB4XDTA1MDYyMDE1NDgzNFoXDTMyMTEwNTE1NDgzNFowOjELMAkGA1UEBhMC + VVMxEjAQBgNVBAoTCUludGVybmV0MjEXMBUGA1UEAxMOc3AuZXhhbXBsZS5vcmcw + gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANlZ1L1mKzYbUVKiMQLhZlfGDyYa + /jjCiaXP0WhLNgvJpOTeajvsrApYNnFX5MLNzuC3NeQIjXUNLN2Yo2MCSthBIOL5 + qE5dka4z9W9zytoflW1LmJ8vXpx8Ay/meG4z//J5iCpYVEquA0xl28HUIlownZUF + 7w7bx0cF/02qrR23AgMBAAGjgZwwgZkwHQYDVR0OBBYEFJZiO1qsyAyc3HwMlL9p + JpN6fbGwMGoGA1UdIwRjMGGAFJZiO1qsyAyc3HwMlL9pJpN6fbGwoT6kPDA6MQsw + CQYDVQQGEwJVUzESMBAGA1UEChMJSW50ZXJuZXQyMRcwFQYDVQQDEw5zcC5leGFt + cGxlLm9yZ4IJAKk8t1hYcMkhMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQAD + gYEAMFq/UeSQyngE0GpZueyD2UW0M358uhseYOgGEIfm+qXIFQF6MYwNoX7WFzhC + LJZ2E6mEvZZFHCHUtl7mGDvsRwgZ85YCtRbvleEpqfgNQToto9pLYe+X6vvH9Z6p + gmYsTmak+kxO93JprrOd9xp8aZPMEprL7VCdrhbZEfyYER0= + + + + + + + + + + + Example Service Provider + Services 'R' Us + http://sp.example.org/ + + + Technical Support + support@sp.example.org + + + + + diff --git a/samltest/data/saml1/binding/SAML1Response.xml b/samltest/data/saml1/binding/SAML1Response.xml new file mode 100644 index 0000000..5671c67 --- /dev/null +++ b/samltest/data/saml1/binding/SAML1Response.xml @@ -0,0 +1,10 @@ + + + + + John Doe + + + diff --git a/samltest/internal.h b/samltest/internal.h index 15b4afb..1cfe28f 100644 --- a/samltest/internal.h +++ b/samltest/internal.h @@ -19,11 +19,10 @@ #include #include #include -#include -#include #include #include #include +#include using namespace opensaml; using namespace xmltooling; diff --git a/samltest/saml1/binding/SAML1POSTTest.h b/samltest/saml1/binding/SAML1POSTTest.h new file mode 100644 index 0000000..2d6da7a --- /dev/null +++ b/samltest/saml1/binding/SAML1POSTTest.h @@ -0,0 +1,145 @@ +/* + * Copyright 2001-2005 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. + */ + +#include "binding.h" + +#include + +using namespace opensaml::saml1p; +using namespace opensaml::saml1; + +class SAML1POSTTest : public CxxTest::TestSuite, public SAMLBindingBaseTestCase { +public: + void setUp() { + m_fields.clear(); + SAMLBindingBaseTestCase::setUp(); + } + + void tearDown() { + m_fields.clear(); + SAMLBindingBaseTestCase::tearDown(); + } + + void testSAML1POSTTrusted() { + try { + // Read message to use from file. + string path = data_path + "saml1/binding/SAML1Response.xml"; + ifstream in(path.c_str()); + DOMDocument* doc=XMLToolingConfig::getConfig().getParser().parse(in); + XercesJanitor janitor(doc); + auto_ptr toSend( + dynamic_cast(XMLObjectBuilder::buildOneFromElement(doc->getDocumentElement(),true)) + ); + janitor.release(); + + // Freshen timestamp. + toSend->setIssueInstant(time(NULL)); + + // Encode message. + auto_ptr encoder(SAMLConfig::getConfig().MessageEncoderManager.newPlugin(SAML1_POST_ENCODER, NULL)); + encoder->encode(m_fields,toSend.get(),"https://sp.example.org/","state",m_creds); + toSend.release(); + + // Decode message. + string relayState; + const RoleDescriptor* issuer=NULL; + bool trusted=false; + QName idprole(SAMLConstants::SAML20MD_NS, IDPSSODescriptor::LOCAL_NAME); + auto_ptr decoder(SAMLConfig::getConfig().MessageDecoderManager.newPlugin(SAML1_POST_DECODER, NULL)); + auto_ptr response( + dynamic_cast( + decoder->decode(relayState,issuer,trusted,*this,m_metadata,&idprole,m_trust) + ) + ); + + // Test the results. + TSM_ASSERT_EQUALS("TARGET was not the expected result.", relayState, "state"); + TSM_ASSERT("SAML Response not decoded successfully.", response.get()); + TSM_ASSERT("Message was not verified.", issuer && trusted); + auto_ptr_char entityID(dynamic_cast(issuer->getParent())->getEntityID()); + TSM_ASSERT("Issuer was not expected.", !strcmp(entityID.get(),"https://idp.example.org/")); + TSM_ASSERT_EQUALS("Assertion count was not correct.", response->getAssertions().size(), 1); + } + catch (XMLToolingException& ex) { + TS_TRACE(ex.what()); + throw; + } + } + + void testSAML1POSTUntrusted() { + try { + // Read message to use from file. + string path = data_path + "saml1/binding/SAML1Response.xml"; + ifstream in(path.c_str()); + DOMDocument* doc=XMLToolingConfig::getConfig().getParser().parse(in); + XercesJanitor janitor(doc); + auto_ptr toSend( + dynamic_cast(XMLObjectBuilder::buildOneFromElement(doc->getDocumentElement(),true)) + ); + janitor.release(); + + // Freshen timestamp and clear ID. + toSend->setIssueInstant(time(NULL)); + toSend->setResponseID(NULL); + + // Encode message. + auto_ptr encoder(SAMLConfig::getConfig().MessageEncoderManager.newPlugin(SAML1_POST_ENCODER, NULL)); + encoder->encode(m_fields,toSend.get(),"https://sp.example.org/","state"); + toSend.release(); + + // Decode message. + string relayState; + const RoleDescriptor* issuer=NULL; + bool trusted=false; + QName idprole(SAMLConstants::SAML20MD_NS, IDPSSODescriptor::LOCAL_NAME); + auto_ptr decoder(SAMLConfig::getConfig().MessageDecoderManager.newPlugin(SAML1_POST_DECODER, NULL)); + auto_ptr response( + dynamic_cast( + decoder->decode(relayState,issuer,trusted,*this,m_metadata,&idprole) + ) + ); + + // Test the results. + TSM_ASSERT_EQUALS("TARGET was not the expected result.", relayState, "state"); + TSM_ASSERT("SAML Response not decoded successfully.", response.get()); + TSM_ASSERT("Message was verified.", issuer && !trusted); + auto_ptr_char entityID(dynamic_cast(issuer->getParent())->getEntityID()); + TSM_ASSERT("Issuer was not expected.", !strcmp(entityID.get(),"https://idp.example.org/")); + TSM_ASSERT_EQUALS("Assertion count was not correct.", response->getAssertions().size(), 1); + + // Trigger a replay. + TSM_ASSERT_THROWS("Did not catch the replay.", + decoder->decode(relayState,issuer,trusted,*this,m_metadata,&idprole,m_trust), + BindingException); + } + catch (XMLToolingException& ex) { + TS_TRACE(ex.what()); + throw; + } + } + + const char* getMethod() const { + return "POST"; + } + + const char* getRequestURL() const { + return "https://sp.example.org/SAML/POST"; + } + + const char* getQueryString() const { + return NULL; + } +}; diff --git a/samltest/samltest.h b/samltest/samltest.h index 92509d8..938f1e8 100644 --- a/samltest/samltest.h +++ b/samltest/samltest.h @@ -15,7 +15,8 @@ */ #include "internal.h" -#include +#include +#include #include #include @@ -31,6 +32,8 @@ public: XMLToolingConfig::getConfig().log_config(); if (!SAMLConfig::getConfig().init()) return false; + SAMLConfig::getConfig().setReplayCache(new ReplayCache()); + if (getenv("SAMLTEST_DATA")) data_path=std::string(getenv("SAMLTEST_DATA")) + "/"; //std::string catpath=data_path + "catalog.xml"; diff --git a/samltest/samltest.vcproj b/samltest/samltest.vcproj index eadeff8..98d301e 100644 --- a/samltest/samltest.vcproj +++ b/samltest/samltest.vcproj @@ -253,6 +253,14 @@ + + + + + + @@ -557,12 +569,16 @@ > + + @@ -907,6 +923,32 @@ + + + + + + + + + + + +