spring-projects / spring-projects/spring-security

Unpredictable context files loading order causes <sec:http element to crash context startup

Open
#4,141 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

Summary

We maintain a modular application that consists (from the Spring configuration perspective) of multiple context files (xml) supplied by different modules, inside each module's jar.
We also have a global platform-security-context.xml file that contains the basic settings for application security context, e.g. the standard authentication handler, a few paths that have to be protected/unprotected, etc. Example is given in the configuration section

We also employ a naming convention [module-name]-{data|web|security|integration|test}-context.xml for module-related context files, and they are all deployed to each jar's META-INF/context/ so Spring can load all the contexts via web.xml using classpath*:META-INF/context/*-context.xml syntax.

The problem occurs when a module wants to define its custom paths (e.g. configure custom authentication under /rest/....). In the near future, we are going to require new modules to add their XML security configuration to the entire platform as our application are going to communicate more with external systems.

Spring Security requires that the patternless <http> element, which is the default, occurs as last in the sequence. With multiple context files, if different context files specify <http> elements, all depends on the order the files are loaded.

Basing on our experience, and this is the reason why we are opening a ticket, the order the context files are loaded is not always deterministic on different application servers/OSes. In most cases, alphabetically ordering context files grants them to be loaded in their natural order, but there have been random-unpredictable cases in which the deployed application didn't follow the file name order. And even if it was 100% alphabetically-deterministic, I think that it is quite ugly having to consider the loading order of the files when dealing with Spring context files, all elements under the same <beans> element should be treated equal :-)

The request is to review the mechanism Spring Security uses to load XML definition of security elements and if possible to pre-order them putting the patternless <http> element at the end of the list.

Actual Behavior

When the context starts, I get the following error

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.filterChainProxy': Invocation of init method failed; nested exception is java.lang.IllegalArgumentException: A universal match pattern ('/**') is defined  before other patterns in the filter chain, causing them to be ignored. Please check the ordering in your <security:http> namespace or FilterChainProxy bean configuration
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1583)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:545)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:751)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:861)
	at org.springframework.context.support.AbstractApplicationContext.__refresh(AbstractApplicationContext.java:541)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java)
	at org.springframework.web.context.ContextLoader.configureAndRefreshWebApplicationContext(ContextLoader.java:444)
	at org.springframework.web.context.ContextLoader.initWebApplicationContext(ContextLoader.java:326)
	at org.springframework.web.context.ContextLoaderListener.contextInitialized(ContextLoaderListener.java:107)
	at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:5099)
	at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5615)
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:147)
	at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1571)
	at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1561)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	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.IllegalArgumentException: A universal match pattern ('/**') is defined  before other patterns in the filter chain, causing them to be ignored. Please check the ordering in your <security:http> namespace or FilterChainProxy bean configuration
	at org.springframework.security.config.http.DefaultFilterChainValidator.checkPathOrder(DefaultFilterChainValidator.java:68)
	at org.springframework.security.config.http.DefaultFilterChainValidator.validate(DefaultFilterChainValidator.java:55)
	at org.springframework.security.web.FilterChainProxy.afterPropertiesSet(FilterChainProxy.java:168)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1642)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1579)
	... 22 more

Expected Behavior

We want Spring security to load security context configuration from multiple files and merge them in a coherent way. In our example /rest/jira/webhook/** should disable all security (authentication and CSRF check)

Configuration
  • platform-security-context.xml

      <headers>
          <xss-protection />
          <hsts />
          <frame-options policy="SAMEORIGIN" />
      </headers>
    
      <access-denied-handler ref="authenticationHandler" />
      <intercept-url pattern="/login*" access="permitAll" />
      <intercept-url pattern="/secure/admin/**" access="isAuthenticated()" />
      <intercept-url pattern="/secure/**" access="isAuthenticated()" />
      <intercept-url pattern="/logout" access="permitAll" />
    
      <http-basic />
      <form-login login-page="/login" default-target-url="/secure" authentication-failure-url="/login?login_error=1" always-use-default-target="false"
          authentication-success-handler-ref="authenticationHandler" authentication-failure-handler-ref="authenticationHandler" />
    
      <logout invalidate-session="true" logout-url="/logout" success-handler-ref="logoutSuccessHandler" />
    
      <custom-filter position="PRE_AUTH_FILTER" ref="preauthFilter" />
      <!-- enable csrf protection -->
      <csrf disabled="false" />
      <session-management invalid-session-url="/login" session-fixation-protection="migrateSession">
          <concurrency-control expired-url="/login" />
      </session-management>
    
  • yankee-security-contest (does NOT work)

     <http security="none" pattern="/rest/jira/webhook/**" />
    
  • renaming the above to charlie-security-context (usually WORKS, but at some random times it did not)

    <http security="none" pattern="/rest/jira/webhook/**" />
    

As you can see, charlie is alphabetically before platform, but yankee comes after

Version

4.2.0.RELEASE

Sample

https://github.com/OpenCST/spring-framework-issues/tree/master/SEC-4141

In the above project you can find both a module that comes earlier than the global security context and another that comes later. Run with

mvn clean package cargo:run -Ptomcat8

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the startup failure with the SEC-4141 sample project using mvn clean package cargo:run -Ptomcat8. Start with DefaultFilterChainValidator.checkPathOrder in the stack trace and the sample's platform-security-context.xml and module context files. Done means multiple XML security contexts load coherently, with the patternless configuration last so the application starts and the webhook path remains unsecured.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.