Regression: ForwardedHeaderFilter no longer support IPv6

[czubin]

Affects:: spring-web 5.3.6

After updating from spring 5.2.8 to 5.3.6 we noticed that ForwardedHeaderFilter would throw:

java.lang.IllegalArgumentException: Invalid IPv4 address: 2a02:1810:84ae:d800:c8da:d498:64ec:6edb
	at org.springframework.web.util.UriComponentsBuilder.parseForwardedFor(UriComponentsBuilder.java:363)
	at org.springframework.web.filter.ForwardedHeaderFilter$ForwardedHeaderExtractingRequest.<init>(ForwardedHeaderFilter.java:246)

The code before:

String baseUrl = this.scheme + "://" + this.host + (port == -1 ? "" : ":" + port);    
Supplier<HttpServletRequest> delegateRequest = () -> (HttpServletRequest) getRequest();    
this.forwardedPrefixExtractor = new ForwardedPrefixExtractor(delegateRequest, baseUrl);

The code as is:

this.remoteAddress = UriComponentsBuilder.parseForwardedFor(request, request.getRemoteAddress());

String baseUrl = this.scheme + "://" + this.host + (port == -1 ? "" : ":" + port);
Supplier<HttpServletRequest> delegateRequest = () -> (HttpServletRequest) getRequest();
this.forwardedPrefixExtractor = new ForwardedPrefixExtractor(delegateRequest, baseUrl);```

The invocation of UriComponentsBuilder.parseForwardedFor only support IPv4.

[rstoyanchev]

The line where the IAE error is raised:

this.remoteAddress = UriComponentsBuilder.parseForwardedFor(request, request.getRemoteAddress());

was added in support of "X-Forward-For" and "Forwarded for:" as part of #23582 with commits 883ad09 and d627f60 while the validation was added in #26748.

The address "2a02:1810:84ae:d800:c8da:d498:64ec:6edb" is indeed invalid. It should have surrounding square brackets. What I'm not sure is is exactly what happened before. I'm guessing that the header causing the issue simply wasn't parsed. Can you please provide the relevant headers in their entirety?

[czubin]

You are correct. Seems I'll have to file a bug with the gateway vendor.

Below the trace (I've removed some parts of the IP's)

"forwarded": [
  "by=193.???.???.???:8444; for=2a02:1812:240b:5465:1c8a:8ab9:8d05:bca7; host=???.api.???.com; proto=https",
  "for=164.???.???.;host=???.???.eu;proto=https;proto-version="
],
"x-forwarded-for": [
  "2a02:1812:240b:5465:1c8a:8ab9:8d05:bca7, 104.???.???.???"
],```

[poutsma]

Closing as invalid.

[PatrickGotthard]

Why was this issue closed as invalid? The problem still exists and older versions accepted such IPv6 addresses.
I know the IPv6 addresses you mentioned are not RFC-compliant (they should be wrapped in "[XYZ]") but isn't it possible to be a little bit more failure-tolerant? Maybe just drop the header instead of failing the complete request?

Seems like the Spring Cloud Gateway team solved the issue: spring-cloud/spring-cloud-gateway#2217

[rstoyanchev]

Unfortunately in this case it is not possible to be lenient due to ambiguity since what comes after the last ":" could be part of the host address or a port. The change in Spring Cloud Gateway is to format IPv6 addresses correctly for forwarded headers it creates, and not for parsing forwarded headers.

[PatrickGotthard]

But what should the user do when there's no chance to fix the incorrect header (e.g. the wrong headers are created by the Service Providers infrastructure)?

In my case I only need the original hostname (X-Forwarded-Host) but the ForwardedHeaderFilter also tries to parse the IP Address and then the complete request crashes. May it be an option to make the ForwardedHeaderFilter configurable, e.g. that you could configure what headers to parse and how lenient it should be?

[rstoyanchev]

X-Forwarded-Host contains host and port, so parsing is required to get the original hostname. Lenient parsing is not possible and such an option would be misleading at best. For a very specific case, if you want or can make certain assumptions, create add your own filter ordered ahead of ForwaredHeaderFilter to modify the X-Forwarded-Host header.

[acdha]

@poutsma This seems like a big problem not to fix: currently it means that you can't deploy behind Cloudflare. I opened a support request for them but have no idea when it will be fixed.

[acdha]

The address "2a02:1810:84ae:d800:c8da:d498:64ec:6edb" is indeed invalid. It should have surrounding square brackets.

I did a bit of research and this appears to be an overly-strict reading: the X-Forwarded-For semantics are not formalized and there are a number of implementations in the wild, including Cloudflare's CDN and nginx, which do not use the RFC 5952 format. The Forwarded header defined in RFC 7239 does specify that format so this strict validation would be appropriate for using that header in preference to X-Forwarded-For.

There is a consideration for the possibility of port numbers being appended to IP addresses in the X-Forwarded-For usage rather than using the more common X-Forwarded-Port. This appears to be most commonly associated with Microsoft's Azure App proxy which e.g. Rails had to handle. This could be handled based on the number of colons since it'd be either 1 or 7, but it definitely makes me wish Forwarded was more widely implemented since it has clear defined semantics for this.

It seems like no matter what the decision is here, Spring should handle the exception more gracefully – an HTTP 400 request would be the most appropriate semantically since there's no way the request will succeed without the client changing it.

[rstoyanchev]

@acdha, X-Forwarded-For is not well specified but the rules for IPv4 and IPv6 are in RFC 3986, section 3.2.2 and that clearly says that IPv6 must have "[" and "]".

[acdha]

@acdha, X-Forwarded-For is not well specified but the rules for IPv4 and IPv6 are in RFC 3986, section 3.2.2 and that clearly says that IPv6 must have "[" and "]".

Yes, but that’s not normative for X-Forwarded-For, which is why so many popular tools don’t implement that. This is directly addressed in RFC 7239:

Note that IPv6 addresses may not be quoted in X-Forwarded-For and may not be enclosed by square brackets, but they are quoted and enclosed in square brackets in "Forwarded".

This makes it hard to deploy Spring apps behind Cloudflare, GCP load balancers, nginx, etc. without an additional proxy layer to rewrite the XFF header.

[rstoyanchev]

There isn't anything normative for X-Forwarded-For, so relying on what the URI spec says doesn't seem so unreasonable. That said I understand your point about the pain of running behind popular proxies.

Looking again in more detail, UriComponentsBuilder#parseForwardedFor performs this validation only for the "Forwarded" header. For "X-Forwarded-For" it takes the string as a host, as-is, so I'm not sure I understand how the handling of "X-Forwarded-For" is causing an issue?

[acdha]

Looking again in more detail, UriComponentsBuilder#parseForwardedFor performs this validation only for the "Forwarded" header. For "X-Forwarded-For" it takes the string as a host, as-is, so I'm not sure I understand how the handling of "X-Forwarded-For" is causing an issue?

Thank you so much for pointing that out — this turns out to be a bug in the AWS API Gateway's dispatching for HTTP backends. I had checked the edge logs, internal load-balancer for the Spring containers, and what's seen by the Lambda function authenticating requests, and those all showed only X-Forwarded-For on the requests. My Spring apps are running in containers, however, and the HTTP integration type does something which is not documented or done for logging or Lambdas by silently converting X-Forwarded-For into Forwarded but does so without normalizing IPv6 addresses (I would bet this happens when they go to add the APIGW as the last hop).

I've reported this upstream.

[gabac]

I assume I should have opened the bug here rather than in spring-cloud: spring-cloud/spring-cloud-gateway#2512

Google Cloud Run has the same issue with not compliant IPv6 addresses