Examine X-Forwarded-Host for additional port information

[boesing]

Q	A
Bugfix	yes

Description

Fixes #111

[boesing]

TBH: Not even sure if it makes any sense to pass a port via Host header. When I connect via TCP to port 443 and add port 80 to the Host header, it will still connect to 443 tho...
I wonder if any webserver out there then tries to internally connect to port 80. I hope they won't as this could be a serious security problem when some1 can use a webserver to actually connect to another port internally.

I guess most if not any webserver (besides apache2) do not pass any port via X-Forwarded-Host header but since there are already 2 reports regarding the same bug, I'd say lets fix this.

[weierophinney]

This looks... pretty sane. My one concern is how this plays with our fixes for [LP-2022-02](https://getlaminas.org/security/advisory/LP-2022-02}, though what it looks like in here is it is properly considering X-Forwarded-Port as the "source of truth" if both that and X-Forwarded-Host are present.

[TimWolla]

though what it looks like in here is it is properly considering X-Forwarded-Port as the "source of truth" if both that and X-Forwarded-Host are present.

@weierophinney From my understanding, the priority is implicitly decided based on the ordering of the headers within the $trustedHeaders list. I find this reasonable, but it should probably be spelled out explicitly that the order of headers matters (if this is the behavior we want to guarantee here).

---

The current version of the PR looks reasonable to me as well, the port is taken from the xfh header (if available), if both xfh and xfp match, it does the right thing by definition and if they differ, then the user is able to select the priority based on the ordering in the list. (If xfh and xfp differ, the reverse proxy is broken beyond repair, so … who cares).

Please just give me the time to also check the Traefik source code / behavior (see the existing comment thread with boesing), so that the decision can be backed up with some hard evidence in case someone complains in the future.

[TimWolla]

The proposed behavior looks correct to me. There should likely be a follow-up that makes ->withHost() more strict to prevent it from accepting garbage (such as colons or whitespace). See also this comment in a previous PR of mine: #97 (comment)