[JENKINS-61738][2] Session hijacking protection hardening (#4615)

* [JENKINS-61738] Session hijacking protection hardening

* Workaround bad test

* Better approach in terms of test

* Get rid of small whitespace

* Update core/src/main/java/hudson/WebAppMain.java
Co-Authored-By: NJeff Thompson <37345299+jeffret-b@users.noreply.github.com>

* Update war/src/main/webapp/WEB-INF/web.xml
Co-Authored-By: NJeff Thompson <37345299+jeffret-b@users.noreply.github.com>

* Use release companion PR
Co-authored-by: NJeff Thompson <37345299+jeffret-b@users.noreply.github.com>

core/src/main/java/hudson/WebAppMain.java

@@ -48,6 +48,8 @@ import hudson.util.ChartUtil;
 import hudson.util.AWTProblem;
 import jenkins.util.JenkinsJVM;
 import org.jvnet.localizer.LocaleProvider;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.jelly.JellyFacet;
 import org.apache.tools.ant.types.FileSet;
 
@@ -58,6 +60,7 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 import javax.servlet.ServletResponse;
+import javax.servlet.SessionTrackingMode;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.TransformerFactoryConfigurationError;
 import java.io.File;
@@ -65,6 +68,7 @@ import java.io.IOException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Date;
+import java.util.EnumSet;
 import java.util.Locale;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -83,6 +87,28 @@ public class WebAppMain implements ServletContextListener {
     // use RingBufferLogHandler class name to configure for backward compatibility
     private static final int DEFAULT_RING_BUFFER_SIZE = SystemProperties.getInteger(RingBufferLogHandler.class.getName() + ".defaultSize", 256);
 
+    /**
+     * System property name to force the session tracking by cookie.
+     * This prevents Tomcat to use the URL tracking in addition to the cookie by default.
+     * This could be useful for instances that requires to have
+     * the {@link jenkins.security.SuspiciousRequestFilter#allowSemicolonsInPath} turned off.
+     * <p>
+     * If you allow semicolon in URL and the session to be tracked by URL and you have
+     * a SecurityRealm that does not invalidate session after authentication,
+     * your instance is vulnerable to session hijacking.
+     * <p>
+     * The SecurityRealm should be corrected but this is a hardening in Jenkins core.
+     * <p>
+     * As this property is read during startup, you will not be able to change it at runtime 
+     * depending on your application server (not possible with Jetty nor Tomcat)
+     * <p>
+     * When running hpi:run, the default tracking is COOKIE+URL.
+     * When running java -jar with Winstone/Jetty, the default setting is set to COOKIE only.
+     * When running inside Tomcat, the default setting is COOKIE+URL.
+     */
+    @Restricted(NoExternalUse.class)
+    public static final String FORCE_SESSION_TRACKING_BY_COOKIE_PROP = WebAppMain.class.getName() + ".forceSessionTrackingByCookie";
+
     private final RingBufferLogHandler handler = new RingBufferLogHandler(DEFAULT_RING_BUFFER_SIZE) {
         @Override public synchronized void publish(LogRecord record) {
             if (record.getLevel().intValue() >= Level.INFO.intValue()) {
@@ -223,6 +249,9 @@ public class WebAppMain implements ServletContextListener {
             installExpressionFactory(event);
 
             context.setAttribute(APP,new HudsonIsLoading());
+            if (SystemProperties.getBoolean(FORCE_SESSION_TRACKING_BY_COOKIE_PROP, true)) {
+                context.setSessionTrackingModes(EnumSet.of(SessionTrackingMode.COOKIE));
+            }
 
             final File _home = home;
             initThread = new Thread("Jenkins initialization thread") {

test/pom.xml

@@ -70,7 +70,7 @@ THE SOFTWARE.
     <dependency>
       <groupId>${project.groupId}</groupId>
       <artifactId>jenkins-test-harness</artifactId>
-      <version>2.62</version>
+      <version>2.63</version>
       <scope>test</scope>
       <exclusions>
         <exclusion>

test/src/test/java/hudson/util/BootFailureTest.java

@@ -3,7 +3,6 @@ package hudson.util;
 import hudson.WebAppMain;
 import hudson.model.Hudson;
 import hudson.model.listeners.ItemListener;
-import static hudson.util.BootFailureTest.makeBootFail;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -14,17 +13,21 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import jenkins.model.Jenkins;
 import org.apache.commons.io.FileUtils;
+import org.eclipse.jetty.util.component.AbstractLifeCycle;
 import org.junit.After;
 import static org.junit.Assert.*;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.HudsonHomeLoader;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.NoListenerConfiguration;
 import org.jvnet.hudson.test.TestEnvironment;
 import org.jvnet.hudson.test.TestExtension;
 import org.kohsuke.stapler.WebApp;
+import org.springframework.util.Assert;
 
 /**
  *
@@ -49,7 +52,6 @@ public class BootFailureTest {
         @Override
         public Hudson newHudson() throws Exception {
             localPort = 0;
-            ServletContext ws = createWebServer();
             wa = new WebAppMain() {
                 @Override
                 public WebAppMain.FileAndDescription getHomeDir(ServletContextEvent event) {
@@ -60,7 +62,26 @@ public class BootFailureTest {
                     }
                 }
             };
-            wa.contextInitialized(new ServletContextEvent(ws));
+            // Without this gymnastic, the jenkins-test-harness adds a NoListenerConfiguration
+            // that prevents us to inject our own custom WebAppMain
+            // With this approach we can make the server calls the regular contextInitialized
+            ServletContext ws = createWebServer((context, server) -> {
+                NoListenerConfiguration noListenerConfiguration = context.getBean(NoListenerConfiguration.class);
+                // future-proof
+                Assert.notNull(noListenerConfiguration);
+                if (noListenerConfiguration != null) {
+                    context.removeBean(noListenerConfiguration);
+                    context.addBean(new AbstractLifeCycle() {
+                        @Override 
+                        protected void doStart() throws Exception {
+                            // default behavior of noListenerConfiguration
+                            context.setEventListeners(null);
+                            // ensuring our custom context will received the contextInitialized event
+                            context.addEventListener(wa);
+                        }
+                    });
+                }
+            });
             wa.joinInit();
 
             Object a = WebApp.get(ws).getApp();

war/src/main/webapp/WEB-INF/web.xml

@@ -271,5 +271,6 @@ THE SOFTWARE.
       <!-- See https://www.owasp.org/index.php/HttpOnly for the discussion of this topic in OWASP -->
       <http-only>true</http-only>
     </cookie-config>
+    <!-- Tracking mode is managed by WebAppMain.FORCE_SESSION_TRACKING_BY_COOKIE_PROP -->
   </session-config>
 </web-app>