Improve CVE-2023-34035 detection

[dreis2211]

⚡ UPDATE ⚡: A proposed solution is available in the latest 6.2 snapshot build. Please see this comment for details. I would love your feedback.

UPDATE: Thanks again, @dreis2211 for the report.

As reported, there are some false positives with the validation put in place in response to CVE-2023-34035. A fix was released in 5.8.6, 6.0.6, and 6.1.3 to improve the validation.

In all cases, a new repo is now available with several more examples across all affected maintenance releases of Spring Security. The readme indicates which samples are vulnerable and which aren't.

• requestMatchers should not count servlets without mappings #13666
• requestMatchers servlet validation error should include information about servlet paths #13667
• Add MvcRequestMatcher reference documentation #13726
• AbstractRequestMatcherRegistry#requestMatchers should pick MvcRequestMatcher when using MockMvc #13849
• Simplify MvcRequestMatcher construction #13562
• Resolve RequestMatcher at request-time #13850

---

The original post follows:

Hi,

with the work on #13551 a regression has been introduced that prevents the startup of applications under certain (unfortunately not very uncommon) circumstances.

There are multiple already mentioned in the issue, another one I'm facing is that Undertow always registers a default servlet even if there are no mappings leading to two registrations being returned here:

spring-security/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java

Line 322 in df239b6

Assert.isTrue(registrations.size() == 1,

For simpler reproduction, simply check out https://github.com/dreis2211/spring-security-13568 and try to startup the app.
It should yield a stacktrace showing this:

Caused by: java.lang.IllegalArgumentException: This method cannot decide whether these patterns are Spring MVC patterns or not. If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); otherwise, please use requestMatchers(AntPathRequestMatcher).
	at org.springframework.util.Assert.isTrue(Assert.java:122) ~[spring-core-6.0.11.jar:6.0.11]
	at org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.requestMatchers(AbstractRequestMatcherRegistry.java:204) ~[spring-security-config-6.1.2.jar:6.1.2]
	at org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.requestMatchers(AbstractRequestMatcherRegistry.java:248) ~[spring-security-config-6.1.2.jar:6.1.2]
	at com.example.springsecuritybugs.SecurityConfiguration.lambda$securityFilterChain$0(SecurityConfiguration.java:14) ~[main/:na]
	at org.springframework.security.config.annotation.web.builders.HttpSecurity.authorizeHttpRequests(HttpSecurity.java:1466) ~[spring-security-config-6.1.2.jar:6.1.2]

I think the assertion needs to be reverted for the time being and revisited at a later point. Or making it a warning log instead of the assertion, should this be really a case that you want to prevent in the future with a proper non-patch release.

Cheers,
Christoph

[dreis2211]

So after digging a bit deeper I have found a workaround

The following example:

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
	http.authorizeHttpRequests((requests) -> requests
		.requestMatchers("/test1").permitAll()
		.anyRequest().authenticated()
	);
	return http.build();
}

Would need to be turned into:

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http, HandlerMappingIntrospector introspector) throws Exception {
	MvcRequestMatcher.Builder mvcMatcherBuilder = new MvcRequestMatcher.Builder(introspector);
	http.authorizeHttpRequests((requests) -> requests
		.requestMatchers(mvcMatcherBuilder.pattern("/test1")).permitAll()
		.anyRequest().authenticated()
	);
	return http.build();
}

But again. This is a breaking change one doesn't expect in a patch release. Also, the MvcRequestMatcher.Builder facility is not really mentioned in the docs apart from using it, when you need to set a custom servletPath.

Should the assertion stick for whatever valid reason or turned into a log statement, the message is somewhat confusing because there are no methods inside AbstractRequestMatcherRegistry that really take either an MvcRequestMatcher or AntPathRequestMatcher, just the interface RequestMatcher. So it took me quite long to figure out - given the lack of docs - how to actually workaround the problem. Especially since building the different variants means using different things (AntPathRequestMatcher has a simple static method, MvcRequestMatcher providing the mentioned builder).

