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

Support for X-Forwarded-For and Forwarded for="..." #23260

Closed
larsgrefer opened this issue Jul 9, 2019 · 12 comments
Closed

Support for X-Forwarded-For and Forwarded for="..." #23260

larsgrefer opened this issue Jul 9, 2019 · 12 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@larsgrefer
Copy link
Contributor

The ForwardedHeaderFilter seems to ignore the X-Forwarded-For header and it's Forwarded complement: https://tools.ietf.org/html/rfc7239#section-5.2

As user of this filter, I would expect X-Forwarded-For or Forwarded for= to be handled and removed from the request. If there is a reason to not handle X-Forwarded-For this should be documented.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 9, 2019
@luvarqpp
Copy link
Contributor

I am trying not to make separate issue, so please look, if this is same problem: spring-projects/spring-security#7081

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jul 17, 2019

@luvarqpp, the server.use-forward-headers property in Spring Boot relies on the server, and does not use the ForwardedHeaderFilter. The two are actually mutually exclusive, and you should either enable server.use-forward-headers property or configure the ForwardedHeaderFilter, but not both.

Related to this, it does look like in Spring Boot 2.2 you'll be able to choose where you get your forwarded header (server or Spring Framework).

@rstoyanchev
Copy link
Contributor

@larsgrefer can you clarify what handling you would expect to see? Overrides to getRemoteAddr, getRemoteHost, and getRemotePort in HttpServletRequest?

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Jul 17, 2019
@larsgrefer
Copy link
Contributor Author

larsgrefer commented Jul 17, 2019

I'd expect overrides to getRemoteAddr and getRemoteHost (the same way like Tomcat's RemoteIpValve does it)

We've migrated our application from Tomcat's RemoteIpValve to Spring's ForwardedHeaderFilter, because we need the support of X-Forwarded-Host. But then we noticed that X-Forwarded-For isn't handled anymore.

@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 Jul 17, 2019
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 17, 2019
@rstoyanchev rstoyanchev added this to the 5.x Backlog milestone Jul 17, 2019
@rstoyanchev rstoyanchev changed the title ForwardedHeaderFilter doesn't handle X-Forwarded-For Support for X-Forwarded-For and Forwarded for Jul 17, 2019
@rstoyanchev rstoyanchev added the status: ideal-for-contribution An issue that a contributor can help us with label Jul 17, 2019
@luvarqpp
Copy link
Contributor

@rstoyanchev Thanks for clarification, but setting server.use-forward-headers to true does not help making simple login project to work correctly. Still first redirect does have wrong Location header. I have my experiment based on spring security example called hello world.

PS: I have made given project to work, when Forwarded header is in use (instead of X-Forwarded-* family). Does tomcat require standardized header for forwarding?

@rstoyanchev
Copy link
Contributor

Thanks for the comment but this isn't the place to ask about Tomcat.

@yawo
Copy link

yawo commented Sep 3, 2019

Nice ! this filter saved me

molassar pushed a commit to molassar/spring-framework that referenced this issue Sep 4, 2019
molassar pushed a commit to molassar/spring-framework that referenced this issue Sep 6, 2019
@rstoyanchev rstoyanchev changed the title Support for X-Forwarded-For and Forwarded for Support for X-Forwarded-For and Forwarded for="..." Sep 24, 2019
@hlang
Copy link

hlang commented Oct 16, 2019

Is there a general idea how to handle the presence of both Forwarded and X-Forwarded- headers at the same time?
The Forwarded header can contain some or all of the parameters: by, for, host, proto
The implementation in UriComponentsBuilder#adaptFromForwardedHeaders right now is not really symetric.
First it checks if there is a Forwarded header, if yes it tries to take the parameters proto, host from this.
Only for proto there is a fallback implemented. Meaning that if proto is not found in Forwarded header, the X-Forwarded-Ssl is checked additionally.
This is not done for host. So if there is a Forwarded header but if this does not contain host-parameter, host and port are not extracted at all.
Wouldn't it be better to also fallback to the X-Forwarded-Host and X-Forwarded-Port header if they exist?

When using spring application behind a fabio proxy this problem occurs. The headers that fabio sets are: forwarded: for=100.93.64.162; proto=http; by=192.168.30.12; httpproto=http/1.1 and x-forwarded-host: 10.65.250.31:9999
So the host is not extracted in current implementation.

Edited: created own issue #23819

@juanmbellini
Copy link

juanmbellini commented Oct 16, 2019

@hlang You must register an instance of the ForwardedHeaderFilter. You can do this in any @Configuration class (assuming you are using spring boot):

@Bean
public FilterRegistrationBean<ForwardedHeaderFilter> forwardedHeaderFilter() {
    final var bean = new FilterRegistrationBean<ForwardedHeaderFilter>();
    bean.setFilter(new ForwardedHeaderFilter());
    bean.setOrder(Ordered.HIGHEST_PRECEDENCE + 10);
    return bean;
}

Note that the ForwardedHeaderFilter is part of the org.springframework.web.filter package, located in the spring-web jar, and the FilterRegistrationBean is part of the org.springframework.boot.web.servlet, in the spring-boot jar.

@hlang
Copy link

hlang commented Oct 16, 2019

@juanmbellini I use the ForwardedHeaderFilter. This works.
The question is to the implementation of UriComponentsBuilder used by this ForwardedHeaderFilter.

@rstoyanchev
Copy link
Contributor

@hlang this has nothing to do with the issue under which you're commenting. That creates noise for anyone interested in this ticket now or in the future. Please, create a separate issue.

jazdw added a commit to MangoAutomation/ma-core-public that referenced this issue Oct 23, 2019
rather than Spring ForwardedHeaderFilter due to bug
spring-projects/spring-framework#23260
molassar pushed a commit to molassar/spring-framework that referenced this issue Nov 11, 2019
@rstoyanchev rstoyanchev removed the status: ideal-for-contribution An issue that a contributor can help us with label Nov 15, 2019
@rstoyanchev rstoyanchev added the status: superseded An issue that has been superseded by another label Nov 15, 2019
@rstoyanchev
Copy link
Contributor

This is superseded by #23582.

@jhoeller jhoeller removed this from the 5.x Backlog milestone Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants