[cpp-sp] branch master updated: SSPCPP-878 - SameSite workaround using second cookie

Scott Cantor cantor.2 at osu.edu
Tue Feb 4 12:16:24 EST 2020


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

scantor pushed a commit to branch master
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=4c1cfb9c16a9a35c748035c02ac101cd6bedf19a

The following commit(s) were added to refs/heads/master by this push:
       new  4c1cfb9   SSPCPP-878 - SameSite workaround using second cookie
4c1cfb9 is described below

commit 4c1cfb9c16a9a35c748035c02ac101cd6bedf19a
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Feb 4 12:15:59 2020 -0500

    SSPCPP-878 - SameSite workaround using second cookie
    
    https://issues.shibboleth.net/jira/browse/SSPCPP-878
---
 schemas/shibboleth-3.0-native-sp-config.xsd | 12 +++++-
 shibsp/handler/impl/AbstractHandler.cpp     | 55 +++++++++++++++++-------
 shibsp/impl/StorageServiceSessionCache.cpp  | 66 +++++++++++++++++++++++------
 shibsp/impl/StorageServiceSessionCache.h    | 11 ++++-
 4 files changed, 113 insertions(+), 31 deletions(-)

diff --git a/schemas/shibboleth-3.0-native-sp-config.xsd b/schemas/shibboleth-3.0-native-sp-config.xsd
index e32d5f4..2a8a274 100644
--- a/schemas/shibboleth-3.0-native-sp-config.xsd
+++ b/schemas/shibboleth-3.0-native-sp-config.xsd
@@ -18,7 +18,7 @@
 
   <annotation>
     <documentation>
-      3.0 schema for XML-based configuration of Shibboleth Native SP instances.
+      3.x schema for XML-based configuration of Shibboleth Native SP instances.
       First appearing in Shibboleth 3.0 release.
     </documentation>
   </annotation>
@@ -58,6 +58,14 @@
     </restriction>
   </simpleType>
 
+  <simpleType name="sameSiteType">
+    <restriction base="string">
+      <enumeration value="None"/>
+      <enumeration value="Lax"/>
+      <enumeration value="Strict"/>
+    </restriction>
+  </simpleType>
+
   <complexType name="PluggableType">
     <sequence>
       <any namespace="##any" processContents="lax" minOccurs="0" maxOccurs="unbounded"/>
@@ -577,6 +585,8 @@
     <attribute name="exportACL" type="conf:listOfStrings"/>
     <attribute name="cookieName" type="conf:string"/>
     <attribute name="cookieProps" type="conf:string"/>
+    <attribute name="sameSiteSession" type="conf:sameSiteType"/>
+    <attribute name="sameSiteFallback" type="boolean"/>
     <attribute name="cookieLifetime" type="unsignedInt"/>
     <attribute name="idpHistory" type="boolean"/>
     <attribute name="idpHistoryDays" type="unsignedInt"/>
diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp
index 56c2959..361e38e 100644
--- a/shibsp/handler/impl/AbstractHandler.cpp
+++ b/shibsp/handler/impl/AbstractHandler.cpp
@@ -186,25 +186,30 @@ void Handler::cleanRelayState(
     const Application& application, const xmltooling::HTTPRequest& request, xmltooling::HTTPResponse& response
     ) const
 {
+    const PropertySet* sessionprop = application.getPropertySet("Sessions");
+
     // Only cookie-based relay state requires cleaning.
     pair<bool,const char*> mech = getString("relayState");
     if (!mech.first) {
         // Check for setting on Sessions element.
-        const PropertySet* sessionprop = application.getPropertySet("Sessions");
-        if (sessionprop)
+        if (sessionprop) {
             mech = sessionprop->getString("relayState");
+        }
     }
     if (!mech.first || !mech.second || strncmp(mech.second, "cookie", 6))
         return;
-    
-    int maxCookies = 25,purgedCookies = 0;
+
+    int maxCookies = 20,purgedCookies = 0;
     mech.second += 6;
     if (*mech.second == ':' && isdigit(*(++mech.second))) {
         maxCookies = atoi(mech.second);
         if (maxCookies == 0)
-            maxCookies = 25;
+            maxCookies = 20;
     }
 
+    pair<bool, bool> sameSiteFallback =
+        sessionprop ? sessionprop->getBool("sameSiteFallback") : pair<bool,bool>(false, false);
+
     string exp;
 
     // Walk the list of cookies backwards by name.
@@ -220,7 +225,8 @@ void Handler::cleanRelayState(
                 // 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());
+                response.setCookie(i->first.c_str(), exp.c_str(), HTTPResponse::SAMESITE_NONE,
+                    sameSiteFallback.first && sameSiteFallback.second);
                 ++purgedCookies;
             }
         }
@@ -236,11 +242,12 @@ void Handler::preserveRelayState(const Application& application, HTTPResponse& r
     if (relayState.empty())
         return;
 
+    const PropertySet* sessionprop = application.getPropertySet("Sessions");
+
     // No setting means just pass state by value.
     pair<bool,const char*> mech = getString("relayState");
     if (!mech.first) {
         // Check for setting on Sessions element.
-        const PropertySet* sessionprop = application.getPropertySet("Sessions");
         if (sessionprop)
             mech = sessionprop->getString("relayState");
     }
@@ -251,6 +258,8 @@ void Handler::preserveRelayState(const Application& application, HTTPResponse& r
         // 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 && relayState.find("ss:") != 0) {
+            pair<bool, bool> sameSiteFallback =
+                sessionprop ? sessionprop->getBool("sameSiteFallback") : pair<bool,bool>(false, false);
             pair<string,const char*> 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.
@@ -258,7 +267,8 @@ void Handler::preserveRelayState(const Application& application, HTTPResponse& r
             generateRandomHex(rsKey, 4);
             rsKey = lexical_cast<string>(time(nullptr)) + '_' + rsKey;
             shib_cookie.first = "_shibstate_" + rsKey;
-            response.setCookie(shib_cookie.first.c_str(), stateval.c_str());
+            response.setCookie(shib_cookie.first.c_str(), stateval.c_str(), HTTPResponse::SAMESITE_NONE,
+                sameSiteFallback.first && sameSiteFallback.second);
             relayState = "cookie:" + rsKey;
         }
     }
@@ -382,10 +392,14 @@ void Handler::recoverRelayState(
     if (strstr(state,"cookie:") == state) {
         state += 7;
         if (*state) {
+            const PropertySet* sessionprop = application.getPropertySet("Sessions");
+            pair<bool, bool> sameSiteFallback =
+                sessionprop ? sessionprop->getBool("sameSiteFallback") : pair<bool,bool>(false, false);
+
             // Pull the value from the "relay state" cookie.
             pair<string,const char*> relay_cookie = application.getCookieNameProps("_shibstate_");
             relay_cookie.first = string("_shibstate_") + state;
-            state = request.getCookie(relay_cookie.first.c_str());
+            state = request.getCookie(relay_cookie.first.c_str(), sameSiteFallback.first && sameSiteFallback.second);
             if (state && *state) {
                 // URL-decode the value.
                 char* rscopy = strdup(state);
@@ -395,7 +409,8 @@ void Handler::recoverRelayState(
                 if (clear) {
                     string exp(relay_cookie.second);
                     exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-                    response.setCookie(relay_cookie.first.c_str(), exp.c_str());
+                    response.setCookie(relay_cookie.first.c_str(), exp.c_str(), HTTPResponse::SAMESITE_NONE,
+                        sameSiteFallback.first && sameSiteFallback.second);
                 }
                 request.absolutize(relayState);
                 return;
@@ -594,7 +609,7 @@ void AbstractHandler::preservePostData(
 #endif
 
     // No specs mean no save.
-    const PropertySet* props=application.getPropertySet("Sessions");
+    const PropertySet* props = application.getPropertySet("Sessions");
     pair<bool,const char*> mech = props ? props->getString("postData") : pair<bool,const char*>(false,nullptr);
     if (!mech.first) {
         m_log.info("postData property not supplied, form data will not be preserved across SSO");
@@ -648,8 +663,10 @@ void AbstractHandler::preservePostData(
 
         pair<string,const char*> shib_cookie = getPostCookieNameProps(application, relayState);
 
+        pair<bool,bool> sameSiteFallback = props ? props->getString("sameSiteFallback") : pair<bool,bool>(false, false);
+
         // Purge any cookies in excess of 25.
-        int maxCookies = 25,purgedCookies = 0;
+        int maxCookies = 20,purgedCookies = 0;
         string exp;
 
         // Walk the list of cookies backwards by name.
@@ -665,7 +682,8 @@ void AbstractHandler::preservePostData(
                     // We're over the limit, so everything here and older gets cleaned up.
                     if (exp.empty())
                         exp = string(shib_cookie.second) + "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-                    response.setCookie(i->first.c_str(), exp.c_str());
+                    response.setCookie(i->first.c_str(), exp.c_str(), HTTPResponse::SAMESITE_NONE,
+                        sameSiteFallback.first && sameSiteFallback.second);
                     ++purgedCookies;
                 }
             }
@@ -676,7 +694,8 @@ void AbstractHandler::preservePostData(
 
         // Set a cookie with key info.
         postkey += shib_cookie.second;
-        response.setCookie(shib_cookie.first.c_str(), postkey.c_str());
+        response.setCookie(shib_cookie.first.c_str(), postkey.c_str(), HTTPResponse::SAMESITE_NONE,
+            sameSiteFallback.first && sameSiteFallback.second);
     }
     else {
         postData.destroy();
@@ -688,16 +707,20 @@ DDF AbstractHandler::recoverPostData(
     const Application& application, const HTTPRequest& request, HTTPResponse& response, const char* relayState
     ) const
 {
+    const PropertySet* props = application.getPropertySet("Sessions");
+    pair<bool,bool> sameSiteFallback = props ? props->getString("sameSiteFallback") : pair<bool,bool>(false, false);
+
     // First we need the post recovery cookie.
     pair<string,const char*> shib_cookie = getPostCookieNameProps(application, relayState);
-    const char* cookie = request.getCookie(shib_cookie.first.c_str());
+    const char* cookie = request.getCookie(shib_cookie.first.c_str(), sameSiteFallback.first && sameSiteFallback.second);
     if (!cookie || !*cookie)
         return DDF();
 
     // Clear the cookie.
     string exp(shib_cookie.second);
     exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-    response.setCookie(shib_cookie.first.c_str(), exp.c_str());
+    response.setCookie(shib_cookie.first.c_str(), exp.c_str(), HTTPResponse::SAMESITE_NONE,
+        sameSiteFallback.first && sameSiteFallback.second);
 
     // Look for StorageService-backed state of the form "ss:SSID:key".
     const char* state = cookie;
diff --git a/shibsp/impl/StorageServiceSessionCache.cpp b/shibsp/impl/StorageServiceSessionCache.cpp
index 0bc8e52..3f86880 100644
--- a/shibsp/impl/StorageServiceSessionCache.cpp
+++ b/shibsp/impl/StorageServiceSessionCache.cpp
@@ -283,8 +283,12 @@ string SSCache::active(const Application& app, const HTTPRequest& request)
         if (!session_id.empty())
             return session_id;
     }
+
+    const PropertySet* props = app.getPropertySet("Sessions");
+    pair<bool,bool> sameSiteFallback = props ? props->getBool("sameSiteFallback") : pair<bool,bool>(false, false);
+
     pair<string, const char*> shib_cookie = app.getCookieNameProps("_shibsession_");
-    const char* session_id = request.getCookie(shib_cookie.first.c_str());
+    const char* session_id = request.getCookie(shib_cookie.first.c_str(), sameSiteFallback.first && sameSiteFallback.second);
     return (session_id ? session_id : "");
 }
 
@@ -571,16 +575,24 @@ void SSCache::insert(
         k += cookietimebuf;
     }
 
-    httpResponse.setCookie(shib_cookie.first.c_str(), k.c_str());
+    pair<bool,HTTPResponse::samesite_t> sameSitePolicy = getSameSitePolicy(app);
+
+    httpResponse.setCookie(shib_cookie.first.c_str(), k.c_str(), sameSitePolicy.second, sameSitePolicy.first);
     sessionID = key.get();
 
     // See if we need to persist the session data itself to a cookie for cross-node recovery.
     if (!m_persistedAttributeIds.empty()) {
-        persist(app, httpResponse, obj, expires);
+        persist(app, httpResponse, obj, expires, sameSitePolicy);
     }
 }
 
-void SSCache::persist(const Application& app, HTTPResponse& httpResponse, DDF& session, time_t expires) const
+void SSCache::persist(
+    const Application& app,
+    HTTPResponse& httpResponse,
+    DDF& session,
+    time_t expires,
+    pair<bool,HTTPResponse::samesite_t>& sameSitePolicy
+) const
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("persist");
@@ -633,7 +645,7 @@ void SSCache::persist(const Application& app, HTTPResponse& httpResponse, DDF& s
             strftime(cookietimebuf, 64, "; expires=%a, %d %b %Y %H:%M:%S GMT", ptime);
             sealed += cookietimebuf;
         }
-        httpResponse.setCookie(shib_cookie.first.c_str(), sealed.c_str());
+        httpResponse.setCookie(shib_cookie.first.c_str(), sealed.c_str(), sameSitePolicy.second, sameSitePolicy.first);
     }
     catch (const std::exception& e) {
         m_log.error("failed to wrap session (%s) with DataSealer: %s", session.name(), e.what());
@@ -896,6 +908,30 @@ LogoutEvent* SSCache::newLogoutEvent(const Application& app) const
 
 #endif
 
+pair<bool,HTTPResponse::samesite_t> SSCache::getSameSitePolicy(const Application& app) const
+{
+    HTTPResponse::samesite_t ss = HTTPResponse::SAMESITE_ABSENT;
+    pair<bool,bool> sameSiteFallback = pair<bool,bool>(false, false);
+
+    const PropertySet* props = app.getPropertySet("Sessions");
+    if (props) {
+        sameSiteFallback = props->getBool("sameSiteFallback");
+        pair<bool,const char*> sameSiteSession = props->getString("sameSiteSession");
+        if (sameSiteSession.first) {
+            if (!strcmp(sameSiteSession.second, "None")) {
+                ss = HTTPResponse::SAMESITE_NONE;
+            }
+            else if (!strcmp(sameSiteSession.second, "Lax")) {
+                ss = HTTPResponse::SAMESITE_LAX;
+            }
+            else if (!strcmp(sameSiteSession.second, "Strict")) {
+                ss = HTTPResponse::SAMESITE_STRICT;
+            }
+        }
+    }
+    return pair<bool,HTTPResponse::samesite_t>(sameSiteFallback.first && sameSiteFallback.second, ss);
+}
+
 Session* SSCache::_find(const Application& app, const char* key, const char* recovery, const char* client_addr, time_t* timeout)
 {
 #ifdef _DEBUG
@@ -1090,8 +1126,10 @@ Session* SSCache::find(const Application& app, HTTPRequest& request, const char*
     if (id.empty())
         return nullptr;
 
+    pair<bool,HTTPResponse::samesite_t> sameSitePolicy = getSameSitePolicy(app);
+
     pair<string, const char*> shib_cookie = app.getCookieNameProps("_shibsealed_");
-    const char* c = request.getCookie(shib_cookie.first.c_str());
+    const char* c = request.getCookie(shib_cookie.first.c_str(), sameSitePolicy.first);
 
     try {
         Session* session = _find(app, id.c_str(), c, client_addr, timeout);
@@ -1105,9 +1143,9 @@ Session* SSCache::find(const Application& app, HTTPRequest& request, const char*
             pair<string,const char*> shib_cookie = app.getCookieNameProps("_shibsession_");
             string exp(shib_cookie.second);
             exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
             shib_cookie = app.getCookieNameProps("_shibsealed_");
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
         }
     }
     catch (const std::exception&) {
@@ -1118,9 +1156,9 @@ Session* SSCache::find(const Application& app, HTTPRequest& request, const char*
             pair<string,const char*> shib_cookie = app.getCookieNameProps("_shibsession_");
             string exp(shib_cookie.second);
             exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
             shib_cookie = app.getCookieNameProps("_shibsealed_");
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
         }
         throw;
     }
@@ -1256,10 +1294,12 @@ void SSCache::remove(const Application& app, const HTTPRequest& request, HTTPRes
     string session_id;
     pair<string,const char*> shib_cookie = app.getCookieNameProps("_shibsession_");
 
+    pair<bool,HTTPResponse::samesite_t> sameSitePolicy = getSameSitePolicy(app);
+
     if (!m_inboundHeader.empty())
         session_id = request.getHeader(m_inboundHeader.c_str());
     if (session_id.empty()) {
-        const char* c = request.getCookie(shib_cookie.first.c_str());
+        const char* c = request.getCookie(shib_cookie.first.c_str(), sameSitePolicy.first);
         if (c && *c)
             session_id = c;
     }
@@ -1270,10 +1310,10 @@ void SSCache::remove(const Application& app, const HTTPRequest& request, HTTPRes
                 response->setResponseHeader(m_outboundHeader.c_str(), nullptr);
             string exp(shib_cookie.second);
             exp += "; expires=Mon, 01 Jan 2001 00:00:00 GMT";
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
 
             shib_cookie = app.getCookieNameProps("_shibsealed_");
-            response->setCookie(shib_cookie.first.c_str(), exp.c_str());
+            response->setCookie(shib_cookie.first.c_str(), exp.c_str(), sameSitePolicy.second, sameSitePolicy.first);
         }
         remove(app, session_id.c_str(), revocationExp);
     }
diff --git a/shibsp/impl/StorageServiceSessionCache.h b/shibsp/impl/StorageServiceSessionCache.h
index ae0f12d..09b3430 100644
--- a/shibsp/impl/StorageServiceSessionCache.h
+++ b/shibsp/impl/StorageServiceSessionCache.h
@@ -32,6 +32,7 @@
 
 #include <ctime>
 #include <boost/shared_ptr.hpp>
+#include <xmltooling/io/HTTPResponse.h>
 
 namespace xmltooling {
     class CondWait;
@@ -164,13 +165,21 @@ namespace shibsp {
         // handle potentially inexact address comparisons
         bool compareAddresses(const char* client_addr, const char* session_addr) const;
 
+        std::pair<bool,xmltooling::HTTPResponse::samesite_t> getSameSitePolicy(const Application& app) const;
+
         // management of buffered sessions
         void dormant(const char* key);
         static void* cleanup_fn(void*);
 
 #ifndef SHIBSP_LITE
         // persistence across nodes
-        void persist(const Application& app, xmltooling::HTTPResponse& httpResponse, DDF& session, time_t expires) const;
+        void persist(
+            const Application& app,
+            xmltooling::HTTPResponse& httpResponse,
+            DDF& session,
+            time_t expires,
+            std::pair<bool,xmltooling::HTTPResponse::samesite_t>& sameSitePolicy
+            ) const;
 #endif
         bool recover(const Application& app, const char* key, const char* data);
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list