From eba163ed4f12b2522552ce06b7e21c66660734c8 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Tue, 15 Feb 2011 16:24:02 +0000 Subject: [PATCH] https://issues.shibboleth.net/jira/browse/SSPCPP-351 --- schemas/shibboleth-2.0-native-sp-config.xsd | 13 ++++- shibsp/handler/impl/AbstractHandler.cpp | 60 +++++++++++++++++++++++- shibsp/handler/impl/AssertionConsumerService.cpp | 20 ++++---- shibsp/handler/impl/LocalLogoutInitiator.cpp | 6 ++- shibsp/handler/impl/SAML2Logout.cpp | 6 ++- shibsp/handler/impl/SAML2LogoutInitiator.cpp | 4 +- shibsp/handler/impl/SAML2SessionInitiator.cpp | 3 +- shibsp/handler/impl/Shib1SessionInitiator.cpp | 3 +- shibsp/handler/impl/WAYFSessionInitiator.cpp | 3 +- shibsp/internal.h | 13 ++++- 10 files changed, 112 insertions(+), 19 deletions(-) diff --git a/schemas/shibboleth-2.0-native-sp-config.xsd b/schemas/shibboleth-2.0-native-sp-config.xsd index a36efe3..6d775b1 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.4"> + version="2.4.2"> @@ -46,6 +46,15 @@ + + + + + + + + + @@ -512,6 +521,8 @@ + + diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp index 33d722a..967351b 100644 --- a/shibsp/handler/impl/AbstractHandler.cpp +++ b/shibsp/handler/impl/AbstractHandler.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -112,7 +112,65 @@ namespace shibsp { } } + void SHIBSP_DLLLOCAL limitRelayState( + Category& log, const Application& application, const HTTPRequest& httpRequest, const char* relayState + ) { + const PropertySet* sessionProps = application.getPropertySet("Sessions"); + if (sessionProps) { + pair relayStateLimit = sessionProps->getString("relayStateLimit"); + if (relayStateLimit.first) { + vector whitelist; + if (!strcmp(relayStateLimit.second, "exact")) { + // Scheme and hostname have to match. + if (!strcmp(httpRequest.getScheme(), "https") && httpRequest.getPort() == 443) { + whitelist.push_back(string("https://") + httpRequest.getHostname() + '/'); + } + else if (!strcmp(httpRequest.getScheme(), "http") && httpRequest.getPort() == 80) { + whitelist.push_back(string("http://") + httpRequest.getHostname() + '/'); + } + ostringstream portstr; + portstr << httpRequest.getPort(); + whitelist.push_back(string(httpRequest.getScheme()) + "://" + httpRequest.getHostname() + ':' + portstr.str() + '/'); + } + else if (!strcmp(relayStateLimit.second, "host")) { + // Allow any scheme or port. + whitelist.push_back(string("https://") + httpRequest.getHostname() + '/'); + whitelist.push_back(string("http://") + httpRequest.getHostname() + '/'); + whitelist.push_back(string("https://") + httpRequest.getHostname() + ':'); + whitelist.push_back(string("http://") + httpRequest.getHostname() + ':'); + } + else if (!strcmp(relayStateLimit.second, "whitelist")) { + // Literal set of comparisons to use. + pair whitelistval = sessionProps->getString("relayStateWhitelist"); + if (whitelistval.first) { +#ifdef HAVE_STRTOK_R + char* pos=nullptr; + const char* token = strtok_r(const_cast(whitelistval.second), " ", &pos); +#else + const char* token = strtok(const_cast(whitelistval.second), " "); +#endif + while (token) { + whitelist.push_back(token); +#ifdef HAVE_STRTOK_R + token = strtok_r(nullptr, " ", &pos); +#else + token = strtok(nullptr, " "); +#endif + } + } + } + for (vector::const_iterator w = whitelist.begin(); w != whitelist.end(); ++w) { + if (XMLString::startsWithI(relayState, w->c_str())) { + return; + } + } + + log.warn("relayStateLimit policy (%s), blocked redirect to (%s)", relayStateLimit.second, relayState); + throw opensaml::SecurityPolicyException("Blocked unacceptable redirect location."); + } + } + } }; void SHIBSP_API shibsp::registerHandlers() diff --git a/shibsp/handler/impl/AssertionConsumerService.cpp b/shibsp/handler/impl/AssertionConsumerService.cpp index e9176c8..4b14643 100644 --- a/shibsp/handler/impl/AssertionConsumerService.cpp +++ b/shibsp/handler/impl/AssertionConsumerService.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -153,6 +153,7 @@ pair AssertionConsumerService::processMessage( ); string relayState; + bool relayStateOK = true; try { // Decode the message and process it in a protocol-specific way. auto_ptr msg(m_decoder->decode(relayState, httpRequest, *(policy.get()))); @@ -161,6 +162,7 @@ pair AssertionConsumerService::processMessage( DDF postData = recoverPostData(application, httpRequest, httpResponse, relayState.c_str()); DDFJanitor postjan(postData); recoverRelayState(application, httpRequest, httpResponse, relayState); + limitRelayState(m_log, application, httpRequest, relayState.c_str()); implementProtocol(application, httpRequest, httpResponse, *(policy.get()), NULL, *msg.get()); auto_ptr_char issuer(policy->getIssuer() ? policy->getIssuer()->getName() : nullptr); @@ -181,13 +183,15 @@ pair AssertionConsumerService::processMessage( } } catch (XMLToolingException& ex) { - // Check for isPassive error condition. - const char* sc2 = ex.getProperty("statusCode2"); - if (sc2 && !strcmp(sc2, "urn:oasis:names:tc:SAML:2.0:status:NoPassive")) { - pair ignore = getBool("ignoreNoPassive", m_configNS.get()); // namespace-qualified if inside handler element - if (ignore.first && ignore.second && !relayState.empty()) { - m_log.debug("ignoring SAML status of NoPassive and redirecting to resource..."); - return make_pair(true, httpResponse.sendRedirect(relayState.c_str())); + if (relayStateOK) { + // Check for isPassive error condition. + const char* sc2 = ex.getProperty("statusCode2"); + if (sc2 && !strcmp(sc2, "urn:oasis:names:tc:SAML:2.0:status:NoPassive")) { + pair ignore = getBool("ignoreNoPassive", m_configNS.get()); // namespace-qualified if inside handler element + if (ignore.first && ignore.second && !relayState.empty()) { + m_log.debug("ignoring SAML status of NoPassive and redirecting to resource..."); + return make_pair(true, httpResponse.sendRedirect(relayState.c_str())); + } } } if (!relayState.empty()) diff --git a/shibsp/handler/impl/LocalLogoutInitiator.cpp b/shibsp/handler/impl/LocalLogoutInitiator.cpp index 5468df9..8a74dc3 100644 --- a/shibsp/handler/impl/LocalLogoutInitiator.cpp +++ b/shibsp/handler/impl/LocalLogoutInitiator.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2009 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -105,7 +105,9 @@ pair LocalLogoutInitiator::run(SPRequest& request, bool isHandler) co // Route back to return location specified, or use the local template. const char* dest = request.getParameter("return"); - if (dest) + if (dest) { + limitRelayState(m_log, app, request, dest); return make_pair(true, request.sendRedirect(dest)); + } return sendLogoutPage(app, request, request, "local"); } diff --git a/shibsp/handler/impl/SAML2Logout.cpp b/shibsp/handler/impl/SAML2Logout.cpp index acfdd26..229496d 100644 --- a/shibsp/handler/impl/SAML2Logout.cpp +++ b/shibsp/handler/impl/SAML2Logout.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -548,8 +548,10 @@ pair SAML2Logout::doRequest(const Application& application, const HTT if (sc && XMLString::equals(sc->getValue(), StatusCode::PARTIAL_LOGOUT)) return sendLogoutPage(application, request, response, "partial"); - if (!relayState.empty()) + if (!relayState.empty()) { + limitRelayState(m_log, application, request, relayState.c_str()); return make_pair(true, response.sendRedirect(relayState.c_str())); + } // Return template for completion of logout. return sendLogoutPage(application, request, response, "global"); diff --git a/shibsp/handler/impl/SAML2LogoutInitiator.cpp b/shibsp/handler/impl/SAML2LogoutInitiator.cpp index cba23f6..5d0f14a 100644 --- a/shibsp/handler/impl/SAML2LogoutInitiator.cpp +++ b/shibsp/handler/impl/SAML2LogoutInitiator.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -375,6 +375,7 @@ pair SAML2LogoutInitiator::doRequest( else { const char* returnloc = httpRequest.getParameter("return"); if (returnloc) { + limitRelayState(m_log, application, httpRequest, returnloc); ret.second = httpResponse.sendRedirect(returnloc); ret.first = true; } @@ -395,6 +396,7 @@ pair SAML2LogoutInitiator::doRequest( string relayState; const char* returnloc = httpRequest.getParameter("return"); if (returnloc) { + limitRelayState(m_log, application, httpRequest, returnloc); relayState = returnloc; preserveRelayState(application, httpResponse, relayState); } diff --git a/shibsp/handler/impl/SAML2SessionInitiator.cpp b/shibsp/handler/impl/SAML2SessionInitiator.cpp index f44dca2..96e3fca 100644 --- a/shibsp/handler/impl/SAML2SessionInitiator.cpp +++ b/shibsp/handler/impl/SAML2SessionInitiator.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -266,6 +266,7 @@ pair SAML2SessionInitiator::run(SPRequest& request, string& entityID, // Always need to recover target URL to compute handler below. recoverRelayState(app, request, request, target, false); + limitRelayState(m_log, app, request, target.c_str()); pair flag = getBool("isPassive", request); isPassive = (flag.first && flag.second); diff --git a/shibsp/handler/impl/Shib1SessionInitiator.cpp b/shibsp/handler/impl/Shib1SessionInitiator.cpp index d3000ca..5ed0c11 100644 --- a/shibsp/handler/impl/Shib1SessionInitiator.cpp +++ b/shibsp/handler/impl/Shib1SessionInitiator.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -139,6 +139,7 @@ pair Shib1SessionInitiator::run(SPRequest& request, string& entityID, // Since we're passing the ACS by value, we need to compute the return URL, // so we'll need the target resource for real. recoverRelayState(app, request, request, target, false); + limitRelayState(m_log, app, request, target.c_str()); } else { // Check for a hardwired target value in the map or handler. diff --git a/shibsp/handler/impl/WAYFSessionInitiator.cpp b/shibsp/handler/impl/WAYFSessionInitiator.cpp index 8d6bd90..36379cb 100644 --- a/shibsp/handler/impl/WAYFSessionInitiator.cpp +++ b/shibsp/handler/impl/WAYFSessionInitiator.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2001-2010 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -110,6 +110,7 @@ pair WAYFSessionInitiator::run(SPRequest& request, string& entityID, // Since we're passing the ACS by value, we need to compute the return URL, // so we'll need the target resource for real. recoverRelayState(request.getApplication(), request, request, target, false); + limitRelayState(m_log, request.getApplication(), request, target.c_str()); prop.second = request.getParameter("discoveryURL"); if (prop.second && *prop.second) diff --git a/shibsp/internal.h b/shibsp/internal.h index 6ccfbc3..bd49761 100644 --- a/shibsp/internal.h +++ b/shibsp/internal.h @@ -1,5 +1,5 @@ /* - * Copyright 2001-2007 Internet2 + * Copyright 2001-2011 Internet2 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -45,8 +45,19 @@ #include #include +#include +#include using namespace xmltooling::logging; using namespace xercesc; +namespace shibsp { + void SHIBSP_DLLLOCAL limitRelayState( + xmltooling::logging::Category& log, + const Application& application, + const xmltooling::HTTPRequest& httpRequest, + const char* relayState + ); +}; + #endif /* __shibsp_internal_h__ */ -- 2.1.4