Session fixation vulnerability in Jenkins

@@ -34,6 +34,7 @@
 import javax.servlet.http.HttpSession;
 import jenkins.security.SecurityListener;
 import jenkins.security.seed.UserSeedProperty;
+import jenkins.util.SystemProperties;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.springframework.security.core.Authentication;
@@ -60,15 +61,13 @@ public AuthenticationProcessingFilter2(String authenticationGatewayUrl) {
  
     @Override
     protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, Authentication authResult) throws IOException, ServletException {
+        if (SystemProperties.getInteger(SecurityRealm.class.getName() + ".sessionFixationProtectionMode", 1) == 2) {
+            // This is the default session fixation prevention fix.
+            // While use of SessionFixationProtectionStrategy would be the canonical Spring Security approach, it seems less compatible with some security realms.
+            request.getSession().invalidate();
+            HttpSession newSession = request.getSession(true);
+        }
         super.successfulAuthentication(request, response, chain, authResult);
-        // make sure we have a session to store this successful authentication, given that we no longer
-        // let HttpSessionContextIntegrationFilter2 to create sessions.
-        // SecurityContextPersistenceFilter stores the updated SecurityContext object into this session later
-        // (either when a redirect is issued, via its HttpResponseWrapper, or when the execution returns to its
-        // doFilter method.
-        /* TODO causes an ISE on the next line:
-        request.getSession().invalidate();
-        */
         HttpSession newSession = request.getSession();
  
         if (!UserSeedProperty.DISABLE_USER_SEED) {

@@ -49,6 +49,7 @@
 import javax.servlet.http.HttpSession;
 import jenkins.model.IdStrategy;
 import jenkins.model.Jenkins;
+import jenkins.util.SystemProperties;
 import jenkins.security.AcegiSecurityExceptionFilter;
 import jenkins.security.BasicHeaderProcessor;
 import jenkins.security.AuthenticationSuccessHandler;
@@ -78,6 +79,7 @@
 import org.springframework.security.web.authentication.rememberme.AbstractRememberMeServices;
 import org.springframework.security.web.authentication.rememberme.RememberMeAuthenticationFilter;
 import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint;
+import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
 import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
  
 /**
@@ -593,6 +595,10 @@ public Filter createFilter(FilterConfig filterConfig) {
         {
             AuthenticationProcessingFilter2 apf = new AuthenticationProcessingFilter2(getAuthenticationGatewayUrl());
             apf.setAuthenticationManager(sc.manager2);
+            if (SystemProperties.getInteger(SecurityRealm.class.getName() + ".sessionFixationProtectionMode", 1) == 1) {
+                // Optionally use the 'canonical' protection from Spring Security; see AuthenticationProcessingFilter2#successfulAuthentication for default
+                apf.setSessionAuthenticationStrategy(new SessionFixationProtectionStrategy());
+            }
             apf.setRememberMeServices(sc.rememberMe2);
             final AuthenticationSuccessHandler successHandler = new AuthenticationSuccessHandler();
             successHandler.setTargetUrlParameter("from");

@@ -0,0 +1,124 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2021 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package hudson.security;
+ 
+import com.gargoylesoftware.htmlunit.util.Cookie;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+ 
+import java.util.Arrays;
+import java.util.List;
+ 
+/**
+ * Split from {@link SecurityRealmTest} because this is parameterized.
+ */
+@RunWith(Parameterized.class)
+public class SecurityRealmSecurity2371Test {
+ 
+    public static final String SESSION_COOKIE_NAME = "JSESSIONID";
+    public static final String USERNAME = "alice";
+ 
+    private final Integer mode;
+ 
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+ 
+    @Parameterized.Parameters
+    public static List<Integer> modes() {
+        return Arrays.asList(null, 1, 2);
+    }
+ 
+    public SecurityRealmSecurity2371Test(Integer mode) {
+        this.mode = mode;
+    }
+ 
+    @Test
+    public void testSessionChangeOnLogin() throws Exception {
+        if (mode != null) {
+            System.setProperty(SecurityRealm.class.getName() + ".sessionFixationProtectionMode", String.valueOf(mode));
+        }
+        try {
+            j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+            j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().toEveryone().grant(Jenkins.ADMINISTER).everywhere().to(USERNAME));
+            final JenkinsRule.WebClient webClient = j.createWebClient();
+            webClient.goTo("");
+            try {
+                webClient.goTo("manage");
+                Assert.fail("anonymous session should not be able to go to /manage");
+            } catch (Exception ex) {
+                // OK
+            }
+            final Cookie anonymousCookie = webClient.getCookieManager().getCookie(SESSION_COOKIE_NAME); // dynamic cookie names are only set when run through Winstone
+            webClient.login(USERNAME);
+            webClient.goTo("");
+            final Cookie aliceCookie = webClient.getCookieManager().getCookie(SESSION_COOKIE_NAME);
+ 
+            // Confirm the session cookie changed
+            // We cannot just call #assertNotEquals(Cookie, Cookie) because it doesn't actually look at #getValue()
+            Assert.assertNotEquals(anonymousCookie.getValue(), aliceCookie.getValue());
+ 
+            // Now ensure the old session was actually invalidated / is not associated with the new auth
+            webClient.getCookieManager().clearCookies();
+            webClient.getCookieManager().addCookie(anonymousCookie);
+            try {
+                webClient.goTo("manage");
+                Assert.fail("anonymous session should not be able to go to /manage");
+            } catch (Exception ex) {
+                // OK
+            }
+        } finally {
+            System.clearProperty(SecurityRealm.class.getName() + ".sessionFixationProtectionMode");
+        }
+    }
+ 
+    /**
+     * Explicitly disable
+     */
+    @Test
+    public void optOut() throws Exception {
+        System.setProperty(SecurityRealm.class.getName() + ".sessionFixationProtectionMode", String.valueOf(0));
+        try {
+            j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+            j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().toEveryone().grant(Jenkins.ADMINISTER).everywhere().to(USERNAME));
+            final JenkinsRule.WebClient webClient = j.createWebClient();
+            webClient.goTo("");
+ 
+            final Cookie anonymousCookie = webClient.getCookieManager().getCookie(SESSION_COOKIE_NAME); // dynamic cookie names are only set when run through Winstone
+            webClient.login(USERNAME);
+            webClient.goTo("");
+            final Cookie aliceCookie = webClient.getCookieManager().getCookie(SESSION_COOKIE_NAME);
+ 
+            // Confirm the session cookie did not change
+            Assert.assertEquals(anonymousCookie.getValue(), aliceCookie.getValue());
+        } finally {
+            System.clearProperty(SecurityRealm.class.getName() + ".sessionFixationProtectionMode");
+        }
+    }
+}