Fix: Add support for running CHProxy behind another proxy
Description
This PR adds a few new server settings: proxy.enable & proxy.header. proxy.enable adds enables the proxy middleware, which by default fetches the IP from well known proxy headers (X-Forwarded-For, X-Real-Ip, Forwarded). It will assume the first IP in the chain (if there are multiple) is the actual client IP. If proxy.header is set to a string, instead the proxy middleware will try to find a header that mathes that string (e.g. X-My-Proxy-Header).
Fixes #216
Pull request type
Please check the type of change your PR introduces:
- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):
Checklist
- [ ] Linter passes correctly
- [ ] Add tests which fail without the change (if possible)
- [ ] All tests passing
- [ ] Extended the README / documentation, if necessary
Does this introduce a breaking change?
- [ ] Yes
- [ ] No
Further comments
There are some points of note:
- I have assumed everywhere that the first IP in the chain is the real client IP. For most use cases of CHProxy this will most likely be the right call. I guess documenting the behaviour is the way to go?
- in the
an.Containsmethod, we now ignore the fact that aRemoteAddrmight not contain a port. We could try to fetch this information from other headers (e.g.X-Forwarded-Port) but we didn't seem to use the port anyway. - For custom headers I directly use the information stored in the header. This can go wrong if the custom header uses a custom scheme as well (e.g. IP string list, something similar to the forwarded header). But to support all edge cases, we would most likely need to support custom middleware.
- Right now I ignore all other proxy related headers and assume that the only thing the proxy changes on the request is the IP. This is not true for all proxies, but will suffice for most.
The main takeaway is that we won't be able to solve all possible use cases, however this should provide decent "default" behaviour for running CHProxy behind another proxy or load balancer.
I noticed an issue in the tests (I still need to update the PR). The test worked well in isolation but failed when I run all the tests:
The tests rely on creating a net.Listener to listenAndServe which wraps the listener in a server handling connections. To end the tests, the net.Listener is closed. However closing the net.Listener doesn't immediately close the server. If instead I make the http.Server available to the tests and call Shutdown the issue is removed. So it seems there are some race issues with multiple server instances running in the tests.
I need to find a clean way to handle this in the code (I used var globalServer *http.Server for easy debugging) and will let you know once I updated the tests.