[FIXED JENKINS-25019]

A truly conforming servlet 3.0 container does not allow us to set "secure cookie" flag beyond ServletContextListener.onInitialized(). If we see that, don't scare the users.
jenkinsci · Oct 17, 2014 · 582128b · 582128b
1 parent d2a0a6c
commit 582128b
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 6 deletions.
diff --git a/changelog.html b/changelog.html
@@ -61,6 +61,10 @@
   <li class=bug>
     Prevent empty file creation if file parameter is left empty.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-3539">issue 3539</a>)
+  <li class=bug>
+    Servlet containers may refuse to let us set <a href="https://www.owasp.org/index.php/SecureFlag">secure cookie flag</a>.
+    Deal with it gracefully.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-25019">issue 25019</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/WebAppMain.java b/core/src/main/java/hudson/WebAppMain.java
@@ -56,6 +56,7 @@
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Date;
@@ -116,6 +117,8 @@ public Locale get() {
 
             installLogger();
 
+            markCookieAsHttpOnly(context);
+
             final FileAndDescription describedHomeDir = getHomeDir(event);
             home = describedHomeDir.file.getAbsoluteFile();
             home.mkdirs();
@@ -251,6 +254,31 @@ public void run() {
         }
     }
 
+    /**
+     * Set the session cookie as HTTP only.
+     *
+     * @see <a href="https://www.owasp.org/index.php/HttpOnly">discussion of this topic in OWASP</a>
+     */
+    private void markCookieAsHttpOnly(ServletContext context) {
+        try {
+            Method m;
+            try {
+                m = context.getClass().getMethod("getSessionCookieConfig");
+            } catch (NoSuchMethodException x) { // 3.0+
+                LOGGER.log(Level.FINE, "Failed to set secure cookie flag", x);
+                return;
+            }
+            Object sessionCookieConfig = m.invoke(context);
+
+            // not exposing session cookie to JavaScript to mitigate damage caused by XSS
+            Class scc = Class.forName("javax.servlet.SessionCookieConfig");
+            Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class);
+            setHttpOnly.invoke(sessionCookieConfig,true);
+        } catch (Exception e) {
+            LOGGER.log(Level.WARNING, "Failed to set HTTP-only cookie flag", e);
+        }
+    }
+
     public void joinInit() throws InterruptedException {
         initThread.join();
     }

diff --git a/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java b/core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java
@@ -14,6 +14,7 @@
 import javax.servlet.ServletContext;
 import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -117,14 +118,17 @@ private void updateSecureSessionFlag() {
             }
             Object sessionCookieConfig = m.invoke(context);
 
-            // not exposing session cookie to JavaScript to mitigate damage caused by XSS
             Class scc = Class.forName("javax.servlet.SessionCookieConfig");
-            Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class);
-            setHttpOnly.invoke(sessionCookieConfig,true);
-
-            Method setSecure = scc.getMethod("setSecure",boolean.class);
+            Method setSecure = scc.getMethod("setSecure", boolean.class);
             boolean v = fixNull(jenkinsUrl).startsWith("https");
-            setSecure.invoke(sessionCookieConfig,v);
+            setSecure.invoke(sessionCookieConfig, v);
+        } catch (InvocationTargetException e) {
+            if (e.getTargetException() instanceof IllegalStateException) {
+                // servlet 3.0 spec seems to prohibit this from getting set at runtime,
+                // though Winstone is happy to accept i. see JENKINS-25019
+                return;
+            }
+            LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e);
         } catch (Exception e) {
             LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e);
         }