From 97199364bae41ed3c0ef3e44d4dd9be2901eab98 Mon Sep 17 00:00:00 2001 From: scantor Date: Sat, 30 Jun 2012 23:10:44 +0000 Subject: [PATCH] https://issues.shibboleth.net/jira/browse/SSPCPP-471 git-svn-id: https://svn.shibboleth.net/cpp-sp/branches/REL_2@3720 cb58f699-b61c-0410-a6fe-9272a202ed29 --- configs/example-shibboleth2.xml | 10 ++++----- configs/shibboleth2.xml | 11 +++++----- configs/win-shibboleth2.xml | 11 +++++----- shibsp/Application.cpp | 13 +++++++----- shibsp/handler/impl/AssertionConsumerService.cpp | 27 +++++++++++++----------- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/configs/example-shibboleth2.xml b/configs/example-shibboleth2.xml index b7cdd1b..8a3dad3 100644 --- a/configs/example-shibboleth2.xml +++ b/configs/example-shibboleth2.xml @@ -102,12 +102,12 @@ You MUST supply an effectively unique handlerURL value for each of your applications. The value defaults to /Shibboleth.sso, and should be a relative path, with the SP computing a relative value based on the virtual host. Using handlerSSL="true", the default, will force - the protocol to be https. You should also add a cookieProps setting of "; path=/; secure; HttpOnly" - in that case. Note that while we default checkAddress to "false", this has a negative - impact on the security of the SP. Stealing cookies/sessions is much easier with this disabled. + the protocol to be https. You should also set cookieProps to "https" for SSL-only sites. + Note that while we default checkAddress to "false", this has a negative impact on the + security of your site. Stealing sessions via cookie theft is much easier with this disabled. --> @@ -125,7 +125,7 @@ + entityID="https://idp.example.org/idp/shibboleth"> diff --git a/configs/shibboleth2.xml b/configs/shibboleth2.xml index bb3879c..b17f0f2 100644 --- a/configs/shibboleth2.xml +++ b/configs/shibboleth2.xml @@ -28,11 +28,12 @@ You MUST supply an effectively unique handlerURL value for each of your applications. The value defaults to /Shibboleth.sso, and should be a relative path, with the SP computing a relative value based on the virtual host. Using handlerSSL="true", the default, will force - the protocol to be https. You should also add a cookieProps setting of "; path=/; secure; HttpOnly" - in that case. Note that while we default checkAddress to "false", this has a negative - impact on the security of the SP. Stealing cookies/sessions is much easier with this disabled. + the protocol to be https. You should also set cookieProps to "https" for SSL-only sites. + Note that while we default checkAddress to "false", this has a negative impact on the + security of your site. Stealing sessions via cookie theft is much easier with this disabled. --> - + - SAML2 SAML1 diff --git a/configs/win-shibboleth2.xml b/configs/win-shibboleth2.xml index 68aa802..c795edf 100644 --- a/configs/win-shibboleth2.xml +++ b/configs/win-shibboleth2.xml @@ -71,11 +71,12 @@ You MUST supply an effectively unique handlerURL value for each of your applications. The value defaults to /Shibboleth.sso, and should be a relative path, with the SP computing a relative value based on the virtual host. Using handlerSSL="true", the default, will force - the protocol to be https. You should also add a cookieProps setting of "; path=/; secure; HttpOnly" - in that case. Note that while we default checkAddress to "false", this has a negative - impact on the security of the SP. Stealing cookies/sessions is much easier with this disabled. + the protocol to be https. You should also set cookieProps to "https" for SSL-only sites. + Note that while we default checkAddress to "false", this has a negative impact on the + security of your site. Stealing sessions via cookie theft is much easier with this disabled. --> - + - SAML2 SAML1 diff --git a/shibsp/Application.cpp b/shibsp/Application.cpp index 3a459f8..e41f8f1 100644 --- a/shibsp/Application.cpp +++ b/shibsp/Application.cpp @@ -63,22 +63,25 @@ const char* Application::getId() const pair Application::getCookieNameProps(const char* prefix, time_t* lifetime) const { static const char* defProps="; path=/; HttpOnly"; + static const char* sslProps="; path=/; secure; HttpOnly"; if (lifetime) *lifetime = 0; if (!prefix) prefix = ""; - const PropertySet* props=getPropertySet("Sessions"); + const PropertySet* props = getPropertySet("Sessions"); if (props) { if (lifetime) { pair lt = props->getUnsignedInt("cookieLifetime"); if (lt.first) *lifetime = lt.second; } - pair p=props->getString("cookieProps"); - if (!p.first) - p.second=defProps; - pair p2=props->getString("cookieName"); + pair p = props->getString("cookieProps"); + if (!p.first || !strcmp(p.second, "http")) + p.second = defProps; + else if (!strcmp(p.second, "https")) + p.second = sslProps; + pair p2 = props->getString("cookieName"); if (p2.first) return make_pair(string(prefix) + p2.second, p.second); return make_pair(string(prefix) + getHash(), p.second); diff --git a/shibsp/handler/impl/AssertionConsumerService.cpp b/shibsp/handler/impl/AssertionConsumerService.cpp index c42abd9..7176f81 100644 --- a/shibsp/handler/impl/AssertionConsumerService.cpp +++ b/shibsp/handler/impl/AssertionConsumerService.cpp @@ -654,36 +654,39 @@ void AssertionConsumerService::maintainHistory( ) const { static const char* defProps="; path=/"; + static const char* sslProps="; path=/; secure"; - const PropertySet* sessionProps=application.getPropertySet("Sessions"); - pair idpHistory=sessionProps->getBool("idpHistory"); + const PropertySet* sessionProps = application.getPropertySet("Sessions"); + pair idpHistory = sessionProps->getBool("idpHistory"); if (idpHistory.first && idpHistory.second) { - pair cookieProps=sessionProps->getString("idpHistoryProps"); + pair cookieProps = sessionProps->getString("idpHistoryProps"); if (!cookieProps.first) - cookieProps=sessionProps->getString("cookieProps"); - if (!cookieProps.first) - cookieProps.second=defProps; + cookieProps = sessionProps->getString("cookieProps"); + if (!cookieProps.first || !strcmp(cookieProps.second, "http")) + cookieProps.second = defProps; + else if (!strcmp(cookieProps.second, "https")) + cookieProps.second = sslProps; // Set an IdP history cookie locally (essentially just a CDC). CommonDomainCookie cdc(request.getCookie(CommonDomainCookie::CDCName)); // Either leave in memory or set an expiration. - pair days=sessionProps->getUnsignedInt("idpHistoryDays"); - if (!days.first || days.second==0) { + pair days = sessionProps->getUnsignedInt("idpHistoryDays"); + if (!days.first || days.second == 0) { string c = string(cdc.set(entityID)) + cookieProps.second; response.setCookie(CommonDomainCookie::CDCName, c.c_str()); } else { - time_t now=time(nullptr) + (days.second * 24 * 60 * 60); + time_t now = time(nullptr) + (days.second * 24 * 60 * 60); #ifdef HAVE_GMTIME_R struct tm res; - struct tm* ptime=gmtime_r(&now,&res); + struct tm* ptime = gmtime_r(&now,&res); #else - struct tm* ptime=gmtime(&now); + struct tm* ptime = gmtime(&now); #endif char timebuf[64]; - strftime(timebuf,64,"%a, %d %b %Y %H:%M:%S GMT",ptime); + strftime(timebuf,64,"%a, %d %b %Y %H:%M:%S GMT", ptime); string c = string(cdc.set(entityID)) + cookieProps.second + "; expires=" + timebuf; response.setCookie(CommonDomainCookie::CDCName, c.c_str()); } -- 2.1.4