Spring Security Provide a way to customize StrictHttpFirewall

Summary

Hi. In our project we recently migrated to Spring boot 2. in this version i figured out that '//' is forbidden in urls and this was a very big problem to us because there are more than thousands of jsp files that have used this incorrect behavior. i tried to config StrictHttpFirewall but it does not support it. in my experience with spring projects, i always saw that everything in it is super customizable so i tried to extend StrictHttpFirewall and implement my own behavior to it but what i faced was that most of StrictHttpFirewall method were private static and i had no access to almost anything. StrictHttpFirewall is a zero or one class. you should pick all or nothing. i'm young in developing world so if i'm wrong please correct me.

Actual Behavior

StrictHttpFirewall is almost immune to customizing. you cant add or remove behavior to it (for example the isNormalize method )

Expected Behavior

if its possible please make StrictHttpFirewall customizable ( like making private methods protected ) so who ever needs, to be able to add or remove specific behavior to this class (by extending for example) without losing the all the functionality that this class proviedes.

Configuration

Version

5.1.2

Sample

Comment From: clevertension

as a so powerful framework, we can assume that the customization is so easy, so i will give you some simple code analysis and the code demonstration

in org.springframework.security.config.annotation.web.builders.WebSecurity , the setApplicationContext(ApplicationContext applicationContext) will get the HttpFirewall from the application context, so you can register this bean by your own

so let me show you the simple code

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.security.web.firewall.FirewalledRequest;
import org.springframework.security.web.firewall.HttpFirewall;
import org.springframework.security.web.firewall.RequestRejectedException;
import org.springframework.stereotype.Component;

@Component
public class MyHttpFirewall implements HttpFirewall {

    @Override
    public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
        return new FirewalledRequest(request) {
            @Override
            public void reset() {
            }
        };
    }

    @Override
    public HttpServletResponse getFirewalledResponse(HttpServletResponse response) {
        return response;
    }

}

Good luck and close this issue please

Comment From: rwinch

You can also expose the DefaultHttpFirewall as a Bean which will use the old behavior. This is more secure than the no-op override above. However, this is trying to sanitize the request which is not as secure as StrictHttpFirewall

Comment From: miladamery

thank you for answers but this wasn't what i've been looking for. suppose that i want this StrictHttpFirewall features completely except that i want its normalize method do one more thing for me for example blocking one more character (it can be anything). what i've been experiencing till now for this kind of behavior was : "ok go extend that class, and implement the required method" but for this one the behavior is "ok go implement that whole thing from the start". at the moment if i want that one more character blocking i think like "shall i ditch method restriction and normalizing feature of StrictHttpFirewall so i can add one more restriction to requests?". i want to keep features that already exist plus one more thing. in the way you described i have to go copy StrictHttpFirewall code into my own class then modify the part i want so i can pick both StrictHttpFirewall features plus what i wanted to add. i thought this is not a good idea, but having StrictHttpFirewall functional methods private static leaves no other choice.

Comment From: rwinch

Thanks for the additional information. I think it is worth making the following public methods available:

• Set<String> getEncodedUrlBlacklist() which allows for adding / removing entries from the existing blacklist
• Set<String> getDecodedUrlBlacklist() which allows for adding / removing entries from the existing blacklist

Would this solve your problem?

Comment From: miladamery

Hi. i'm so sorry for being this late. i was busy a lot at work and i didn't expect an answer to the closed issue. Yes i think that would be enough.

Comment From: clevertension

@rwinch are you working to fix this issue? if not, i can give a hand

Comment From: rwinch

@clevertension Thanks for the offer. Yes please submit a PR!

Comment From: ief2009

'//' is forbidden for hard code in isNormalized(),so expose the blacklist available may not enough.

my advice is:

1. remove '//' check from isNormalized() method

2. add a new field forbidden_double_forward_slash

private static final List<String> FORBIDDEN_DOUBLE_FORWARDSLASH = Collections.unmodifiableList(Arrays.asList("//", "%2f%2f", "%2f%2F", "%2F%2f", "%2F%2F"));

1. provide a public method for customization

setAllowUrlEncodedDoubleSlash(boolean allowUrlEncodedDoubleSlash)

1. the exposure of blacklist is still available for expert user

is it ok? @rwinch

Comment From: clevertension

@ief2009 good catch, i check the source code carefully and find that double slash is actually hard coded in the isNormalized method, so the method exposure can not solve this issue

@rwinch i think this PR is good, it can both solve this double slash issue and give a more flexible way to process the blacklist, please check it