[shibboleth-sp2] 18/89: SSPCPP-356 - Better support message-level security on the back channel

Ferenc Wágner wferi at moszumanska.debian.org
Thu Sep 1 09:24:04 UTC 2016


This is an automated email from the git hooks/post-receive script.

wferi pushed a commit to branch master
in repository shibboleth-sp2.

commit d1090392d7e609623e44c3b24ca1c90df9199f79
Author: Scott Cantor <cantor.2 at osu.edu>
Date:   Tue May 10 15:28:19 2016 -0400

    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">
 
   <import namespace="http://www.w3.org/2000/09/xmldsig#" schemaLocation="xmldsig-core-schema.xsd" />
   <import namespace="urn:oasis:names:tc:SAML:2.0:assertion" schemaLocation="saml-schema-assertion-2.0.xsd"/>
@@ -43,6 +43,7 @@
       <enumeration value="false"/>
       <enumeration value="front"/>
       <enumeration value="back"/>
+      <enumeration value="conditional" />
     </restriction>
   </simpleType>
 
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<const xercesc::DOMElement*,const char*> > 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<saml2::Subject> subject(saml2::SubjectBuilder::buildSubject());
 
             // Encrypt the NameID?
-            if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) {
-                auto_ptr<EncryptedID> 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<EncryptedID> 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<NameID> namewrapper(ctx.getNameID()->cloneNameID());
+                    subject->setNameID(namewrapper.get());
+                    namewrapper.release();
+                }
             }
             else {
                 auto_ptr<NameID> 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<saml2::Subject> subject(saml2::SubjectBuilder::buildSubject());
 
             // Encrypt the NameID?
-            if (encryption.first && (!strcmp(encryption.second, "true") || !strcmp(encryption.second, "back"))) {
-                auto_ptr<EncryptedID> 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<EncryptedID> 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<NameID> namewrapper(name->cloneNameID());
+                    subject->setNameID(namewrapper.get());
+                    namewrapper.release();
+                }
             }
             else {
-                subject->setNameID(name->cloneNameID());
+                auto_ptr<NameID> 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<const EntityDescriptor*>(to.getRole().getParent()));
-    pair<bool,const char*> flag = m_relyingParty->getString("signing");
-    if (flag.first && (!strcmp(flag.second, "true") || !strcmp(flag.second, "back"))) {
+    pair<bool, const char*> 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<const EntityDescriptor*>(role->getParent()) : nullptr;
     const PropertySet* relyingParty = application.getRelyingParty(entity);
-    pair<bool,const char*> 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<bool,const char*> 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<bool,long> 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<bool,long> 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<LogoutRequest> 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<bool,long> SAML2LogoutInitiator::doRequest(
                 try {
                     if (!XMLString::equals(epit->getBinding(), binding.get()))
                         continue;
-                    auto_ptr<LogoutRequest> msg(buildRequest(application, *session, *role));
+                    auto_ptr<LogoutRequest> msg(buildRequest(application, *session, *role, epit->getLocation()));
 
                     // Log the request.
                     if (logout_event) {
@@ -368,8 +372,8 @@ pair<bool,long> 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<LogoutResponse*>(srt.get()))) {
@@ -456,7 +460,7 @@ pair<bool,long> SAML2LogoutInitiator::doRequest(
             preserveRelayState(application, httpResponse, relayState);
         }
 
-        auto_ptr<LogoutRequest> msg(buildRequest(application, *session, *role, encoder));
+        auto_ptr<LogoutRequest> msg(buildRequest(application, *session, *role, ep->getLocation(), encoder));
         msg->setDestination(ep->getLocation());
 
         // Log the request.
@@ -467,7 +471,7 @@ pair<bool,long> 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<bool,long> SAML2LogoutInitiator::doRequest(
 #ifndef SHIBSP_LITE
 
 auto_ptr<LogoutRequest> 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<EntityDescriptor*>(role.getParent()));
 
@@ -512,22 +519,37 @@ auto_ptr<LogoutRequest> SAML2LogoutInitiator::buildRequest(
 
     const NameID* nameid = session.getNameID();
     pair<bool,const char*> flag = relyingParty->getString("encryption");
-    if (flag.first &&
-        (!strcmp(flag.second, "true") || (encoder && !strcmp(flag.second, "front")) || (!encoder && !strcmp(flag.second, "back")))) {
-        auto_ptr<EncryptedID> 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<EncryptedID> 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<NameID> namewrapper(nameid->cloneNameID());
+            msg->setNameID(namewrapper.get());
+            namewrapper.release();
+        }
     }
     else {
-        msg->setNameID(nameid->cloneNameID());
+        auto_ptr<NameID> 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<bool,long> 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<bool,long> 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);

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-shibboleth/shibboleth-sp2.git



More information about the Pkg-shibboleth-devel mailing list