Having all of that said, I don't think using the MvcRequestMatcher.Builder is a cool API that should be the way to go here in the future. Spring Security configuration is already complex enough once your application becomes somewhat more sophisticated and turning a simple requestMatchers("/test1") into a variant where the user needs to build matchers via the builder, seems a bit bloated. I'd rather have the methods mvcMatchers() & antMatchers() back, which got deprecated and removed.

In summary: I think the assertion should just be removed and also not turned into a log message, warning people to migrate to something else. requestMatchers should simply do what it does and use mvc matchers in the end.

Cheers

[albertus82]

https://spring.io/security/cve-2023-34035 explains well what has changed and how to fix this error, however I agree that a patch release should not introduce breaking changes like this.

[dreis2211]

Oh, I've missed that announcement. Thanks for sharing....

But even then:

An application is not vulnerable if any of the following is true:
...
The application uses requestMatchers(String) only for Spring MVC endpoints

Which is the case in the reproducer app I shared. I wouldn't expect a breaking change for those applications.

[vitalyster]

So, if the app is only uses MVC we can safely wait for proper non-breaking change fix?

[samety22]

Hi,

what is the Satus of this issue? i faced the same Problem!

Must i realy use MvcRequestMatcher.Builder mvcMatcherBuilder = new MvcRequestMatcher.Builder(introspector); for the feature? Or will this Solved with the Next Version 3.1.3?

Edit: With 3.1.1 Its working without MVCRequestMatcher.Builder. The Problem appears only on 3.1.2

[albertus82]

Must i realy use MvcRequestMatcher.Builder mvcMatcherBuilder = new MvcRequestMatcher.Builder(introspector); for the feature? Or will this Solved with the Next Version 3.1.3?

You can use the constructor MvcRequestMatcher(introspector, pattern) in place of the builder; I found it more compact. Refer to https://spring.io/security/cve-2023-34035 for more examples.

[cwatzl]

[...] however I agree that a patch release should not introduce breaking changes like this.

To make matters worse, @WebMvcTest based tests don't even catch this. I have tests that actually import the SecurityFilterChain and validate some authorization rules, and they didn't fail. Being confident in my test coverage, I just went ahead and merged the minor update suggested by my Renovate Bot, only to have my coworkers and the deployment pipeline scream at me hours later, needlessly costing us several hours. It would be nice if at the very least the testing support libraries could catch this.

[sutra]

So after digging a bit deeper I have found a workaround

But this seems does not work if deploy as a war in Tomcat.

[sutra]

But this seems does not work if deploy as a war in Tomcat.

Resolved, I have another line:

-                       .csrf(csrf -> csrf.ignoringRequestMatchers("/**"))
+                       .csrf(csrf -> csrf.ignoringRequestMatchers(AntPathRequestMatcher.antMatcher("/**")))

[jzheaux]

Thanks, Everyone, for reaching out. We avoid breaking changes in maintenance releases; however, as happens with CVEs at times, it's necessary to introduce one. Thank you for your patience during this upgrade.

As has already been stated, there are examples in the CVE report for how to make the appropriate changes. I've also added a blog post to bring more visibility to the needed changes.

Or making it a warning log instead of the assertion

I do not believe this can be downgraded to a warning message, given that requestMatchers(String) cannot be securely used when there are multiple servlets in place. Further analysis may prove otherwise, and I'd be happy to introduce a migration path that is easier to consume.

[OlliL]

So with an out-of-the-box embedded Undertow setup which provides a "default" io.undertow.servlet.handlers.DefaultServlet as well as a "dispatcherServlet" org.springframework.web.servlet.DispatcherServlet (both part of a io.undertow.servlet.spec.ServletRegistrationImpl) requestMatchers(String) can't be used? No idea about Tomcat...
So in other words: When using Spring MVC, requestMatchers(String) can't be used?

[albertus82]

I think requestMatchers(String) should be deprecated because it's ambiguous and can fail at runtime; I see no reason to keep it.

[OlliL]

