[java-identity-provider] branch main updated: IDP-1872 - Use __Host- prefix cookie names to harden cookies

Scott Cantor cantor.2 at osu.edu
Wed Nov 10 21:13:59 UTC 2021


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

scantor pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=0ae2032af5c9693a29ad1529dec3b364c55f4971

The following commit(s) were added to refs/heads/main by this push:
     new 0ae2032af IDP-1872 - Use __Host- prefix cookie names to harden cookies
0ae2032af is described below

commit 0ae2032af5c9693a29ad1529dec3b364c55f4971
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Nov 10 16:13:56 2021 -0500

    IDP-1872 - Use __Host- prefix cookie names to harden cookies
    
    https://shibboleth.atlassian.net/browse/IDP-1872
---
 .../authn/spnego/impl/SPNEGOAutoLoginManager.java  | 32 ++++++++++++++++++----
 .../spnego/impl/SPNEGOAutoLoginManagerTest.java    | 11 ++++----
 .../shibboleth/idp/conf/session-manager-system.xml |  1 +
 idp-conf/src/main/resources/conf/idp.properties    |  4 +++
 idp-conf/src/test/resources/conf/idp.properties    |  6 +++-
 idp-war/src/main/webapp/WEB-INF/web.xml            |  7 ++++-
 6 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManager.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManager.java
index 8b540f8d3..cca6bf587 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManager.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManager.java
@@ -26,6 +26,7 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.net.CookieManager;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,7 +39,7 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
     /** Name of the SPNEGO auto-login signaling parameter. */
     @Nonnull @NotEmpty public static final String AUTOLOGIN_PARAMETER_NAME = "_shib_idp_SPNEGO_enable_autologin";
 
-    /** Name of the SPNEGO auto-login cookie. */
+    /** Default name of the SPNEGO auto-login cookie. */
     @Nonnull @NotEmpty public static final String AUTOLOGIN_COOKIE_NAME = "_idp_spnego_autologin";
 
     /** SPNEGO auto-login cookie value representing true. */
@@ -49,7 +50,15 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
 
     /** Manages creation of cookies. */
     @NonnullAfterInit private CookieManager cookieManager;
+    
+    /** Auto-login cookie name. */
+    @Nonnull @NotEmpty private String cookieName;
 
+    /** Constructor. */
+    public SPNEGOAutoLoginManager() {
+        cookieName = AUTOLOGIN_COOKIE_NAME;
+    }
+    
     /**
      * Set the {@link CookieManager} to use.
      * 
@@ -70,6 +79,19 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
         return cookieManager;
     }
 
+    /**
+     * Set the auto-login cookie name.
+     * 
+     * @param name cookie name
+     * 
+     * @since 4.2.0
+     */
+    public void setCookieName(@Nonnull @NotEmpty final String name) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
+        cookieName = Constraint.isNotEmpty(StringSupport.trimOrNull(name), "Cookie name cannot be null or empty");
+    }
+    
     /** {@inheritDoc} */
     @Override
     protected void doInitialize() throws ComponentInitializationException {
@@ -85,7 +107,7 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
      * Enable auto-login, i.e. set cookie to 'true'.
      */
     public void enable() {
-        cookieManager.addCookie(AUTOLOGIN_COOKIE_NAME, AUTOLOGIN_COOKIE_VALUE_TRUE);
+        cookieManager.addCookie(cookieName, AUTOLOGIN_COOKIE_VALUE_TRUE);
         log.debug("Auto-login has been enabled.");
     }
 
@@ -93,7 +115,7 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
      * Disable auto-login. i.e. unset cookie.
      */
     public void disable() {
-        cookieManager.unsetCookie(AUTOLOGIN_COOKIE_NAME);
+        cookieManager.unsetCookie(cookieName);
         log.debug("Auto-login has been disabled.");
     }
 
@@ -103,7 +125,7 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
      * @return true if auto-login is enabled.
      */
     public boolean isEnabled() {
-        return cookieManager.cookieHasValue(AUTOLOGIN_COOKIE_NAME, AUTOLOGIN_COOKIE_VALUE_TRUE);
+        return cookieManager.cookieHasValue(cookieName, AUTOLOGIN_COOKIE_VALUE_TRUE);
     }
 
     /**
@@ -115,7 +137,7 @@ public class SPNEGOAutoLoginManager extends AbstractInitializableComponent {
         /*
          * auto-login is considered disabled if cookie is absent or value is anything except "true".
          */
-        final String value = getCookieManager().getCookieValue(AUTOLOGIN_COOKIE_NAME, null);
+        final String value = getCookieManager().getCookieValue(cookieName, null);
         return value == null || !value.equals(AUTOLOGIN_COOKIE_VALUE_TRUE);
     }
 
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManagerTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManagerTest.java
index 13091f515..83065b2e8 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManagerTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/spnego/impl/SPNEGOAutoLoginManagerTest.java
@@ -52,6 +52,7 @@ public class SPNEGOAutoLoginManagerTest {
 
         SPNEGOAutoLoginManager autoLoginManager = new SPNEGOAutoLoginManager();
         autoLoginManager.setCookieManager(cookieManager);
+        autoLoginManager.setCookieName("spnego_autologin");
         autoLoginManager.initialize();
 
         return autoLoginManager;
@@ -64,7 +65,7 @@ public class SPNEGOAutoLoginManagerTest {
 
         autoLoginManager.enable();
 
-        Cookie cookie = res.getCookie(SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_NAME);
+        Cookie cookie = res.getCookie("spnego_autologin");
         Assert.assertNotNull(cookie);
         Assert.assertEquals(cookie.getValue(), SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_VALUE_TRUE);
     }
@@ -76,7 +77,7 @@ public class SPNEGOAutoLoginManagerTest {
 
         autoLoginManager.disable();
 
-        Cookie cookie = res.getCookie(SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_NAME);
+        Cookie cookie = res.getCookie("spnego_autologin");
         Assert.assertNotNull(cookie);
         Assert.assertNull(cookie.getValue());
     }
@@ -84,7 +85,7 @@ public class SPNEGOAutoLoginManagerTest {
     @Test
     public void givenCookieTrue_onlyIsEnabledShouldReturnTrue() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
-        req.setCookies(new Cookie(SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_NAME,
+        req.setCookies(new Cookie("spnego_autologin",
                 SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_VALUE_TRUE));
         SPNEGOAutoLoginManager autoLoginManager = createAutoLoginManager(req, null);
 
@@ -104,7 +105,7 @@ public class SPNEGOAutoLoginManagerTest {
     @Test
     public void givenCookieFalse_onlyIsDisabledShouldReturnTrue() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
-        req.setCookies(new Cookie(SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_NAME, AUTOLOGIN_COOKIE_VALUE_FALSE));
+        req.setCookies(new Cookie("spnego_autologin", AUTOLOGIN_COOKIE_VALUE_FALSE));
         SPNEGOAutoLoginManager autoLoginManager = createAutoLoginManager(req, null);
 
         Assert.assertFalse(autoLoginManager.isEnabled());
@@ -114,7 +115,7 @@ public class SPNEGOAutoLoginManagerTest {
     @Test
     public void givenCookieOtherValue_onlyIsDisabledShouldReturnTrue() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
-        req.setCookies(new Cookie(SPNEGOAutoLoginManager.AUTOLOGIN_COOKIE_NAME, AUTOLOGIN_COOKIE_VALUE_OTHER));
+        req.setCookies(new Cookie("spnego_autologin", AUTOLOGIN_COOKIE_VALUE_OTHER));
         SPNEGOAutoLoginManager autoLoginManager = createAutoLoginManager(req, null);
 
         Assert.assertFalse(autoLoginManager.isEnabled());
diff --git a/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/session-manager-system.xml b/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/session-manager-system.xml
index 7983bbf1b..c2150b286 100644
--- a/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/session-manager-system.xml
+++ b/idp-conf-impl/src/main/resources/net/shibboleth/idp/conf/session-manager-system.xml
@@ -67,6 +67,7 @@
             p:httpServletResponse-ref="shibboleth.HttpServletResponse"
             p:authenticationFlowDescriptors="#{@'shibboleth.AuthenticationFlowDescriptorManager'.getComponents()}"
             p:cookieManager-ref="shibboleth.CookieManager"
+            p:cookieName="%{idp.session.cookieName:shib_idp_session}"
             p:storageService-ref="#{'%{idp.session.StorageService:shibboleth.ClientSessionStorageService}'.trim()}"
             p:consistentAddressCondition="#{getObject('%{idp.session.consistentAddressCondition:}'.trim()) ?:
                 T(net.shibboleth.idp.session.impl.StorageBackedSessionManager.DefaultConsistentAddressConditionFactory).getDefaultConsistentAddressCondition(%{idp.session.consistentAddress:true})}"
diff --git a/idp-conf/src/main/resources/conf/idp.properties b/idp-conf/src/main/resources/conf/idp.properties
index 8b5d71a35..804cd8aa7 100644
--- a/idp-conf/src/main/resources/conf/idp.properties
+++ b/idp-conf/src/main/resources/conf/idp.properties
@@ -96,6 +96,8 @@ idp.trust.certificates = shibboleth.ExplicitKeyX509TrustEngine
 # Configuration of client- and server-side storage plugins
 #idp.storage.cleanupInterval = PT10M
 idp.storage.htmlLocalStorage = true
+#idp.storage.clientSessionStorageName = shib_idp_session_ss
+#idp.storage.clientPersistentStorageName = shib_idp_persistent_ss
 
 # Set to true to expose more detailed errors in responses to SPs
 #idp.errors.detailed = false
@@ -114,6 +116,8 @@ idp.storage.htmlLocalStorage = true
 # Set to "shibboleth.StorageService" for server-side storage of user sessions
 #idp.session.StorageService = shibboleth.ClientSessionStorageService
 
+# Name of cookie used for session
+#idp.session.cookieName = shib_idp_session
 # Size of session IDs
 #idp.session.idSize = 32
 # Bind sessions to IP addresses
diff --git a/idp-conf/src/test/resources/conf/idp.properties b/idp-conf/src/test/resources/conf/idp.properties
index 37c1297f2..65bc787cc 100644
--- a/idp-conf/src/test/resources/conf/idp.properties
+++ b/idp-conf/src/test/resources/conf/idp.properties
@@ -25,7 +25,7 @@ idp.scope = example.org
 #idp.cookie.secure = true
 #idp.cookie.httpOnly = true
 #idp.cookie.domain =
-#idp.cookie.path =
+idp.cookie.path = /
 #idp.cookie.maxAge = 31536000
 # These control operation of the SameSite filter, which is off by default.
 #idp.cookie.sameSite = None
@@ -92,6 +92,8 @@ idp.trust.certificates = shibboleth.ExplicitKeyX509TrustEngine
 # Configuration of client- and server-side storage plugins
 #idp.storage.cleanupInterval = PT10M
 idp.storage.htmlLocalStorage = true
+idp.storage.clientSessionStorageName = __Host-shib_idp_session_ss
+idp.storage.clientPersistentStorageName = __Host-shib_idp_persistent_ss
 
 # Set to true to expose more detailed errors in responses to SPs
 #idp.errors.detailed = false
@@ -110,6 +112,8 @@ idp.storage.htmlLocalStorage = true
 # Set to "shibboleth.StorageService" for server-side storage of user sessions
 #idp.session.StorageService = shibboleth.ClientSessionStorageService
 
+# Name of cookie used for session
+idp.session.cookieName = __Host-shib_idp_session
 # Size of session IDs
 #idp.session.idSize = 32
 # Bind sessions to IP addresses
diff --git a/idp-war/src/main/webapp/WEB-INF/web.xml b/idp-war/src/main/webapp/WEB-INF/web.xml
index 0940c5739..2f8581117 100644
--- a/idp-war/src/main/webapp/WEB-INF/web.xml
+++ b/idp-war/src/main/webapp/WEB-INF/web.xml
@@ -181,8 +181,13 @@
     <session-config>
         <session-timeout>15</session-timeout>
         <cookie-config>
-            <http-only>true</http-only>
+            <!-- Uncomment to add __Host- protection. -->
+            <!--
+            <name>__Host-JSESSIONID</name>
+            <path>/</path>
+            -->
             <secure>true</secure>
+            <http-only>true</http-only>
         </cookie-config>
         <tracking-mode>COOKIE</tracking-mode>
     </session-config>

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


More information about the commits mailing list