<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [X] Commit message has the format required by CONTRIBUTING guide - [X] Commits are split per component (core, individual modules, libs, utils, ...) - [X] Each component has a single commit (if not, squash them into one commit) - [X] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [X] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [X] PR should be backported to stable branches - [X] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail --> As I reported to developers' mailing list (https://lists.kamailio.org/pipermail/sr-dev/2021-October/064243.html) if support for proxy protocol is enabled, kamailio is not able to match a tcp DEST_IP/DEST_PORT to an alias of an already existing tcp connection. This beacause the (local) destination ip on which the connection was initiated is overwritten by the destination IP and port carried by the proxy protocol header. Since the original connection info is saved when parsing the ortocol we can use that information to build a new alias and to use it to match outgoing tcp connection later. You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2888
-- Commit Summary --
* <a href="https://github.com/kamailio/kamailio/pull/2888/commits/3bd59bb17a575a4d369799887b98981b20ea076b">core: tcp - add alias for cinfo dst IP</a>
-- File Changes --
M src/core/tcp_main.c (11)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2888.patch https://github.com/kamailio/kamailio/pull/2888.diff
@phildeb approved this pull request.
Thanks for the pull request. You probably considered that, but would it not make sense to only add the additional two aliases if the proxy protocol ("tcp_accept_haproxy") is actually enabled?
Hi Henning, that's a good question. Currently the cinfo struct of the tcp connection is only filled by the proxy protocol parsing (oterwise is init to null) so the check when adding the alias gives the same results (we also chek that the dst IP there is different from the one stored in rcv, to not have duplicate aliases). I've chosen this approach in case that, for other reasons, in the future, we have cinfo != rcv in the connection struct.
Thanks for the reply, then it should be equivalent. I would suggest to add a comment in this regards to the new logic, to make it clear for eventual future changes. Lets wait a few more days for other comments before merging.
@grumvalski pushed 1 commit.
86d897534d5c814de1f87744f8d5d6ca6a35f384 core: tcp - add alias for cinfo dst IP
Right, I've added a comment and force pushed.
Hello Federico @grumvalski I tried before use `haproxy` protocol and found Kamailio implementation is very restrictive.
As example 1) CPU 100% usage when created TCP connection and data do not send #2658 2) no ability to define a list of trusted sources, because now any fraud host can send crafted haproxy packet and break ACL rules used on Kamailio side. Relevant feature `set_real_ip_from` exist in nginx ([Link](https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from)). From my point of view it is a big security hole.
Could you also look for limitations described above.
Just for info, nginx config snippet with haproxy feature. ``` server { listen 0.0.0.0:3128 proxy_protocol; listen [::]:3128 proxy_protocol; set_real_ip_from 4.101.84.5/32; set_real_ip_from 4.101.84.133/32; set_real_ip_from 4.236.25.5/32; set_real_ip_from 4.236.25.133/32; real_ip_header proxy_protocol; ... ```
Thanks for the update @grumvalski. @sergey-safarov - lets not start an (unrelatated) discussion in this pull request. Use the sr-dev list instead, there is already the "Proxy protocol issue" thread, or start a new one.
Looks ok for me, I'm fine to merge it if no other comments.
Merged #2888 into master.
Thanks for merging Henning. Should we backport it also to stable branches?
@grumvalski: if it does not work without it, then you can backport - you have commit access.
-
Daniel-Constantin Mierla -
Federico Cabiddu
-
Henning Westerholt
-
Philippe
-
sergey-safarov