Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iPhone Xʀ requests blocked by StrictHttpFirewall #9037

Closed
qeepcologne opened this issue Sep 21, 2020 · 9 comments
Closed

iPhone Xʀ requests blocked by StrictHttpFirewall #9037

qeepcologne opened this issue Sep 21, 2020 · 9 comments
Assignees
Labels
status: feedback-provided Feedback has been provided type: bug A general bug

Comments

@qeepcologne
Copy link

qeepcologne commented Sep 21, 2020

Describe the bug
iphone XR cannot connect via websocket, because useragent contains special characters:
iOS 13.7.0 Alamofire/5.2.2 iPhone Xʀ

   org.springframework.security.web.firewall.RequestRejectedException: The request was rejected because the header value "iOS 14.0.0 Alamofire/5.2.2 iPhone XÊ" is not allowed.
            at org.springframework.security.web.firewall.StrictHttpFirewall$StrictFirewalledRequest.validateAllowedHeaderValue(StrictHttpFirewall.java:739)
            at org.springframework.security.web.firewall.StrictHttpFirewall$StrictFirewalledRequest.access$000(StrictHttpFirewall.java:605)
            at org.springframework.security.web.firewall.StrictHttpFirewall$StrictFirewalledRequest$1.nextElement(StrictHttpFirewall.java:647)
            at org.springframework.security.web.firewall.StrictHttpFirewall$StrictFirewalledRequest$1.nextElement(StrictHttpFirewall.java:637

I think this change caused the problem:
#8644

To Reproduce
websocket connect with user agent above. The problem is all clients that i tested cannot send illegal characters, e.g.
wscat -c wss://staging.qeep.net/restapi/ws --header "User-Agent:iPhone Xʀ"
only apple can send this mess.

Expected behavior
let the requests pass or add clear warning about the change and how to get back the old behaviour

After downgrade to 5.3.4 requests are working, so definitive cause by 5.4.0

@qeepcologne qeepcologne added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 21, 2020
@jzheaux jzheaux self-assigned this Sep 21, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Sep 21, 2020

Hi, @qeepcologne, sorry to hear that you're having trouble.

I've locally added StrictHttpFirewall unit tests that use a User-Agent of iOS 13.7.0 Alamofire/5.2.2 iPhone Xʀ, and they pass. I'm not able to reproduce your issue with the information provided. So, there may be something upstream in your request that is incorrectly processing the special character.

Are you able to provide a minimal sample that reproduces the issue?

In the meantime, you can relax the check by supplying a predicate to StrictHttpFirewall#setAllowedHeaderValues:

@Bean 
public HttpFirewall firewall() {
    StrictHttpFirewall firewall = new StrictHttpFirewall();
    firewall.setAllowedHeaderValues(value -> true); // any header value is acceptable
}

Thanks for pointing out the docs. I've added a ticket to document that new feature.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 21, 2020
@qeepcologne
Copy link
Author

qeepcologne commented Sep 22, 2020

Hi jzheaux, thanks for the quick response.

You are right, i tried to reproduce it via testcase like you and and it worked.
I took the useragent from tomcat access log, maybe that device is sending something else (invisible control character) that is not in the access logs or that is lost when copy&paste into the testcase. We don't own such a device, but some of our users do.

I turned off the header value validation as you suggested and we can live with that if it is documented. In our case (app) we can filter the useragent on the client side, but there are also web requests. If there are more people with that issue, we need some exception for the UserAgent Header, otherwise everybody has to turn it off completely, which is not the purpose of the feature.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 22, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Sep 22, 2020

Before going down the route of adding a User-Agent exception, we should find out what's causing the problem to see if it can be fixed at its root cause.

invisible control character

It could be. A character encoding mismatch seems likely.

The URL encoding for ʀ is %CA%80. Some applications incorrectly decompose this into two separate characters. Notably, your logs say Ê whose URL encoding is %CA, the first of the two bytes. Since %80 is at the boundary of what a byte can represent (%7F), it wouldn't surprise me to learn that a character encoding mismatch converted it into an invalid character.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 22, 2020
@qeepcologne
Copy link
Author

There should be no encoding mismatch, as far as i know only ISO-8859-1 characters are allowed in http headers.
https://tools.ietf.org/html/rfc5987
Everything else need to be encoded and this is not the case, the value does not begin witha charset name, apple ignores the standard.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 23, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Sep 24, 2020

There should be no encoding mismatch, as far as i know only ISO-8859-1 characters are allowed in http headers.

Correct, the mismatch is something like Apple is expecting the header to be read as UTF-8, but Tomcat, for example, reads it as ISO-8859-1.

A quick test of StrictHttpFirewall confirms that reading ʀ as ISO-8859-1 and setting that as the value will cause the firewall to fail:

MockHttpServletRequest request = new MockHttpServletRequest();
HttpFirewall firewall = new StrictHttpFirewall();
request.addHeader("r", new String("ʀ".getBytes(), ISO_8859_1));
firewall.getFirewalledRequest(this.request).getHeader("r"); // throws exception

I'm not sure if your application server can be configured to read headers as UTF-8, but that's the next place I would look.

@jzheaux jzheaux closed this as completed Sep 24, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Oct 1, 2020

An improved workaround is to parse the header values as UTF-8, like so:

@Bean 
public HttpFirewall httpFirewall() {
    StrictHttpFirewall firewall = new StrictHttpFirewall();
    Pattern allowed = Pattern.compile("[\\p{IsAssigned}&&[^\\p{IsControl}]]*");
    firewall.setAllowedHeaderValues((header) -> {
        String parsed = new String(header.getBytes(ISO_8859_1), UTF_8);
        return allowed.matcher(parsed).matches();
    });
}

I believe this is an improvement since this way StrictHttpFirewall can continue to check header values as it already does.

@Ysunil016
Copy link

Thanks, This is Helpful :)

@mxossadm
Copy link

An improved workaround is to parse the header values as UTF-8, like so:

@Bean 
public HttpFirewall httpFirewall() {
    StrictHttpFirewall firewall = new StrictHttpFirewall();
    Pattern allowed = Pattern.compile("[\\p{IsAssigned}&&[^\\p{IsControl}]]*");
    firewall.setAllowedHeaderValues((header) -> {
        String parsed = new String(header.getBytes(ISO_8859_1), UTF_8);
        return allowed.matcher(parsed).matches();
    });
}

I believe this is an improvement since this way StrictHttpFirewall can continue to check header values as it already does.

Hello, I encountered the same problem, but I do not know how to make the above code take effect in my Java application;
In other words, where should I put this part of the code to make it work?
Looking forward to your reply.

@jzheaux
Copy link
Contributor

jzheaux commented May 23, 2022

@mxossadm, thanks for reaching out. To keep GitHub focused on issues, please consider posting your question on StackOverflow instead. You are welcome to post a link to your question on this issue if you like so that others can find it and respond.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants