Improve CVE-2023-34035 detection

[dreis2211]

:zap: UPDATE :zap:: 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.

• [x] #13666
• [x] #13667
• [x] #13726
• [x] https://github.com/spring-projects/spring-security/issues/13849
• [ ] #13562
• [x] #13850

---

The original post follows:

Hi,

with the work on https://github.com/spring-projects/spring-security/issues/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: https://github.com/spring-projects/spring-security/blob/df239b6448ccf138b0c95b5575a88f33ac35cd9a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java#L322

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?

[dreis2211]

I've opened https://issues.redhat.com/browse/UNDERTOW-2295 to address (or at least discuss) the additional servlet being registered in Undertow and will shortly provide a PR over there.

I can only share my frustration at this point and would like to again emphasize that we're currently in a situation where very vanilla apps unaffected by the security issue need to do something. Probably slowing down the adoption rate of newer Spring-Boot versions and therefore fixes for other (security) issues. I'm a huge fan (& contributor) of the Spring ecosystem, this won't change any of my views about that. But let me be also clear here: this hasn't been the gold standard in communication and customer-care I've been used to and I do hope this is internally discussed in a retrospective to improve future issues like that.

[RRGT19]

I have a basic Spring Boot v3.1.1 gradle application with a few endpoints for a SPA web app. I use the Spring Web, Spring Security and OAuth2 Resource Server. I'm using the embedded Tomcat server. Tried to upgrade to 3.1.2 and I encountered this breaking change.

According to the suggestion, I have no idea how to check if I have unnecessary servlets configured out there. How to check that? how to disable one if this is the case? I think that more info about this would be useful to have.

[OlliL]

@RRGT19 - What embedded servlet container are you using? I assume Undertow? You could start with a breakpoint in AbstractRequestMatcherRegistry.java#L198 and inspect the content of registrations

By the way - I dug a bit deeper and checked other Servlet Containers:

• Standard embedded Apache Tomcat setups are not negatively affected by this change because only Springs DispatcherServlet gets registered.
• Standard embedded Jetty 11 setups (with jakarta servlet-api downgraded to 5.0.0) are also not negatively affected.
• No idea about a Spring Boot WAR deployment in whatever standalone Server - haven't tested that (and I wont....)

[RRGT19]

@OlliL I'm using the embedded Tomcat server.

Edited: The registrations variable has two results, dispatcherServlet and messageDispatcherServlet. This last one I guess it was auto-configured due to internal SOAP JAX-RPC calls. The docs says this is an alternative to the standard Spring-MVC DispatcherServlet.

I have the wsdl4j, apache.axis and axis-jaxrpc dependencies.

[OlliL]

@RRGT19 The error this issue is about only occurs starting with 6.1.2

[RRGT19]

@OlliL My mistake, I was looking at the class when I rollback to 3.1.1. I've edited my comment above.

[jzheaux]

Thanks for the additional info, @OlliL and @dreis2211.

I see no(?) way in preventing Undertow to register its default Servlet.

I was unclear. I am still researching Undertow itself, and it wasn't my intent to imply more than my suggested code workaround and that if unneeded servlets can be removed, then they should be.

I can only share my frustration at this point and would like to again emphasize that we're currently in a situation where very vanilla apps unaffected by the security issue need to do something

I understand and hear your frustration. @rwinch and I are working on this together to see what can be done to reduce the method's reactivity and also simplify any remaining mitigations. Your concerns are heard. The previous post of code and mitigation advice is for all those who find themselves on this issue and is not intended to dismiss or over-simplify the impact that you are experiencing.

[OlliL]

Thanks for continuing working on that issue and considering on how to improve the situation @jzheaux . Thats appreciated!

Please also let me share my opinion on that as well.

the CVE

