Skip to content

Commit 582128b

Browse files
committed
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.
1 parent d2a0a6c commit 582128b

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

Diff for: changelog.html

+4
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@
6161
<li class=bug>
6262
Prevent empty file creation if file parameter is left empty.
6363
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-3539">issue 3539</a>)
64+
<li class=bug>
65+
Servlet containers may refuse to let us set <a href="https://www.owasp.org/index.php/SecureFlag">secure cookie flag</a>.
66+
Deal with it gracefully.
67+
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-25019">issue 25019</a>)
6468
</ul>
6569
</div><!--=TRUNK-END=-->
6670

Diff for: core/src/main/java/hudson/WebAppMain.java

+28
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.io.File;
5757
import java.io.FileOutputStream;
5858
import java.io.IOException;
59+
import java.lang.reflect.Method;
5960
import java.net.URL;
6061
import java.net.URLClassLoader;
6162
import java.util.Date;
@@ -116,6 +117,8 @@ public Locale get() {
116117

117118
installLogger();
118119

120+
markCookieAsHttpOnly(context);
121+
119122
final FileAndDescription describedHomeDir = getHomeDir(event);
120123
home = describedHomeDir.file.getAbsoluteFile();
121124
home.mkdirs();
@@ -251,6 +254,31 @@ public void run() {
251254
}
252255
}
253256

257+
/**
258+
* Set the session cookie as HTTP only.
259+
*
260+
* @see <a href="https://www.owasp.org/index.php/HttpOnly">discussion of this topic in OWASP</a>
261+
*/
262+
private void markCookieAsHttpOnly(ServletContext context) {
263+
try {
264+
Method m;
265+
try {
266+
m = context.getClass().getMethod("getSessionCookieConfig");
267+
} catch (NoSuchMethodException x) { // 3.0+
268+
LOGGER.log(Level.FINE, "Failed to set secure cookie flag", x);
269+
return;
270+
}
271+
Object sessionCookieConfig = m.invoke(context);
272+
273+
// not exposing session cookie to JavaScript to mitigate damage caused by XSS
274+
Class scc = Class.forName("javax.servlet.SessionCookieConfig");
275+
Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class);
276+
setHttpOnly.invoke(sessionCookieConfig,true);
277+
} catch (Exception e) {
278+
LOGGER.log(Level.WARNING, "Failed to set HTTP-only cookie flag", e);
279+
}
280+
}
281+
254282
public void joinInit() throws InterruptedException {
255283
initThread.join();
256284
}

Diff for: core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java

+10-6
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import javax.servlet.ServletContext;
1515
import java.io.File;
1616
import java.io.IOException;
17+
import java.lang.reflect.InvocationTargetException;
1718
import java.lang.reflect.Method;
1819
import java.util.logging.Level;
1920
import java.util.logging.Logger;
@@ -117,14 +118,17 @@ private void updateSecureSessionFlag() {
117118
}
118119
Object sessionCookieConfig = m.invoke(context);
119120

120-
// not exposing session cookie to JavaScript to mitigate damage caused by XSS
121121
Class scc = Class.forName("javax.servlet.SessionCookieConfig");
122-
Method setHttpOnly = scc.getMethod("setHttpOnly",boolean.class);
123-
setHttpOnly.invoke(sessionCookieConfig,true);
124-
125-
Method setSecure = scc.getMethod("setSecure",boolean.class);
122+
Method setSecure = scc.getMethod("setSecure", boolean.class);
126123
boolean v = fixNull(jenkinsUrl).startsWith("https");
127-
setSecure.invoke(sessionCookieConfig,v);
124+
setSecure.invoke(sessionCookieConfig, v);
125+
} catch (InvocationTargetException e) {
126+
if (e.getTargetException() instanceof IllegalStateException) {
127+
// servlet 3.0 spec seems to prohibit this from getting set at runtime,
128+
// though Winstone is happy to accept i. see JENKINS-25019
129+
return;
130+
}
131+
LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e);
128132
} catch (Exception e) {
129133
LOGGER.log(Level.WARNING, "Failed to set secure cookie flag", e);
130134
}

0 commit comments

Comments
 (0)