The default servlet in my case has no path mapping (mappings is an empty ArrayList) - so how does it affect the registered Spring MVC DispatcherServlet? I assume the Default Servlet will never be picked up anyway? So while there are two Servlets registered, the DefaultServlet won't be used anyway?
So I don't see why registrations is only checked for its size. I see no way in disabling the registration of Underthows DefaultServlet. io.undertow.servlet.handlers.ServletPathMatches.setupServletChains() always registers it.

[dreis2211]

I wonder if we could provide a better migration path with the following.

--- a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java
+++ b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java
@@ -39,6 +39,7 @@ import org.springframework.security.web.util.matcher.RegexRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.CollectionUtils;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.servlet.handler.HandlerMappingIntrospector;

@@ -201,7 +202,11 @@ public abstract class AbstractRequestMatcherRegistry<C> {
                if (!hasDispatcherServlet(registrations)) {
                        return requestMatchers(RequestMatchers.antMatchersAsArray(method, patterns));
                }
-               Assert.isTrue(registrations.size() == 1,
+               // Filter out "empty" servlets without any mappings
+               List<? extends ServletRegistration> servletRegistrations = registrations.values().stream()
+                               .filter(registration -> !CollectionUtils.isEmpty(registration.getMappings()))
+                               .toList();
+               Assert.isTrue(servletRegistrations.size() == 1,
                                "This method cannot decide whether these patterns are Spring MVC patterns or not. If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); otherwise, please use requestMatchers(AntPathRequestMatcher).");
                return requestMatchers(createMvcMatchers(method, patterns).toArray(new RequestMatcher[0]));
        }

In case you think this could be a (part of the) solution @jzheaux I'll provide a PR.

At least with this, some (certainly not all) applications unaffected by the security issue don't have a requirement on changing their code. I do share the sentiment that these applications should not require a change - anything other than the version number.

I also still have the opinion that the former mvcMatchers & antMatchers would be a better fit than the bloated builder or constructor options we have now. And I would fully agree with @albertus82 that the requestMatchers(String) seems not fit for purpose at all anymore and a deprecation would really make this more obvious.

In my particular case, I'm also not talking about applications under my (full) control, but me providing internal libraries that need to impose breaking changes to our internal customers now as well. For something that they are unaffected by, really.

[jzheaux]

@dreis2211, @albertus82, thanks for the ideas and perspective.

Based on your feedback, I've updated the Mitigation section in the security advisory to suggest that individuals remove unneeded servlets from their application runtime.

We don't want to deprecate requestMatchers(String) as it works quite well in situations like Boot applications that aren't deployed to a separate servlet container. Also, I imagine that most applications do not need the extra servlets that an application server is providing and would do well to remove the unneeded configuration.

I don't think that the method can be smarter than it is at this point since servlet containers configure and expose default servlets differently (some do have mappings, for example). Also, some containers add other servlets by default like JspServlet, and allow overriding of each. Verifying intent and behavior becomes tricky and error-prone quickly.

Where I lean is in providing a simpler way to recover when this situation comes up.

I do share the sentiment that these applications should not require a change

I can understand the position of frustration that you are coming from. Many folks just did a major version upgrade and that can be tricky on its own. That said, when it's revealed that a default usage of Spring Security is insecure, it's important to respond quickly and clearly so that we can all move to a more secure place.

Given that, the happy medium is to make it easier to recover. The current recovery options are not ideal, so let's work on that.

I also still have the opinion that the former mvcMatchers & antMatchers would be a better fit

This could be, though I think this more generally speaks to a desire to simplify the mitigation. IOW, so long as it is easy to do the secure thing, it shouldn't matter so much when a developer finds out the thing they first did was insecure.

One approach that I am liking right now is to introduce a builder that simplifies creating the right request matcher.

I'll work on vetting the pros and cons of all the ideas shared so far. In the meantime, folks are welcome to use the following builder:

@Component
public class RequestMatcherBuilder {
    private final HandlerMappingIntrospector introspector;
    private final String servletPath;

    @Autowired
    public RequestMatcherBuilder(HandlerMappingIntrospector introspector) {
        this(introspector, null);
    }

    private RequestMatcherBuilder(HandlerMappingIntrospector introspector, String servletPath) {
        this.introspector = introspector;
        this.servletPath = servletPath;
    }

    public MvcRequestMatcher[] matchers(String... patterns) {
        MvcRequestMatcher[] matchers = new MvcRequestMatcher[patterns.length];
        for (int index = 0; index < patterns.length; index++) {
            matchers[index] = new MvcRequestMatcher(this.introspector, patterns[index]);
            if (this.servletPath != null) {
                matchers[index].setServletPath(this.servletPath);
            }
        }
        return matchers;
    }

    public RequestMatcherBuilder servletPath(String path) {
        return new RequestMatcherBuilder(this.introspector, path);
    }
}

Then an application can do:

@Bean 
SecurityFilterChain appSecurity(HttpSecurity http, RequestMatcherBuilder mvc) throws Exception {
    http
        .authorizeHttpRequests((authorize) -> authorize
            .requestMatchers(mvc.servletPath("/graphql").matchers("/**")).hasAuthority("graphql"))
            .requestMatchers(mvc.matchers("/flights/**", "/user/**")).hasRole("USER")
            .anyRequest().authenticated()
        )
        .csrf((csrf) -> csrf
            .ignoringRequestMatchers(mvc.matchers("/to", "/be", "/ignored"))
        )
        // ...
}

