From a6c3fcf221f06bc3430b06a57e47df0fa4391121 Mon Sep 17 00:00:00 2001 From: scantor Date: Thu, 3 May 2012 16:32:16 +0000 Subject: [PATCH] https://issues.shibboleth.net/jira/browse/SSPCPP-444 git-svn-id: https://svn.shibboleth.net/cpp-sp/branches/REL_2@3638 cb58f699-b61c-0410-a6fe-9272a202ed29 --- shibsp/handler/Handler.h | 14 ++++++ shibsp/handler/impl/AbstractHandler.cpp | 65 ++++++++++++++++++++++++---- shibsp/handler/impl/SAML2LogoutInitiator.cpp | 1 + shibsp/handler/impl/SessionInitiator.cpp | 2 + 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/shibsp/handler/Handler.h b/shibsp/handler/Handler.h index 3a2e623..3a9a325 100644 --- a/shibsp/handler/Handler.h +++ b/shibsp/handler/Handler.h @@ -63,6 +63,20 @@ namespace shibsp { virtual void log(SPRequest::SPLogLevel level, const std::string& msg) const; /** + * Prevents unused relay state from building up by cleaning old state from the client. + * + *

Handlers that generate relay state should call this method as a house cleaning + * step. + * + * @param application the associated Application + * @param request incoming HTTP request + * @param response outgoing HTTP response + */ + virtual void cleanRelayState( + const Application& application, const xmltooling::HTTPRequest& request, xmltooling::HTTPResponse& response + ) const; + + /** * Implements various mechanisms to preserve RelayState, * such as cookies or StorageService-backed keys. * diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp index 057e7d3..179c39f 100644 --- a/shibsp/handler/impl/AbstractHandler.cpp +++ b/shibsp/handler/impl/AbstractHandler.cpp @@ -177,6 +177,54 @@ void Handler::log(SPRequest::SPLogLevel level, const string& msg) const ); } +void Handler::cleanRelayState( + const Application& application, const xmltooling::HTTPRequest& request, xmltooling::HTTPResponse& response + ) const +{ + // Only cookie-based relay state requires cleaning. + pair mech = getString("relayState"); + if (!mech.first) { + // Check for setting on Sessions element. + const PropertySet* sessionprop = application.getPropertySet("Sessions"); + if (sessionprop) + mech = sessionprop->getString("relayState"); + } + if (!mech.first || !mech.second || strncmp(mech.second, "cookie", 6)) + return; + + int maxCookies = 25,purgedCookies = 0; + mech.second += 6; + if (*mech.second == ':' && isdigit(*(++mech.second))) { + maxCookies = atoi(mech.second); + if (maxCookies == 0) + maxCookies = 25; + } + + string exp; + + // Walk the list of cookies backwards by name. + const map& cookies = request.getCookies(); + for (map::const_reverse_iterator i = cookies.rbegin(); i != cookies.rend(); ++i) { + // Process relay state cookies only. + if (starts_with(i->first, "_shibstate_")) { + if (maxCookies > 0) { + // Keep it, but count it against the limit. + --maxCookies; + } + else { + // We're over the limit, so everything here and older gets cleaned up. + if (exp.empty()) + exp = string(application.getCookieNameProps("_shibstate_").second) + "; expires=Mon, 01 Jan 2001 00:00:00 GMT"; + response.setCookie(i->first.c_str(), exp.c_str()); + ++purgedCookies; + } + } + } + + if (purgedCookies > 0) + log(SPRequest::SPDebug, string("purged ") + lexical_cast(purgedCookies) + " stale relay state cookie(s) from client"); +} + void Handler::preserveRelayState(const Application& application, HTTPResponse& response, string& relayState) const { // The empty string implies no state to deal with. @@ -194,22 +242,22 @@ void Handler::preserveRelayState(const Application& application, HTTPResponse& r if (!mech.first || !mech.second || !*mech.second) return; - if (!strcmp(mech.second, "cookie")) { + if (!strncmp(mech.second, "cookie", 6)) { // Here we store the state in a cookie and send a fixed // value so we can recognize it on the way back. if (relayState.find("cookie:") != 0) { - const URLEncoder* urlenc = XMLToolingConfig::getConfig().getURLEncoder(); - pair shib_cookie=application.getCookieNameProps("_shibstate_"); - string stateval = urlenc->encode(relayState.c_str()) + shib_cookie.second; + pair shib_cookie = application.getCookieNameProps("_shibstate_"); + string stateval = XMLToolingConfig::getConfig().getURLEncoder()->encode(relayState.c_str()) + shib_cookie.second; // Generate a random key for the cookie name instead of the fixed name. string rsKey; - generateRandomHex(rsKey, 5); + generateRandomHex(rsKey, 4); + rsKey = lexical_cast(time(nullptr)) + '_' + rsKey; shib_cookie.first = "_shibstate_" + rsKey; response.setCookie(shib_cookie.first.c_str(), stateval.c_str()); relayState = "cookie:" + rsKey; } } - else if (strstr(mech.second,"ss:")==mech.second) { + else if (!strncmp(mech.second, "ss:", 3)) { if (relayState.find("ss:") != 0) { mech.second+=3; if (*mech.second) { @@ -323,7 +371,7 @@ void Handler::recoverRelayState( } } - // Look for cookie-backed state of the form "cookie:key". + // Look for cookie-backed state of the form "cookie:timestamp_key". if (strstr(state,"cookie:")==state) { state += 7; if (*state) { @@ -333,11 +381,10 @@ void Handler::recoverRelayState( state = request.getCookie(relay_cookie.first.c_str()); if (state && *state) { // URL-decode the value. - char* rscopy=strdup(state); + char* rscopy = strdup(state); XMLToolingConfig::getConfig().getURLEncoder()->decode(rscopy); relayState = rscopy; free(rscopy); - if (clear) { string exp(relay_cookie.second); exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT"; diff --git a/shibsp/handler/impl/SAML2LogoutInitiator.cpp b/shibsp/handler/impl/SAML2LogoutInitiator.cpp index d6ffbc3..7ae8f39 100644 --- a/shibsp/handler/impl/SAML2LogoutInitiator.cpp +++ b/shibsp/handler/impl/SAML2LogoutInitiator.cpp @@ -442,6 +442,7 @@ pair SAML2LogoutInitiator::doRequest( application.limitRedirect(httpRequest, returnloc); relayState = returnloc; httpRequest.absolutize(relayState); + cleanRelayState(application, httpRequest, httpResponse); preserveRelayState(application, httpResponse, relayState); } diff --git a/shibsp/handler/impl/SessionInitiator.cpp b/shibsp/handler/impl/SessionInitiator.cpp index ac145a2..b45551d 100644 --- a/shibsp/handler/impl/SessionInitiator.cpp +++ b/shibsp/handler/impl/SessionInitiator.cpp @@ -143,6 +143,8 @@ bool SessionInitiator::checkCompatibility(SPRequest& request, bool isHandler) co pair SessionInitiator::run(SPRequest& request, bool isHandler) const { + cleanRelayState(request.getApplication(), request, request); + const char* entityID = nullptr; pair param = getString("entityIDParam"); if (isHandler) { -- 2.1.4