[JENKINS-25019] updateSecureSessionFlag fails to set secure cookie flag - Jenkins Jira

XML

Word

Printable

Details

Type: Bug
Status: Resolved (View Workflow)
Priority: Minor
Resolution: Fixed
Component/s: core
Labels:
- security
Environment:
Amazon Linux
Tomcat 7.0.55
Jenkins 1.565.3

Similar Issues:

Show

Description

The following stacktrace is logged when Jenkins is started on Tomcat 7.

Oct 07, 2014 12:18:17 PM INFO jenkins.InitReactorRunner$1 onAttained
Started initialization
Oct 07, 2014 12:18:17 PM INFO jenkins.InitReactorRunner$1 onAttained
Listed all plugins
Oct 07, 2014 12:18:17 PM INFO jenkins.InitReactorRunner$1 onAttained
Prepared all plugins
Oct 07, 2014 12:18:21 PM WARNING jenkins.model.JenkinsLocationConfiguration updateSecureSessionFlag
Failed to set secure cookie flag
java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at jenkins.model.JenkinsLocationConfiguration.updateSecureSessionFlag(JenkinsLocationConfiguration.java:123)
	at jenkins.model.JenkinsLocationConfiguration.load(JenkinsLocationConfiguration.java:71)
	at jenkins.model.JenkinsLocationConfiguration.<init>(JenkinsLocationConfiguration.java:46)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
	at com.google.inject.internal.DefaultConstructionProxyFactory$1.newInstance(DefaultConstructionProxyFactory.java:86)
	at com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:108)
	at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:88)
	at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:269)
	at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
	at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1058)
	at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
	at com.google.inject.Scopes$1$1.get(Scopes.java:65)
	at hudson.ExtensionFinder$GuiceFinder$FaultTolerantScope$1.get(ExtensionFinder.java:429)
	at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:41)
	at com.google.inject.internal.InjectorImpl$3$1.call(InjectorImpl.java:1005)
	at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1051)
	at com.google.inject.internal.InjectorImpl$3.get(InjectorImpl.java:1001)
	at hudson.ExtensionFinder$GuiceFinder._find(ExtensionFinder.java:391)
	at hudson.ExtensionFinder$GuiceFinder.find(ExtensionFinder.java:382)
	at hudson.ExtensionFinder._find(ExtensionFinder.java:151)
	at hudson.ClassicPluginStrategy.findComponents(ClassicPluginStrategy.java:341)
	at hudson.ExtensionList.load(ExtensionList.java:295)
	at hudson.ExtensionList.ensureLoaded(ExtensionList.java:248)
	at hudson.ExtensionList.iterator(ExtensionList.java:138)
	at jenkins.model.Jenkins.getDescriptorByType(Jenkins.java:1193)
	at hudson.plugins.copyartifact.BuildSelectorParameter.initAliases(BuildSelectorParameter.java:99)
	at hudson.plugins.copyartifact.CopyArtifactPlugin.postInitialize(CopyArtifactPlugin.java:35)
	at hudson.PluginManager$2$1$2.run(PluginManager.java:376)
	at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:169)
	at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:282)
	at jenkins.model.Jenkins$7.runTask(Jenkins.java:905)
	at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:210)
	at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalStateException: Property HttpOnly can not be added to SessionCookieConfig for context /jenkins as the context has been initialised
	at org.apache.catalina.core.ApplicationSessionCookieConfig.setHttpOnly(ApplicationSessionCookieConfig.java:107)
	... 43 more

Oct 07, 2014 12:18:22 PM INFO jenkins.InitReactorRunner$1 onAttained
Started all plugins

I assume this will also happen with other versions of Tomcat >= 7.0.41 (See https://issues.apache.org/bugzilla/show_bug.cgi?id=54974).

Should Jenkins check the state of the context before attempting to set the HttpOnly property?

Attachments

Issue Links

is blocking

JENKINS-36499 Migrate openid to parent pom

Closed

Activity

Ascending order - Click to sort in descending order

Kohsuke Kawaguchi added a comment - 2014-10-17 02:04

These methods have the javadoc that says the following:

Throws: IllegalStateException - if the ServletContext from which this SessionCookieConfig was acquired has already been initialized

I coudln't find what it means for "ServletContext ... has already been initialized".

Servlet 3.0 spec section 4.4 says the following:

The following methods are added to ServletContext since Servlet 3.0 to enable programmatic definition of servlets, filters and the url pattern that they map to. These methods can only be called during the initialization of the application either from the contexInitialized method of a ServletContextListener implementation or from the onStartup method of a ServletContainerInitializer implementation.

... although "the following methods" do not include getSessionCookieConfig().

Based on those two data points and how Tomcat behaves, I assume the intent of the spec is that the method in question can be only called from ServletContextListener.contextInitialized().

Unfortunately, this means we can't reliably invoke the "setSecure" method, because during the initialization we do not know if Jenkins is only serving HTTPs or not. Winstone seems to accept this happily, so what I'm going to do is to call this method optimistically, but suppress IllegalStateException if we get one. Users who are running on Tomcat and other "more conforming" application containers would have to configure this by themselves through container-specific means. See this document for how to do this with Tomcat.

Fortunately, SECURITY-120 is classified as hardening and not vulnerability, and javadoc of setSecure() method alludes that if the session was created over HTTPS, then the secure cookie flag is set by default.

The code that sets HTTP-only flag can be moved into ServletContextListener.contextInitialized() to ensure it works correctly in Tomcat.

Kohsuke Kawaguchi added a comment - 2014-10-17 02:04 These methods have the javadoc that says the following: Throws: IllegalStateException - if the ServletContext from which this SessionCookieConfig was acquired has already been initialized I coudln't find what it means for "ServletContext ... has already been initialized". Servlet 3.0 spec section 4.4 says the following: The following methods are added to ServletContext since Servlet 3.0 to enable programmatic definition of servlets, filters and the url pattern that they map to. These methods can only be called during the initialization of the application either from the contexInitialized method of a ServletContextListener implementation or from the onStartup method of a ServletContainerInitializer implementation. ... although "the following methods" do not include getSessionCookieConfig() . Based on those two data points and how Tomcat behaves, I assume the intent of the spec is that the method in question can be only called from ServletContextListener.contextInitialized() . Unfortunately, this means we can't reliably invoke the "setSecure" method, because during the initialization we do not know if Jenkins is only serving HTTPs or not. Winstone seems to accept this happily, so what I'm going to do is to call this method optimistically, but suppress IllegalStateException if we get one. Users who are running on Tomcat and other "more conforming" application containers would have to configure this by themselves through container-specific means. See this document for how to do this with Tomcat. Fortunately, SECURITY-120 is classified as hardening and not vulnerability, and javadoc of setSecure() method alludes that if the session was created over HTTPS, then the secure cookie flag is set by default. The code that sets HTTP-only flag can be moved into ServletContextListener.contextInitialized() to ensure it works correctly in Tomcat.

SCM/JIRA link daemon added a comment - 2014-10-17 03:03

Code changed in jenkins
User: Kohsuke Kawaguchi
Path:
changelog.html
core/src/main/java/hudson/WebAppMain.java
core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java
http://jenkins-ci.org/commit/jenkins/582128b9ac179a788d43c1478be8a5224dc19710
Log:
[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.

SCM/JIRA link daemon added a comment - 2014-10-17 03:03 Code changed in jenkins User: Kohsuke Kawaguchi Path: changelog.html core/src/main/java/hudson/WebAppMain.java core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java http://jenkins-ci.org/commit/jenkins/582128b9ac179a788d43c1478be8a5224dc19710 Log: [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.

dogfood added a comment - 2014-10-17 04:01

Integrated in jenkins_main_trunk #3762
[FIXED JENKINS-25019] (Revision 582128b9ac179a788d43c1478be8a5224dc19710)

Result = SUCCESS
kohsuke : 582128b9ac179a788d43c1478be8a5224dc19710
Files :

core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java
core/src/main/java/hudson/WebAppMain.java
changelog.html

dogfood added a comment - 2014-10-17 04:01 Integrated in jenkins_main_trunk #3762 [FIXED JENKINS-25019] (Revision 582128b9ac179a788d43c1478be8a5224dc19710) Result = SUCCESS kohsuke : 582128b9ac179a788d43c1478be8a5224dc19710 Files : core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java core/src/main/java/hudson/WebAppMain.java changelog.html

Arnaud Héritier added a comment - 2014-11-03 21:45

affects also the latest LTS 1.580.1

Arnaud Héritier added a comment - 2014-11-03 21:45 affects also the latest LTS 1.580.1

Arnaud Héritier added a comment - 2014-11-05 08:33

olivergondza could you include it in 1.580.2 please ?

Arnaud Héritier added a comment - 2014-11-05 08:33 olivergondza could you include it in 1.580.2 please ?

Daniel Beck added a comment - 2014-11-05 10:18

Fix may have caused ~~JENKINS-25337~~ so I'm not so sure about the LTS worthiness.

Daniel Beck added a comment - 2014-11-05 10:18 Fix may have caused JENKINS-25337 so I'm not so sure about the LTS worthiness.

Jesse Glick added a comment - 2014-12-18 21:03

Since this fix makes it possible to run on Tomcat, a common container, whereas ~~JENKINS-25337~~ affects a less common container and can be worked around by properly configuring it, I think this should still be an LTS candidate.

Jesse Glick added a comment - 2014-12-18 21:03 Since this fix makes it possible to run on Tomcat, a common container, whereas JENKINS-25337 affects a less common container and can be worked around by properly configuring it, I think this should still be an LTS candidate.

Jesse Glick added a comment - 2014-12-18 21:06

Never mind, just saw that when unfixed this is only a warning.

Jesse Glick added a comment - 2014-12-18 21:06 Never mind, just saw that when unfixed this is only a warning.

Jesse Glick added a comment - 2016-03-17 20:17

FYI when using maven-hpi-plugin 1.117+ this stack trace will be printed during mvn hpi:run for plugins using the 1.580.x line, since the fix seems to have gone into 1.586 and not been backported.

Jesse Glick added a comment - 2016-03-17 20:17 FYI when using maven-hpi-plugin 1.117+ this stack trace will be printed during mvn hpi:run for plugins using the 1.580.x line, since the fix seems to have gone into 1.586 and not been backported.

People

Assignee:: Unassigned

Reporter:: Tom Crossland

Votes:: 0 Vote for this issue

Watchers:: 8 Start watching this issue

Dates

Created:: 2014-10-07 10:48

Updated:: 2016-07-27 17:47

Resolved:: 2014-10-17 03:03