The majority of folks will only need the matchers method, though the servletPath method is for those who are routing traffic to additional servlets.

Again, I understand that breaking changes are rare for a maintenance release and that this is unexpected and painful. Thank you for your efforts to make this an easier transition.

[OlliL]

Based on your feedback, I've updated the Mitigation section in the security advisory to suggest that individuals remove unneeded servlets from their application runtime.

I see no(?) way in preventing Undertow to register its default Servlet.
Please check https://github.com/undertow-io/undertow/blob/7d87eef9534807d00c974f92dc46be3d09b703a0/servlet/src/main/java/io/undertow/servlet/handlers/ServletPathMatches.java#L311

So how you suggest to implement In many cases, such a servlet can be removed from your container's global configuration.?

We don't want to deprecate requestMatchers(String) as it works quite well in situations like Boot applications that aren't deployed to a separate servlet container. Also, I imagine that most applications do not need the extra servlets that an application server is providing and would do well to remove the unneeded configuration.

I'd call my Boot application a kinda standard one with no fancy configuration using spring-boot-starter-undertow - and requestMatchers(String) can't be used anymore. Can you please give a configuration example where it still works?

[CyberRookie]

You can set the servlet paths in your application.properties file to point to WEB-INF and META-INF.

…

________________________________ From: Anant Jagania ***@***.***> Sent: Wednesday, August 23, 2023 1:56 PM To: spring-projects/spring-security ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [spring-projects/spring-security] Improve CVE-2023-34035 detection (Issue #13568) I am using Spring Boot 3.1.2 and Spring Security 6.1.3 and still have the same issue. Spring registers two servlets as shown below. The Dispatcher servlet is mapped to "/". If I disable jsp servlet then my JSP's won't be shown and it says invalid path due to "Path with WEB-INF or META-INF: " Please advise on any workaround. There is no second servlet mapped to any path. Caused by: java.lang.IllegalArgumentException: This method cannot decide whether these patterns are Spring MVC patterns or not. If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); otherwise, please use requestMatchers(AntPathRequestMatcher). This is because there is more than one mappable servlet in your servlet context: {org.apache.jasper.servlet.JspServlet=[*.jspx, *.jsp], org.springframework.web.servlet.DispatcherServlet=[/]}. — Reply to this email directly, view it on GitHub<#13568 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/APLVTXMZLW5FHP7KGU262E3XWY743ANCNFSM6AAAAAA2RUK5AQ>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[anantjagania]

Thanks for the reply. My understanding is setting up view resolver to WEB-INF/jsp and suffix .jsp should be enough for spring to resolve jsps.

I do not see any other properties to set servlet paths to WEB-INF. The servlet path is already set to "/". Spring actually looks for jsp resources inside META-INF/resources so I tried to move my jsps inside META-INF/resources from META-INF/resources/WEB-INF . That passed the check for isInvaidPath but now it downloads the jsp on the browser rather than showing it. I do have all the necessary tomcat libraries in Classpath.

[VannHungg]

how to add HttpMethod.GET or POST into .requestMatchers(mvcMatcherBuilder.pattern("/")).permitAll() like this way

[Ariel15682]

how to add HttpMethod.GET or POST into .requestMatchers(mvcMatcherBuilder.pattern("/")).permitAll() like this way

Did you find the solution?
I think , for Spring Security 6, solved it like this;

MVC Controller:
requestMatchers(mvcMatcherBuilder.pattern("/api/users/**")).permitAll()

Otherwise:
.requestMatchers(AntPathRequestMatcher.antMatcher(HttpMethod.GET,"/cars")).permitAll()

Complete example method:

public SecurityFilterChain filterChain(HttpSecurity http, HandlerMappingIntrospector introspector) throws Exception {

         MvcRequestMatcher.Builder mvcMatcherBuilder = new MvcRequestMatcher.Builder(introspector).servletPath("/");
        
         http.csrf((csrf) -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()))
               
             .authorizeHttpRequests(auth ->
                 auth
                     .requestMatchers(mvcMatcherBuilder.pattern("/api/users/**")).permitAll() //MVC Controller
                     .requestMatchers(AntPathRequestMatcher.antMatcher(h2ConsolePath + "/**")).permitAll()
                     .requestMatchers(AntPathRequestMatcher.antMatcher(HttpMethod.GET,"/cars")).permitAll() //REST
                     .anyRequest().authenticated())

             .formLogin(Customizer.withDefaults())
                
             .httpBasic(Customizer.withDefaults())

             .headers(headers -> headers.frameOptions(HeadersConfigurer.FrameOptionsConfig::sameOrigin));

         return http.build();
}

I hope you understand and have been helpful

[plantexchen]

A lot of comments, but I can not find a solution to my problem. I have this error when I deploy my WAR on JBoss. What can I do about it?

This method cannot decide whether these patterns are Spring MVC patterns or not. If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); otherwise, please use requestMatchers(AntPathRequestMatcher).

This is because there is more than one mappable servlet in your servlet context: {org.apache.jasper.servlet.JspServlet=[*.jsp, .jspx], javax.faces.webapp.FacesServlet=[/faces/, *.jsf, *.faces, *.xhtml], org.springframework.web.servlet.DispatcherServlet=[/]}.

For each MvcRequestMatcher, call MvcRequestMatcher#setServletPath to indicate the servlet path.

I don't know what this JSP/JSF is, we don't use it specifically. Maybe we use it under the hood, but it runs perfectly on the embedded Tomcat and only fails on JBoss. We use the default servlet path.

[dari220]

@Ariel15682
Have You tested a spring app having jasper container dependency, or implements jsp files?
For me, wrapping ant- or mvcMatchers in requestMatchers did not work.
Migrated spring boot 2.7.13 -> 3.1.3 in development. Currently all endpoints are public.
.authorizeHttpRequests(authorize ->authorize.anyRequest().permitAll()

[Ariel15682]

@dari220
Unfortunately I did not test any application that contains jasper as dependency. I'm sorry, I can't help you

[dari220]

Debugging results in Version 3.1.3 (with embedded Tomcat) when using AntPathRequestMatcher

The folder containig jsp files e.g. '/WEB-INF/jsp/**' must be added to unsecured paths.

Internal RequestDispatching is routed through security filter chain. Is this new feature?

[DenisKorolev]

@Ariel15682, thank you for your solution. I had issue with H2 and it helped me.

Caused by: java.lang.IllegalArgumentException: This method cannot decide whether these patterns are Spring MVC patterns or not. If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); otherwise, please use requestMatchers(AntPathRequestMatcher).

This is because there is more than one mappable servlet in your servlet context: {org.h2.server.web.JakartaWebServlet=[/h2-console/*], org.springframework.web.servlet.DispatcherServlet=[/]}.

But there is no need in .servletPath("/") from your example. If that part is present, then my path wasn't parsed properly and the rule .permitAll() was ignored and .anyRequest().authenticated() was applied instead. It took me hours to find it out.
Here's the code that worked in my situation:

@Configuration
public class SecurityConfig {

    // My enpdoints start from /v1 so this pattern is ok for me
    private static final String API_URL_PATTERN = "/v1/**";

    @Bean
    public SecurityFilterChain getSecurityFilterChain(HttpSecurity http,
                                                      HandlerMappingIntrospector introspector) throws Exception {
        MvcRequestMatcher.Builder mvcMatcherBuilder = new MvcRequestMatcher.Builder(introspector);

        http.csrf(csrfConfigurer ->
                csrfConfigurer.ignoringRequestMatchers(mvcMatcherBuilder.pattern(API_URL_PATTERN),
                        PathRequest.toH2Console()));

        http.headers(headersConfigurer ->
                headersConfigurer.frameOptions(HeadersConfigurer.FrameOptionsConfig::sameOrigin));

        http.authorizeHttpRequests(auth ->
                auth
                        .requestMatchers(mvcMatcherBuilder.pattern(API_URL_PATTERN)).permitAll()
                        //This line is optional in .authenticated() case as .anyRequest().authenticated()
                        //would be applied for H2 path anyway
                        .requestMatchers(PathRequest.toH2Console()).authenticated()
                        .anyRequest().authenticated()
        );

        http.formLogin(Customizer.withDefaults());
        http.httpBasic(Customizer.withDefaults());

        return http.build();
    }
}

Update 1: changed code for H2 console to work properly in browser and implemented @jzheaux recommendation.

[jzheaux]

@plantexchen I'm sorry that you are having trouble. You mentioned that you tested things with JBoss and embedded Tomcat. Can you describe in what circumstances you are using each (e.g. embedded for development, JBoss for production)?

[jzheaux]

@MarkvanOsch I'm not sure I can comment on whether you need the second DispatcherServlet. If you indeed do not, then yes, you are welcome to remove it and the error should be resolved.

However, if you do need both, then please see this sample and specifically this commit log for an example of how to specify request matchers correctly.

How can I just disable this second DispatcherServlet

Without more understanding of your situation, I'm a little surprised to hear that you have this dependency but don't need the dispatcher servlet, as it's a core part of the web service module. But not being the expert in that case, I'd refer you to the web services team.

[dari220]

@jzheaux

In WebMvcConfigurer

` @OverRide
public void configureViewResolvers(ViewResolverRegistry registry) {

	InternalResourceViewResolver viewResolver = new InternalResourceViewResolver();
	viewResolver.setViewClass(JstlView.class);
	viewResolver.setPrefix("/WEB-INF/jsp/");
	viewResolver.setSuffix(".jsp");
	registry.viewResolver(viewResolver);
}`

The class 'InternalResourceViewResolver' states I'm INTERNAL and I'm VIEW Resolver.
So, when servletpath is /WEB-INF/jsp/main.jsp, (main.jsp is the name of the jsp page) it's not necessary to expose "/WEB-INF/jsp" folder publicly.
Currently, in my app this servletpath is rejected by AuthenticationEntryPoint Handler.

Complete Process:
Controller with servletpath "/home" should return view main.jsp to the client.

1. Client fires Uri '/home'
2. Spring Constroller forwards to resource with path "/WEB-INF/jsp/main.jsp"
3. AuthenticationEntryPoint states, its a failure.

How would You solve this?

[jzheaux]

Hi, @dari220, I'm sorry that you are having trouble. I'd prefer to keep the focus in this thread on how to adapt the requestMatchers call. Please post your question to StackOverflow with the spring-mvc tag for help. Afterward, please feel free to update your comment here with a link to the StackOverflow post, to increase the visibility of your question.

[jzheaux]

@DenisKorolev, @Ariel15682, and others using H2 Console, you may be able to simplify things by using Spring Boot's H2 Console RequestMatcher like so:

http
    .authorizeHttpRequests((authorize) -> authorize
        .requestMatchers(PathRequest.toH2Console()).permitAll()
        // ...
    )
// ...

[dari220]

Hi, @dari220, I'm sorry that you are having trouble. I'd prefer to keep the focus in this thread on how to adapt the requestMatchers call. Please post your question to StackOverflow with the spring-mvc tag for help. Afterward, please feel free to update your comment here with a link to the StackOverflow post, to increase the visibility of your question.

Hi , @jzheaux, thank You for the hint -> 'thats not the subject of requestMatchers'. I was sure it is.
I had to check the docu and found additional switch to make Security Filter Chain working.
All Dispatches Are Authorized

Add this line to your security filter chain.
.dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.INCLUDE).permitAll()
Though, I can't guarantee this covers everything.
Hope this helps to others.

[Pax90]

Hi @MarkvanOsch, In our application we also use spring-boot-starter-web for our rest interface and spring-boot-starter-web-services exclusively to consume a soap web service.

According to github, the web services starter contains two auto-configurations:

1. WebServicesAutoConfiguration.java - server functionality
2. WebServiceTemplateAutoConfiguration.java - client functionality

We only need the latter autoconfiuration for our client operations. That's why I added the following annotation:

@EnableAutoConfiguration(exclude=WebServicesAutoConfiguration.class)

Maybe this is the right way for your application too.

[MarkvanOsch]

@Pax90 Hi David, thanks! I can confirm that this solution works for our project with the same setup as yours. 👌

[jzheaux]

Given that #13850 and #13562 were just merged, I'd like to post an update here and a request for folks affected to try out the latest snapshot.

In summary, the following changes were made:

1. The validation checks were further refined to derive an appropriately configured MvcRequestMatcher or AntPathRequestMatcher based on the ServletContext. In the current snapshot, only folks who have a non-root DispatcherServlet will be affected
2. The mitigation has been simplified by introducing a new DSL call in authorizeHttpRequests to clarify the servlet for which each request matcher is configured. It works like this:

If you have DispatcherServlet mapped to /mvc/*, the default servlet provided by your container, and a GraphQL servlet mapped to /graphql, then you may have a configuration like this:

.authorizeHttpRequests((authorize) -> authorize
    .requestMatchers("/my/controller/**").hasRole("USER")
    .requestMatchers("/admin/controller/**").hasRole("ADMIN")
    .requestMatchers("/graphql").authenticated()
)

Because DispatcherServlet is mapped to something other than root, some of the above URIs are relative (/my/controller, /admin/controller) and some are not (/graphql), but Spring Security cannot tell which. Because of that, Spring Security will throw an exception at startup.

In the latest 6.2 snapshot, you can now do this:

.authorizeHttpRequests((authorize) -> authorize
    .forServletPattern("/mvc/*", (mvc) -> mvc
        .requestMatchers("/my/controller/**").hasRole("USER")
        .requestMatchers("/admin/controller/**").hasRole("ADMIN")
    )
    .forServletPattern("/graphql", (graphql) -> graphql
        .anyRequest().authenticated()
    )
)

This gives Spring Security enough information now to ensure that the correct request matcher instance is created since it now also has the missing /mvc prefix to the Spring MVC URIs.

Note that while I recommend always specifying the servlet path in this way if DispatcherServlet is not root, in the event that DispatcherServlet is the only servlet (this is how spring-boot-starter-web functions OOTB), Spring Security can still make the appropriate inference since that's the only servlet that it could be.

Please see more details in the SNAPSHOT reference.

I'm currently looking into what aspects of #13850 and #13562 can be safely backported to 5.8, 6.0, and 6.1 and will report that progress on those tickets.

[zartc]

Hi all,

To limit the usage of verbatim strings in code and to prevent code vs application.yml configuration mismatch, we could benefit from an injectable bean providing the spring-boot well-known paths (root, h2-console, templates, static) as they are currently configured in the spring context.

As an example, when I first tried the new solution proposed above by @jzheaux I received an error: "The given pattern /h2-console/** doesn't seem to match any configured servlets" because I had typed:
.forServletPattern("/h2-console/**", h2 -> h2 ...) instead of
.forServletPattern("/h2-console/*", h2 -> h2 ...)

I would have loved to have something like the SpringPaths class to inject in my filterChain configuration method:

public SecurityFilterChain filterChain(HttpSecurity http, SpringPaths paths) throws Exception {
    http.authorizeHttpRequests((authorize) -> authorize
            .forServletPattern(paths.mvc(), mvc -> mvc ... )
            .forServletPattern(paths.h2Console(), h2 -> h2 ... )
            .forServletPattern(paths.statics(), static -> static ... )
    );

regards.

[zoaked]

Hi everyone,

Thank you all for the discussion in this issue and explaining the situation. There are a couple of things I would like to provide feedback on:

So far the discussion has been around using relative paths with multiple servlets when none of them are set as the root. Would it make sense to have an additional check added to org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.requestMatchers(HttpMethod, String...) that allows it to handle when absolute paths are used? When all the information needed is being provided it seems like there wouldn't be a vulnerability as long as the start of every pattern matches up to one of the configured servlet's mappings. Using the example @jzheaux provided earlier the config would look like:

http.authorizeHttpRequests((authorize) -> authorize
    .requestMatchers("/mvc/my/controller/**").hasRole("USER")
    .requestMatchers("/mvc/admin/controller/**").hasRole("ADMIN")
    .requestMatchers("/graphql").authenticated()
)

Secondly, I noticed that the forServletPattern() method was added to AuthorizationManagerRequestMatcherRegistry, but not to other areas of the chain such as CsrfConfigurer or IgnoredRequestConfigurer. Are there plans to expand the pattern out or do those places need to use request matcher builders?

Lastly, we use separate DispatcherServlets to host static content (/app/* and /static/*) and found the org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.computeErrorMessage(Collection<? extends ServletRegistration>) error message to be a bit confusing. When the loop is formatting the exception message it uses a map to do so, but because there are multiple instances of the same servlet class it ends up clobbering the previous entry on each iteration of the loop. In the end the error is saying there are multiple servlets, but only lists the mappings for whichever one happened to come last. Could it use a list so they all get shown or combine the mapping collections? ... This is because there is more than one mappable servlet in your servlet context: {org.springframework.web.servlet.DispatcherServlet=[/app/*]}.

[jzheaux]

that allows it to handle when absolute paths are used?

Thanks, @zoaked, for this observation. The concern is that there is no way for Spring Security to know which you are doing (since both are just strings) and thus there is no way to use that information to allow or reject the configuration.

At the very least, a new method with a new method parameter is needed, e.g.

.servletPatternMatchers("/mvc", "/my/controller/**").hasRole("USER")
.servletPatternMatchers("/mvc", "/admin/controller/**").hasRole("ADMIN")
.servletPatterntMatchers("/graphql").authenticated()

so that the servlet path value is clear. And an early version of the DSL changes did precisely this.

The reason for moving it into a separate DSL altogether is it makes it clear that what you are doing is configuring endpoints by-servlet, not just providing additional request matching detail.

I would have loved to have something like the SpringPaths class to inject in my filterChain configuration method:

public SecurityFilterChain filterChain(HttpSecurity http, SpringPaths paths) throws Exception {
    http.authorizeHttpRequests((authorize) -> authorize
            .forServletPattern(paths.mvc(), mvc -> mvc ... )
            .forServletPattern(paths.h2Console(), h2 -> h2 ... )
            .forServletPattern(paths.statics(), static -> static ... )
    );

Thanks, @zartc, I think this is a good idea; as you said, the precise servlet pattern is not always apparent since most servlets are auto-configured.

[jzheaux]

Based on some feedback from #13794, we've decided to hold off on this new DSL for just a bit longer.

All the enhanced validation is still scheduled for the next point releases, including 6.2.0. As a reminder, the validation enhancements (#13666, #13667, and #13850) are made so that Spring Security only errors when both of these are true:

1. DispatcherServlet is not root, and
2. There are other servlets

[1057105012]

i think not a good idea, always error in war

[jzheaux]

@1057105012 can you please provide more detail, hopefully in the form of a minimal GitHub project, about your arrangement? I don't believe it's correct that it always errors in a war setup, so I'll need more information to see what will help your situation best.