From d1090392d7e609623e44c3b24ca1c90df9199f79 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Tue, 10 May 2016 15:28:19 -0400 Subject: [PATCH] SSPCPP-356 - Better support message-level security on the back channel https://issues.shibboleth.net/jira/browse/SSPCPP-356 --- schemas/shibboleth-2.0-native-sp-config.xsd | 3 +- shibsp/SPConfig.cpp | 47 ++++++++++++++++ shibsp/SPConfig.h | 12 +++++ .../resolver/impl/QueryAttributeResolver.cpp | 35 ++++++++---- .../impl/SimpleAggregationAttributeResolver.cpp | 37 +++++++++---- shibsp/binding/impl/SOAPClient.cpp | 4 +- shibsp/handler/AbstractHandler.h | 27 ++++++++++ shibsp/handler/impl/AbstractHandler.cpp | 20 +++++-- shibsp/handler/impl/SAML2ArtifactResolution.cpp | 2 +- shibsp/handler/impl/SAML2Logout.cpp | 2 +- shibsp/handler/impl/SAML2LogoutInitiator.cpp | 62 +++++++++++++++------- shibsp/handler/impl/SAML2NameIDMgmt.cpp | 2 +- shibsp/handler/impl/SAML2SessionInitiator.cpp | 9 +++- 13 files changed, 209 insertions(+), 53 deletions(-) diff --git a/schemas/shibboleth-2.0-native-sp-config.xsd b/schemas/shibboleth-2.0-native-sp-config.xsd index efc1d94..582ed12 100644 --- a/schemas/shibboleth-2.0-native-sp-config.xsd +++ b/schemas/shibboleth-2.0-native-sp-config.xsd @@ -9,7 +9,7 @@ elementFormDefault="qualified" attributeFormDefault="unqualified" blockDefault="substitution" - version="2.5.3"> + version="2.6"> @@ -43,6 +43,7 @@ + diff --git a/shibsp/SPConfig.cpp b/shibsp/SPConfig.cpp index e8b6d2f..3184176 100644 --- a/shibsp/SPConfig.cpp +++ b/shibsp/SPConfig.cpp @@ -498,3 +498,50 @@ void SPInternalConfig::term() SPConfig::term(); } + +#ifndef SHIBSP_LITE +bool SPConfig::shouldSignOrEncrypt(const char* setting, const char* endpoint, bool isUserAgentPresent) +{ + if (setting && (!strcmp(setting, "true") || !strcmp(setting, isUserAgentPresent ? "front" : "back"))) { + return true; + } + else if (!setting || !strcmp(setting, "conditional")) { + if (isUserAgentPresent || !endpoint) { + return true; + } + + // Conditional on the back channel means to sign if TLS isn't used or if on port 443. + // This compensates for the fact that using the default TLS port likely implies no use + // of client TLS by the server, allowing us to migrate off of the back channel with no + // configuration changes. +#ifdef HAVE_STRCASECMP + if (strncasecmp(endpoint, "http://", 7) == 0) { +#else + if (strnicmp(endpoint, "http://", 7) == 0) { +#endif + return true; + } +#ifdef HAVE_STRCASECMP + else if (strncasecmp(endpoint, "https://", 8) == 0) { +#else + else if (strnicmp(endpoint, "https://", 8) == 0) { +#endif + const char* colon = strchr(endpoint + 8, ':'); + if (colon) { +#ifdef HAVE_STRCASECMP + if (strncasecmp(colon, ":443/", 5) == 0) { +#else + if (strnicmp(colon, ":443/", 5) == 0) { +#endif + return true; + } + } + else { + return true; + } + } + } + + return false; +} +#endif diff --git a/shibsp/SPConfig.h b/shibsp/SPConfig.h index 2c80561..ed021f5 100644 --- a/shibsp/SPConfig.h +++ b/shibsp/SPConfig.h @@ -299,6 +299,18 @@ namespace shibsp { */ xmltooling::PluginManager< Handler,std::string,std::pair > SingleLogoutServiceManager; +#ifndef SHIBSP_LITE + /** + * Determine whether messages should be digitally signed or encrypted based on the setting and endpoint. + * + * @param setting the applicable "signing" or "encryption" property in effect + * @param isUserAgentPresent true iff the user agent is mediating the exchange + * @param URL of endpoint to receive message + * @return whether requests should be digitally signed or encrypted + */ + static bool shouldSignOrEncrypt(const char* setting, const char* endpoint, bool isUserAgentPresent); +#endif + protected: /** Global ServiceProvider instance. */ ServiceProvider* m_serviceProvider; diff --git a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp index 05f63aa..1980f45 100644 --- a/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp +++ b/shibsp/attribute/resolver/impl/QueryAttributeResolver.cpp @@ -484,17 +484,30 @@ void QueryResolver::SAML2Query(QueryContext& ctx) const auto_ptr subject(saml2::SubjectBuilder::buildSubject()); // Encrypt the NameID? - if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) { - auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); - encrypted->encrypt( - *ctx.getNameID(), - *(application.getMetadataProvider()), - mcc, - false, - relyingParty->getXMLString("encryptionAlg").second - ); - subject->setEncryptedID(encrypted.get()); - encrypted.release(); + if (SPConfig::shouldSignOrEncrypt(encryption.first ? encryption.second : "conditional", loc.get(), false)) { + try { + auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); + encrypted->encrypt( + *ctx.getNameID(), + *(application.getMetadataProvider()), + mcc, + false, + relyingParty->getXMLString("encryptionAlg").second + ); + subject->setEncryptedID(encrypted.get()); + encrypted.release(); + } + catch (std::exception& ex) { + // If we're encrypting deliberately, failure should be fatal. + if (encryption.first && strcmp(encryption.second, "conditional")) { + throw; + } + // If opportunistically, just log and move on. + m_log.info("Conditional encryption of NameID in AttributeQuery failed: %s", ex.what()); + auto_ptr namewrapper(ctx.getNameID()->cloneNameID()); + subject->setNameID(namewrapper.get()); + namewrapper.release(); + } } else { auto_ptr namewrapper(ctx.getNameID()->cloneNameID()); diff --git a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp index dfe739f..e4068d7 100644 --- a/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp +++ b/shibsp/attribute/resolver/impl/SimpleAggregationAttributeResolver.cpp @@ -390,20 +390,35 @@ void SimpleAggregationResolver::doQuery(SimpleAggregationContext& ctx, const cha auto_ptr subject(saml2::SubjectBuilder::buildSubject()); // Encrypt the NameID? - if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) { - auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); - encrypted->encrypt( - *name, - *(policy->getMetadataProvider()), - mcc, - false, - relyingParty->getXMLString("encryptionAlg").second + if (SPConfig::shouldSignOrEncrypt(encryption.first ? encryption.second : "conditional", loc.get(), false)) { + try { + auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); + encrypted->encrypt( + *name, + *(policy->getMetadataProvider()), + mcc, + false, + relyingParty->getXMLString("encryptionAlg").second ); - subject->setEncryptedID(encrypted.get()); - encrypted.release(); + subject->setEncryptedID(encrypted.get()); + encrypted.release(); + } + catch (std::exception& ex) { + // If we're encrypting deliberately, failure should be fatal. + if (encryption.first && strcmp(encryption.second, "conditional")) { + throw; + } + // If opportunistically, just log and move on. + m_log.info("Conditional encryption of NameID in AttributeQuery failed: %s", ex.what()); + auto_ptr namewrapper(name->cloneNameID()); + subject->setNameID(namewrapper.get()); + namewrapper.release(); + } } else { - subject->setNameID(name->cloneNameID()); + auto_ptr namewrapper(name->cloneNameID()); + subject->setNameID(namewrapper.get()); + namewrapper.release(); } saml2p::AttributeQuery* query = saml2p::AttributeQueryBuilder::buildAttributeQuery(); diff --git a/shibsp/binding/impl/SOAPClient.cpp b/shibsp/binding/impl/SOAPClient.cpp index 8ecb29b..cd01c8f 100644 --- a/shibsp/binding/impl/SOAPClient.cpp +++ b/shibsp/binding/impl/SOAPClient.cpp @@ -61,8 +61,8 @@ void SOAPClient::send(const soap11::Envelope& env, const char* from, MetadataCre { // Check for message signing requirements. m_relyingParty = m_app.getRelyingParty(dynamic_cast(to.getRole().getParent())); - pair flag = m_relyingParty->getString("signing"); - if (flag.first && (!strcmp(flag.second, "true") || !strcmp(flag.second, "back"))) { + pair flag = m_relyingParty->getString("signing"); + if (SPConfig::shouldSignOrEncrypt(flag.first ? flag.second : "conditional", endpoint, false)) { m_credResolver=m_app.getCredentialResolver(); if (m_credResolver) { m_credResolver->lock(); diff --git a/shibsp/handler/AbstractHandler.h b/shibsp/handler/AbstractHandler.h index d53599a..7c9726a 100644 --- a/shibsp/handler/AbstractHandler.h +++ b/shibsp/handler/AbstractHandler.h @@ -111,6 +111,33 @@ namespace shibsp { ) const; /** + * Encodes and sends SAML 2.0 message, optionally signing it in the process. + * If the method returns, the message MUST NOT be freed by the caller. + * + * @param encoder the MessageEncoder to use + * @param msg the message to send + * @param relayState any RelayState to include with the message + * @param destination location to send message, if not a backchannel response + * @param role recipient of message, if known + * @param application the Application sending the message + * @param httpResponse channel for sending message + * @param defaultSigningProperty the effective value of the "signing" property if unset + * @return the result of sending the message using the encoder + */ + long sendMessage( + const opensaml::MessageEncoder& encoder, + xmltooling::XMLObject* msg, + const char* relayState, + const char* destination, + const opensaml::saml2md::RoleDescriptor* role, + const Application& application, + xmltooling::HTTPResponse& httpResponse, + const char* defaultSigningProperty + ) const; + + /** + * @deprecated + * * Encodes and sends SAML 2.0 message, optionally signing it in the process. * If the method returns, the message MUST NOT be freed by the caller. * diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp index d7adee0..9b1c410 100644 --- a/shibsp/handler/impl/AbstractHandler.cpp +++ b/shibsp/handler/impl/AbstractHandler.cpp @@ -499,14 +499,26 @@ long AbstractHandler::sendMessage( const Application& application, HTTPResponse& httpResponse, bool signIfPossible +) const +{ + return sendMessage(encoder, msg, relayState, destination, role, application, httpResponse, signIfPossible ? "true" : "conditional"); +} + +long AbstractHandler::sendMessage( + const MessageEncoder& encoder, + XMLObject* msg, + const char* relayState, + const char* destination, + const saml2md::RoleDescriptor* role, + const Application& application, + HTTPResponse& httpResponse, + const char* defaultSigningProperty ) const { const EntityDescriptor* entity = role ? dynamic_cast(role->getParent()) : nullptr; const PropertySet* relyingParty = application.getRelyingParty(entity); - pair flag = signIfPossible ? make_pair(true,(const char*)"true") : relyingParty->getString("signing"); - if (flag.first && (!strcmp(flag.second, "true") || - (encoder.isUserAgentPresent() && !strcmp(flag.second, "front")) || - (!encoder.isUserAgentPresent() && !strcmp(flag.second, "back")))) { + pair flag = relyingParty->getString("signing"); + if (SPConfig::shouldSignOrEncrypt(flag.first ? flag.second : defaultSigningProperty, destination, encoder.isUserAgentPresent())) { CredentialResolver* credResolver = application.getCredentialResolver(); if (credResolver) { Locker credLocker(credResolver); diff --git a/shibsp/handler/impl/SAML2ArtifactResolution.cpp b/shibsp/handler/impl/SAML2ArtifactResolution.cpp index ae1150c..8001949 100644 --- a/shibsp/handler/impl/SAML2ArtifactResolution.cpp +++ b/shibsp/handler/impl/SAML2ArtifactResolution.cpp @@ -328,7 +328,7 @@ pair SAML2ArtifactResolution::processMessage(const Application& appli payload.release(); long ret = sendMessage( - *m_encoder, resp.get(), relayState.c_str(), nullptr, policy->getIssuerMetadata(), application, httpResponse, "signResponses" + *m_encoder, resp.get(), relayState.c_str(), nullptr, policy->getIssuerMetadata(), application, httpResponse, "conditional" ); resp.release(); // freed by encoder return make_pair(true, ret); diff --git a/shibsp/handler/impl/SAML2Logout.cpp b/shibsp/handler/impl/SAML2Logout.cpp index c3318b5..190add6 100644 --- a/shibsp/handler/impl/SAML2Logout.cpp +++ b/shibsp/handler/impl/SAML2Logout.cpp @@ -688,7 +688,7 @@ pair SAML2Logout::sendResponse( } auto_ptr_char dest(logout->getDestination()); - long ret = sendMessage(*encoder, logout.get(), relayState, dest.get(), role, application, httpResponse, front); + long ret = sendMessage(*encoder, logout.get(), relayState, dest.get(), role, application, httpResponse, "conditional"); logout.release(); // freed by encoder return make_pair(true, ret); } diff --git a/shibsp/handler/impl/SAML2LogoutInitiator.cpp b/shibsp/handler/impl/SAML2LogoutInitiator.cpp index a9cc01e..37c150b 100644 --- a/shibsp/handler/impl/SAML2LogoutInitiator.cpp +++ b/shibsp/handler/impl/SAML2LogoutInitiator.cpp @@ -90,7 +90,11 @@ namespace shibsp { auto_ptr_char m_protocol; #ifndef SHIBSP_LITE auto_ptr buildRequest( - const Application& application, const Session& session, const RoleDescriptor& role, const MessageEncoder* encoder=nullptr + const Application& application, + const Session& session, + const RoleDescriptor& role, + const XMLCh* endpoint, + const MessageEncoder* encoder=nullptr ) const; LogoutEvent* newLogoutEvent( @@ -358,7 +362,7 @@ pair SAML2LogoutInitiator::doRequest( try { if (!XMLString::equals(epit->getBinding(), binding.get())) continue; - auto_ptr msg(buildRequest(application, *session, *role)); + auto_ptr msg(buildRequest(application, *session, *role, epit->getLocation())); // Log the request. if (logout_event) { @@ -368,8 +372,8 @@ pair SAML2LogoutInitiator::doRequest( logout_event->m_saml2Request = nullptr; } - auto_ptr_char dest(epit->getLocation()); SAML2SOAPClient client(soaper, false); + auto_ptr_char dest(epit->getLocation()); client.sendSAML(msg.release(), application.getId(), mcc, dest.get()); srt.reset(client.receiveSAML()); if (!(logoutResponse = dynamic_cast(srt.get()))) { @@ -456,7 +460,7 @@ pair SAML2LogoutInitiator::doRequest( preserveRelayState(application, httpResponse, relayState); } - auto_ptr msg(buildRequest(application, *session, *role, encoder)); + auto_ptr msg(buildRequest(application, *session, *role, ep->getLocation(), encoder)); msg->setDestination(ep->getLocation()); // Log the request. @@ -467,7 +471,7 @@ pair SAML2LogoutInitiator::doRequest( } auto_ptr_char dest(ep->getLocation()); - ret.second = sendMessage(*encoder, msg.get(), relayState.c_str(), dest.get(), role, application, httpResponse, true); + ret.second = sendMessage(*encoder, msg.get(), relayState.c_str(), dest.get(), role, application, httpResponse, "true"); ret.first = true; msg.release(); // freed by encoder @@ -494,8 +498,11 @@ pair SAML2LogoutInitiator::doRequest( #ifndef SHIBSP_LITE auto_ptr SAML2LogoutInitiator::buildRequest( - const Application& application, const Session& session, const RoleDescriptor& role, const MessageEncoder* encoder - ) const + const Application& application, + const Session& session, + const RoleDescriptor& role, + const XMLCh* endpoint, + const MessageEncoder* encoder) const { const PropertySet* relyingParty = application.getRelyingParty(dynamic_cast(role.getParent())); @@ -512,22 +519,37 @@ auto_ptr SAML2LogoutInitiator::buildRequest( const NameID* nameid = session.getNameID(); pair flag = relyingParty->getString("encryption"); - if (flag.first && - (!strcmp(flag.second, "true") || (encoder && !strcmp(flag.second, "front")) || (!encoder && !strcmp(flag.second, "back")))) { - auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); - MetadataCredentialCriteria mcc(role); - encrypted->encrypt( - *nameid, - *(application.getMetadataProvider()), - mcc, - encoder ? encoder->isCompact() : false, - relyingParty->getXMLString("encryptionAlg").second + auto_ptr_char dest(endpoint); + if (SPConfig::shouldSignOrEncrypt(flag.first ? flag.second : "conditional", dest.get(), encoder != nullptr)) { + try { + auto_ptr encrypted(EncryptedIDBuilder::buildEncryptedID()); + MetadataCredentialCriteria mcc(role); + encrypted->encrypt( + *nameid, + *(application.getMetadataProvider()), + mcc, + encoder ? encoder->isCompact() : false, + relyingParty->getXMLString("encryptionAlg").second ); - msg->setEncryptedID(encrypted.get()); - encrypted.release(); + msg->setEncryptedID(encrypted.get()); + encrypted.release(); + } + catch (std::exception& ex) { + // If we're encrypting deliberately, failure should be fatal. + if (flag.first && strcmp(flag.second, "conditional")) { + throw; + } + // If opportunistically, just log and move on. + m_log.info("Conditional encryption of NameID in LogoutRequest failed: %s", ex.what()); + auto_ptr namewrapper(nameid->cloneNameID()); + msg->setNameID(namewrapper.get()); + namewrapper.release(); + } } else { - msg->setNameID(nameid->cloneNameID()); + auto_ptr namewrapper(nameid->cloneNameID()); + msg->setNameID(namewrapper.get()); + namewrapper.release(); } XMLCh* msgid = SAMLConfig::getConfig().generateIdentifier(); diff --git a/shibsp/handler/impl/SAML2NameIDMgmt.cpp b/shibsp/handler/impl/SAML2NameIDMgmt.cpp index 34bc87d..933e493 100644 --- a/shibsp/handler/impl/SAML2NameIDMgmt.cpp +++ b/shibsp/handler/impl/SAML2NameIDMgmt.cpp @@ -512,7 +512,7 @@ pair SAML2NameIDMgmt::sendResponse( auto_ptr_char dest(nim->getDestination()); - long ret = sendMessage(*encoder, nim.get(), relayState, dest.get(), role, application, httpResponse); + long ret = sendMessage(*encoder, nim.get(), relayState, dest.get(), role, application, httpResponse, "conditional"); nim.release(); // freed by encoder return make_pair(true, ret); } diff --git a/shibsp/handler/impl/SAML2SessionInitiator.cpp b/shibsp/handler/impl/SAML2SessionInitiator.cpp index 794bf2f..cb2c19a 100644 --- a/shibsp/handler/impl/SAML2SessionInitiator.cpp +++ b/shibsp/handler/impl/SAML2SessionInitiator.cpp @@ -756,7 +756,14 @@ pair SAML2SessionInitiator::doRequest( } long ret = sendMessage( - *encoder, req.get(), relayState.c_str(), dest.get(), role, app, httpResponse, role ? role->WantAuthnRequestsSigned() : false + *encoder, + req.get(), + relayState.c_str(), + dest.get(), + role, + app, + httpResponse, + (role && role->WantAuthnRequestsSigned()) ? "true" : "false" ); req.release(); // freed by encoder return make_pair(true, ret); -- 2.1.4