Unfortunally the CVE does not state what exactly the problem is which needs to be adressed. It only explains that there is "a problem" and lists the circumstances under which this problem appears. All who where not involved in any internal CVE discussions have only the answer in AbstractRequestMatcherRegistry.java at hand - without knowing what it tries to answer exactly. Judging from how the code was in 6.1.1 and is now in 6.1.2 I can only assume that the logic which lead to register the String either as ant or as mvc pattern was not correct under all circumstances and needed to be fixed to be more precise in its decission of when to create an AntMatcher and when to create a MvcMatcher. And since there are situations where the code can't decide we are now left with the circumstance that the user has to decide by throwing that exception.

To be honest - I currently fail to see where the real issue was with 6.1.1 and how 6.1.2 tries to solve it. If 6.1.1 made wrong assumptions about creating an AntMatcher or a MvcMatcher I assume the endpoint might have left unsecured. But isn't that the same case now, when the user receives this exception and ends up making a wrong decission in what Matcher to use? At the end there are plenty of mistakes someone can make in configuring Endpoint-Matchers so I would assume anyone interested in its application security has at least tests in place making sure all endpoints which are expected to be secured are indeed secured.

Can you share more details on that CVE so its easier for others to see what problems had to be fixed and maybe leading to an improved solution?

the requestMatchers method

Ideally methods are deterministic. Especially if you provide some sort of a framework or library many others are intended to use. If they are not, you'll end up with a lot of issues being opened and questions people are asking just because they fail to understand what circumstances lead to the fact that the method is either doing what is intended to do - or not. So in my opinion one of the following must happen:

1. the method must be fixed to work in "all supported configurations" as expected, or
2. deprecate the method and go back to the situation where the user has to always decide if the decission magic can't cover "all supported configurations"
3. have it documented in which of the "supported configurations" this method must not be used (in a way a regular user understands if he is affected or not - "having multiple Servlets registered" does not fulfil this imho)

While solution 1 seems to be the best one and solution 3 is the worst one in regards of user experience.

So back to this issue: If requestMatchers(String... patterns) is there to determine if the user needs an AntMatcher or a MvcMatcher with the goal in mind to make specifying request patterns easier for the user, that logic must either work in the supported configurations or its time to withdraw that method. Its not wise to have a method which only works in "some configuration" as we'll see several issues being opened and plenty of stackoverflow questions just because this method can be used "sometimes" and "sometimes" not... This might finally lead to the situation where everyone proposes to better use requestMatchers(RequestMatcher... requestMatchers) making the user always decide anyway - just to get a deterministic behaviour which is independent of the used configuration.

[jzheaux]

@OlliL thanks for sharing more details about your position. I understand the position you are coming from, and there are many points that resonate.

Unfortunately, I may not be able to disclose (as CVEs go) to the degree that the community would like. That said, I think I can provide more detail than I have so far.

The issue stems from the fact that MvcRequestMatcher is primarily designed to work for endpoints that will be serviced by Spring MVC. This extends back to older CVEs and the addition of mvcMatchers. As you said, this left applications wondering whether they should pick antMatchers or mvcMatchers, and unfortunately antMatchers was often incorrectly selected.

The practical truth is that mvcMatchers should almost always be picked in a Spring MVC application (version 5 or earlier) since it is fit for purpose for requests that will get serviced by Spring MVC. Doing otherwise may leave that application open to the earlier CVE that inspired mvcMatchers.

Because of our observation that folks continued to select antMatchers for requests serviced by Spring MVC, one goal of 6.0 was to introduce behavior that would make the choice for them, which was the inspiration for requestMatchers.

The original logic for requestMatchers was roughly: "If Spring MVC is on the classpath, use an MVC matcher; otherwise, use an Ant matcher". This reflects quite well the very effective rule of thumb that the team has expressed over the years and, as already mentioned, is nearly always correct.

Where it is incorrect is when an application is creating authorization rules for traffic bound to more than just DispatcherServlet, say like the following:

http
    .authorizeHttpRequests((authorize) -> authorize
        .requestMatchers("/traffic-bound-for-spring-mvc").hasAuthority("mvc")
        .requestMatchers("/traffic-bound-for-some-other-servlet").hasAuthority("other")

In this case, Spring Security 6.1.1 and earlier is using MvcRequestMatcher without accounting for the other servlet's servlet path. This is incorrect.

The reason it is a CVE is that default usage of requestMatchers(String) can lead to formulating a request matcher instance that doesn't correctly match the intended requests.

There are plenty of API-based solutions. So far, they all roughly surround making it easier to clarify the servlet-path part of the URI so that the request matching is correctly computed.

The fact remains that this is an uncommon scenario with an outsized security consequence. The question in my mind is how to correctly communicate the possible need for mitigation.

What has at least been revealed to me from this ticket is that the heuristic for determining the need to mitigate is noisier than I originally thought. When it comes to mitigation advice, the signal needs to be strong or, as you alluded to, it leads people to either ignore it or avoid that part of the API altogether.

But isn't that the same case now, when the user receives this exception and ends up making a wrong decision in what Matcher to use?

In my mind, there is a balance to be struck here no matter what. Given that, one thing I'm open to is changing the assertion to a warning message since that appears to be a better expression of Spring Security's level of confidence that it is indeed an issue that needs addressing.

In the meantime, we can continue to look for APIs or other solutions that more clearly guide people to give Spring Security the information it needs to match the request correctly.

"all supported configurations" ... just because this method can be used "sometimes" and "sometimes" not

To a degree, this cannot be avoided. That issue was always there with antMatchers and mvcMatchers. In a way, now the codebase itself is trying to alert folks and invite adjustment, and I agree that it may be in too noisy of a way.

But, just because requestMatchers is fit for only the vast majority of circumstances does not mean that it should be deprecated. I believe that requestMatchers provides more security than asking the community to decide between antMatchers and mvcMatchers, for example. And, I remain open to other solutions that make the secure choice also the simplest choice.

[christianhujer]

Where it is incorrect is when an application is creating authorization rules for traffic bound to more than just DispatcherServlet

Would this also be incorrect configuration if there are exactly two DispatcherServlets - the regular one from Spring, and the additional one from org.springframework.boot:spring-boot-devtools?

Here's what's happened to me: I've updated, all tests passed, including this one:

class ApplicationTest {
    @Test
    fun mainRuns() {
        main("--server.port=0")
        assertNotNull(appContext)
        assertEquals(0, exit(appContext))
    }
}

We even deployed to production already. Then, a dev wanted to debug something locally, and reported this problem. It turned out that out of our four configurations dev, test, smoke, and prod, only dev was affected, probably because of org.springframework.boot:spring-boot-devtools pulling in a second DispatcherServlet.

When does somebody have more than one DispatcherServlet, other than from org.springframework.boot:spring-boot-devtools? How do I know, when using Spring, that I have more than one DispatcherServlet?

Could the solution detect whether there is only one more DispatcherServlet, and whether that DispatcherServlet is the one from org.springframework.boot:spring-boot-devtools, and play nicely in that case?

(I don't know if org.springframework.boot:spring-boot-devtools pulls in a second DispatcherServlet, it just looks likely from what I observe.)

[OlliL]

@christianhujer you can check the first sentence of my answer here. With that breakpoint you can see (at runtime....) what different Servlets you have.

https://github.com/spring-projects/spring-security/issues/13568#issuecomment-1650677792

[jaybon1]

The same issue occurs when using the H2 database console. (Spring boot 3.1.2)

Is there a way to resolve the issue without removing the H2 console servlet?

requestMatchers(String)

Thanks.

[jzheaux]

@jaybon1, if you have more than one servlet, it's likely that at least one of them is using a path-based servlet mapping; for example, /path/*. This is indeed the case when using H2 Console and Spring MVC. In that case, one servlet is mapped to /h2-console/*, and the other is mapped to /.

In that case, you need to specify that servlet path when formulating the authorization rule. Please see this sample and this commit log for details. (I will see about adding a sample that uses H2 Console for more targeted advice.)

[jzheaux]

@RRGT19, if you have more than one servlet (DispatcherServlet or otherwise), it's likely that at least one of them is using a path-based servlet mapping; for example, /path/*. In your case, I imagine that messageDispatcherServlet is mapped to /message/* and dispatcherServlet is mapped to /.

In that case, you need to specify that servlet path when formulating the authorization rule. Please see this sample and this commit log for details.

[jzheaux]

@christianhujer, if you have more than one servlet (even if it's two DispatcherServlets), it's likely that at least one of them is using a path-based servlet mapping; for example, /path/*.

In that case, you need to specify that servlet path when formulating corresponding authorization rules. Please see this sample and this commit log for details. (I will see about adding a sample that uses dev-tools for more targeted advice.)

[RRGT19]

@jzheaux Thanks for the explanation and the demo. The servletPath that is configured here should be the same value as server.servlet.context-path ? In my case I configure the OS variable SERVER_SERVLET_CONTEXT_PATH to /api.

Or it has nothing to do with this? sorry for the ignorance on the subject

[marbon87]

@jzheaux thanks a lot for your objective discussion of the problem and your work.

I am in the same situation as @dreis2211 by providing a company internal security library for our spring boot based applications (with tomcat). We most often have multiple DispatcherServlets in our applications, where the Spring Mvc DispatcherServlet is the main one, handling about 99,9% of the requests..

What do you think about providing the option to configure the default request matcher per SecurityFilterChain. In that way we can configure a separate SecurityFilterChain per DispatcherServlet and configure the appropiate request matcher for that servlet.

[budaestew]

Is there any progress on the issue with Undertow in security version 6.1.2?

As a workaround, I created an alternate DispatcherServletRegistrationBean that specifies Undertow's default name default instead of dispatcherServlet.

@Bean(name = DEFAULT_DISPATCHER_SERVLET_REGISTRATION_BEAN_NAME)
public DispatcherServletRegistrationBean dispatcherServletRegistration(DispatcherServlet dispatcherServlet,
        WebMvcProperties webMvcProperties, ObjectProvider<MultipartConfigElement> multipartConfig) {
    DispatcherServletRegistrationBean registration = new DispatcherServletRegistrationBean(dispatcherServlet,
            webMvcProperties.getServlet().getPath());
    registration.setName(io.undertow.servlet.handlers.ServletPathMatches.DEFAULT_SERVLET_NAME);
    registration.setLoadOnStartup(webMvcProperties.getServlet().getLoadOnStartup());
    multipartConfig.ifAvailable(registration::setMultipartConfig);
    return registration;
}

[jzheaux]

@marbon87 I think this is a good idea and one of a few that we are considering. I tend to prefer that folks specify the servlet at the authorization level instead to reduce repetition (separate filter chains require authentication mechanisms to be redeclared), but I believe the core idea is the same as what you are offering. I hope to have more to share soon.

[jzheaux]

@christianhujer I haven't been able to reproduce the issue by adding only the devtools dependency. WIll you please file a separate issue with a sample application, and I'll take a look?

[marbon87]

@marbon87 I think this is a good idea and one of a few that we are considering. I tend to prefer that folks specify the servlet at the authorization level instead to reduce repetition (separate filter chains require authentication mechanisms to be redeclared), but I believe the core idea is the same as what you are offering. I hope to have more to share soon.

Sounds got to me. I do not want to stress or claim anything but could you give any information about the time plan for a possible fix. The reason i am asking is that we have a scheduled release for our company internal framework at the beginning of september and do not want to edit hundrets of applications twice.

[MarkvanOsch]

Hi, I also encountered this issue and want to ask some advice for my usecase.

I have a project providing Rest api endpoints and consuming Web services soap clients, using these dependencies:

• spring-boot-starter-web
• spring-boot-starter-web-services

After upgrading to the latest spring-security 6.1.x I get this error:

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.springframework.web.servlet.DispatcherServlet=[/], org.springframework.ws.transport.http.MessageDispatcherServlet=[/services/*]}.

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

What is your advice in this case. How can I just disable this second DispatcherServlet, because not used? Or should I just go for:

requestMatchers(AntPathRequestMatcher)

[anantjagania]

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=[/]}.

[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 GitHubhttps://github.com/spring-projects/spring-security/issues/13568#issuecomment-1690393375, or unsubscribehttps://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.