[kamailio/kamailio] topos: adding param contact_hostname (#1596)
<!-- 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 - [ ] Small bug fix (non-breaking change which fixes an issue) - [x] 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 --> - [ ] 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 --> When using TLS with TOPOS the remote UAS could open a new TLS socket to the contact header. The contact header must contain a domain name with a signed certitificate. Adding the parameter to TOPOS module to control the host part of the contact headers ``` modparam("topos", "contact_hostname", "proxy.domain.com") ``` Default behavior is left unchanged I will update documentation it his modification is confirmed to be valuable. Else I would be curious to know how this should be handled differently. You can view, comment on, or merge this pull request online at: https://github.com/kamailio/kamailio/pull/1596 -- Commit Summary -- * topos: adding param contact_hostname -- File Changes -- M src/modules/topos/topos_mod.c (2) M src/modules/topos/tps_storage.c (13) -- Patch Links -- https://github.com/kamailio/kamailio/pull/1596.patch https://github.com/kamailio/kamailio/pull/1596.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596
I am fine to add this parameter, but the patch lacks checking if the size is exceeding the available space in the buffer -- it checks only sv.len, which is the uri from where to extract ip, port and transport, not it should also check for param len. If this and the docs are added, then can be merged. There is an alternative to use record_route_preset(), because the contact address is built from the Record-Route URI added by kamailio. But again, I am fine to add a parameter if it is found to be more convenient and simpler for config. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-405879711
Hi, reading the code I found it was taken from the RR header, I missed `record_route_preset()` This param was not absolutely necessary but I guess it is making things more clear for someone wanting to use TOPOS. And maybe simpler as well. I have tested the code "back ported" only for now, but I believe this should be enough to make sure it is working fine. Thanks for the review -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-406019020
Just to clarify: do you want that the port and transport from Record-Route to be kept in the Contact when using the parameter with the domain, right? That practically results only in doing an A/AAA DNS query on the domain, if you want SRV or NAPTR to be done, then this way is not working, but I see also the benefits of preserving the port and transport from R-R. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-408407922
I think it is better to use the contact header param only taking optional port and transport from it. And not use the Record route information at all this way we support NAPTR/SRV and things are less confusing for the user. Thanks for the pointing this out, it was not my intent to use the port + transport from the RR. I will : - update the PR and the doc too explicitly state that the host part can also contain params. - parse the module contact param in mod_init and fail if is it invalid. - make sure we are not using puri.port and puri.transport from the RR - do a few tests -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-408566488
I just made more tests trying to control the transport from the param. I then realized that support for NAPTR in the Contact is not making sense because we should use the transport used by the received request. Remote UACs should be doing SRV lookups even if the transport is specified, unless you know otherwise. I have reached the conclusion that the best option is to use the port and transport from RR. At this point I do not see any limitations with this way of doing things. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-408684291
When Kamailio is the UAC we select the transport as usual and request it in the Contact header (reusing the logic used with RR) and remote UA SRV failover can only use the same transport ... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-408685136
If we conclude that the value of this new param is too low, we can drop this MR and add more documentation on how Topos is using RR and how you end-up controlling the contact header instead. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#issuecomment-409631585
Merged #1596 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1596#event-1787136565
participants (2)
-
Daniel-Constantin Mierla -
Julien Chavanton