Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

HAproxy advised configuration sends IPv4-mapped address on X-Forwarded-For #12124

Closed
villepeh opened this issue Mar 1, 2022 · 6 comments · Fixed by #12279
Closed

HAproxy advised configuration sends IPv4-mapped address on X-Forwarded-For #12124

villepeh opened this issue Mar 1, 2022 · 6 comments · Fixed by #12279
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution

Comments

@villepeh
Copy link
Contributor

villepeh commented Mar 1, 2022

Description

I decided to try out HAproxy as Synapse reverse proxy. It looks like the advised configuration sends an incorrect IP with X-Forwarded-For, at least if your server supports both IPv4 and IPv6. It seems to "append" v6 address to your v4 address. I'm assuming this will cause problems with black-/whitelisting and such.

image

Also visible on HAproxy logs

image

Steps to reproduce

  • Install Synapse
  • Install HAproxy 2.5.4
  • Follow the reverse proxy instructions here
  • Start up Synapse and connect.

Expected behavior: Client and server being reported with a correct IP address, not this "v4 + v6 hybrid".

Fix

I just followed the instructions here instead: LINK

Instead of:
frontend https
bind :::443 v4v6 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1
[...]

...use this:
frontend https
bind *:443,:::443 v6only ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1
[...]

(and do the same for the federation port if needed)

Reverse proxy now forwards correct IPv4 and IPv6 address to Synapse.

Perhaps the guide should be updated?

Version information

  • Homeserver: Synapse

  • Version: 1.53.0

  • Install method: pip (python 3.9)

  • Platform: Slackware Linux.

(My first post here. Hope I got this right :)

@reivilibre
Copy link
Contributor

(I looked at http://cbonte.github.io/haproxy-dconv/configuration-1.6.html#5.1-v6only to see what the options mean.)

Interesting. I would hope this doesn't cause trouble with blacklisting — as far as I know it's just a different textual representation for IPv4 addresses encoded as IPv6 (https://datatracker.ietf.org/doc/html/rfc5156#section-2.2).
Hence saying that it's an 'incorrect' address doesn't seem right: it looks like a correct address to me (perhaps it's not what you wanted for some other reason, but seems valid nonetheless).

However I have no experience with IPv6 really (my ISP still doesn't support it, for a start) and I would like confirmation from someone who knows more about HAProxy and IPv6...

@reivilibre
Copy link
Contributor

I asked and it seems like this notation is fine, so it couldn't be called an 'incorrect' format.
Also worth noting that the blocklists store IPv6-compatible addresses internally, even for IPv4 addresses, so there shouldn't be a security issue here (if you find one, please do report it).

So really this issue seems to boil down to whether or not this is 'desirable', from the perspective of having nice logs. I can see why you find this less nice than just the plain IPv4 address, so I'd be happy to accept a PR to change the documentation for this.
(However it would be nice to check with someone who knows HAProxy that this is doing what it says it should.)

@villepeh villepeh changed the title HAproxy advised configuration sends incorrect IP on X-Forwarded-For HAproxy advised configuration sends IPv4-mapped address on X-Forwarded-For Mar 2, 2022
@villepeh
Copy link
Contributor Author

villepeh commented Mar 2, 2022

Interesting. I would hope this doesn't cause trouble with blacklisting — as far as I know it's just a different textual representation for IPv4 addresses encoded as IPv6 (https://datatracker.ietf.org/doc/html/rfc5156#section-2.2).
Hence saying that it's an 'incorrect' address doesn't seem right: it looks like a correct address to me (perhaps it's not what you wanted for some other reason, but seems valid nonetheless).

Ah, I wasn't aware of this RFC! Thanks for letting me know. I changed the title.

However it would be nice to check with someone who knows HAProxy that this is doing what it says it should.

I agree. My experience with it is the last 24 hours :) So far it seems to play nicely with Synapse.

@reivilibre reivilibre added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels Mar 2, 2022
@villepeh
Copy link
Contributor Author

#12279

@villepeh
Copy link
Contributor Author

@reivilibre I made a small PR. I'm not familiar with how GitHub works, so I hope I got it right. I included Delegation instructions as a "bonus".

@villepeh
Copy link
Contributor Author

The related PR is now merged. Closing!

@richvdh richvdh linked a pull request Mar 28, 2022 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants