Fix client IP detection for IPv6 clients in forward auth

[Elandrya2711]

rSetIPHeader in the forward auth module splits r.RemoteAddr at the first colon before parsing it. For an IPv6 client the remote address looks like [2a01:4f8::1]:54321, so the cut yields [2a01, net.ParseIP fails and the function returns without setting X-Forwarded-For. At that point HandleAuthProviderRouting has already copied the client's headers into the authorization request (all of them, in the default configuration with no included headers set), so an X-Forwarded-For header supplied by the client survives untouched. Over IPv6, the client decides which IP the authorization server sees.

To reproduce: put a host behind forward auth with a provider that makes IP-based decisions, for example tinyauth with an IP bypass rule for 10.2.45.5, and send the same request with X-Forwarded-For: 10.2.45.5 twice. Over IPv4 the provider sees the real address and answers 401. Over IPv6 it sees 10.2.45.5, logs IP is in bypass list, skipping authentication, and the request reaches the protected backend with a 200. Confirmed on v3.3.3, v3.3.4 and current main.

This change parses the address with net.SplitHostPort, the same way GetRequesterIPUntrusted in mod/netutils/ipmatch.go already handles it, with a fallback to the raw value for a portless address. When no IP can be determined at all, the headers are now deleted instead of being left as they arrived, so a client-supplied value can never travel to the authorization provider. Behaviour for IPv4 clients is unchanged.

The deprecated Authelia module (src/mod/auth/sso/deprecated/authelia/authelia.go) contains the same colon-split pattern. I left it untouched to keep this diff small, but I can patch it the same way if you want.

[tobychui]

It looks good to me, but forward auth is a community maintained feature, and I have no infrastructure to test this out, so I am trusting you on this.

You have tested it on your build and it works right?

[Elandrya2711]

Yes. I built the branch and tested the fix end to end, not just the unit level.

The build is clean: go build, go vet and gofmt all pass, and the existing forward tests stay green.

For the behaviour I ran a local Zoraxy build with tinyauth v5.1.3 behind it, using an IP bypass rule for 10.2.45.0/24 and Zoraxy set as a trusted proxy. Sending the same request with X-Forwarded-For: 10.2.45.5 twice:

• Before the fix: over IPv4 tinyauth sees 127.0.0.1 and answers 401, over IPv6 it sees 10.2.45.5, logs IP is in bypass list, skipping authentication and the request reaches the backend with 200.
• After the fix: both IPv4 and IPv6 send 127.0.0.1 to tinyauth and both get 401. The forged header no longer survives.

I also checked the no-IP path (an address net.ParseIP can't handle): the client-supplied header is deleted rather than passed through, so it stays fail-closed.

[tobychui]

Cool, thanks for letting me know.

Since currently the v3.3.4 branch already at rc-3 and will not accept anymore changes that is not related to stability issue, I will keep it here first and migrate this to v3.3.5 rc-1 release later after the v3.3.4 release is out.

[Elandrya2711]

Fair enough on the freeze, but I'd put this in a different bucket: it's an auth bypass, not a feature. With forward auth and a provider doing IP-based decisions, an IPv6 client picks the IP the authorization server sees and walks past auth with a 200. Worth noting the forged value isn't tied to the client's address family: an IPv6 client can claim to be 127.0.0.1 or any internal v4 address, so v4-only bypass rules are in scope too. Anything the provider keys on client IP is affected, including rate limits and its audit log. The diff is 11 lines in one function, nothing else in the request path, IPv4 unchanged. Most freezes carve out security fixes for exactly this case.

If v3.3.4 stays closed, no argument from me, but could you then note it in the release notes or an advisory? Workaround for anyone who can't wait: set an explicit Request Headers allowlist that omits X-Forwarded-For and X-Original-IP. Empty list is the default and copies all client headers through, which is what makes the forged one survive.

[AnthonyMichaelTDM]

I would like to see a regression test for this behavior

[Elandrya2711]

Added TestRSetIPHeader in util_test.go: IPv4/IPv6 with and without port, client-supplied header overwritten in both families, headers dropped when no IP can be parsed. 5 of 7 cases fail on current main, all pass with the fix, IPv4 cases pass on both.