Module: kamailio
Branch: master
Commit: 53afec4962e2dc3ec0971ba97da94f37c57780d3
URL: https://github.com/kamailio/kamailio/commit/53afec4962e2dc3ec0971ba97da94f3…
Author: Daniel-Constantin Mierla <miconda(a)gmail.com>
Committer: Daniel-Constantin Mierla <miconda(a)gmail.com>
Date: 2017-08-31T11:19:50+02:00
core: clist - helper macro to test if a clist is empty
---
Modified: src/core/clist.h
---
Diff: https://github.com/kamailio/kamailio/commit/53afec4962e2dc3ec0971ba97da94f3…
Patch: https://github.com/kamailio/kamailio/commit/53afec4962e2dc3ec0971ba97da94f3…
---
diff --git a/src/core/clist.h b/src/core/clist.h
index 2b796e6ff4..143c61951c 100644
--- a/src/core/clist.h
+++ b/src/core/clist.h
@@ -15,8 +15,8 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
@@ -121,4 +121,10 @@
#define clist_foreach_safe(head, v, bak, dir) \
for((v)=(head)->dir, (bak)=(v)->dir; (v)!=(void*)(head); \
(v)=(bak), (bak)=(v)->dir)
+
+/*! \brief test if clist is empty (1 - empty; 0 - not empty)*/
+#define clist_empty(head, dir) \
+ (((head)->dir!=(void*)(head))?0:1)
+
+
#endif
The topoh module uses its `mask_ip` parameter to tag and identify fields that have been encrypted with its header value. As of Kamailio 4.4.6 code has been added to make sure that `mask_ip` is properly used in the request URI, or `Via`, or any other appropriate SIP header variable before it attempts to perform decryption. Here is a code block that was added at line 738 in `modules/topoh/th_msg.c`
```
/* Do nothing if ruri is not encoded */
if ((REQ_LINE(msg).uri.len<th_uri_prefix.len) ||
(strncasecmp(REQ_LINE(msg).uri.s,th_uri_prefix.s,th_uri_prefix.len)!=0))
{
LM_DBG("ruri [%.*s] is not encoded",REQ_LINE(msg).uri.len,REQ_LINE(msg).uri.s);
return 0;
}
```
This effectively skips any Request URIs that don't properly match the expected `th_uri_prefix` which is created using the `mask_ip` value assigned to `topoh`.
<!--
Explain what you did, what you expected to happen, and what actually happened.
-->
So if I set topoh's `mask_ip` and assume I have set a `mask_key`:
```
modparam("topoh", "mask_ip", "192.168.99.184")
```
Then an `ACK` message like this gets detected and decoded as expected:
```
ACK sip:192.168.99.184;line=sr-1IFG6oxISo4wSekmUolOBKVwbolIboxd6JdwS7xiUekISKPm10NH18Rz1uBZtTpG SIP/2.0
```
However, if one of my incoming carriers decides to be extra special and append port `:5060` to the request URI like this:
```
ACK sip:192.168.99.184:5060;line=sr-1IFG6oxISo4wSekmUolOBKVwbolIboxd6JdwS7xiUekISKPm10NH18Rz1uBZtTpG SIP/2.0
```
It gets skipped by topoh because it no longer detects this variation of the URI and I get a message like this:
```
Aug 29 21:36:10 ip-172-31-4-69 /usr/sbin/kamailio[4629]: DEBUG: topoh [th_msg.c:742]: th_unmask_ruri(): ruri [sip:192.168.99.184:5060;line=sr-1IFG
6oxISo4wSekmUolOBKVwbolIboxd6JdwS7xiUekISKPm10NH18Rz1uBZtTpG] is not encoded
```
### Troubleshooting
I thought I might be smart and tried to change the `mask_ip` to `192.168.99.184:5060` but this is additionally blocked by the code that validates `Via` headers in `/modules/topoh/th_msg.c` line 393:
```
/* Skip if via is not encoded */
if (via->host.len!=th_ip.len
|| strncasecmp(via->host.s, th_ip.s, th_ip.len)!=0)
{
LM_DBG("via %d is not encoded",i);
continue;
}
```
It only compares the `host` part of the VIA with the `mask_ip` parameter which is `192.168.99.184:5060` (including the port) and therefore doesn't match and is skipped in decoding.
#### Reproduction
1. Set `topoh` module `mask_ip` to any acceptable IP address
2. Attempt to handle any traffic from a carrier that adds `:5060` automatically to the end of its request URIs
3. The call will go through and then drop ~90 seconds due to incessant attempts of the Kamailio server attempting to reach the bogus `mask_ip` address
#### Debugging Data
I believe the above information is fairly plain. I have included the pertinent debug logs, but it is fairly well describing why `topoh` is not decoding certain lines that it should be.
#### Log Messages
Example of a line that should be decoded which is not being decoded because the incoming carrier has added `:5060` to the `mask_ip` used in the Request URI.
```
Aug 29 21:36:10 ip-172-31-4-69 /usr/sbin/kamailio[4629]: DEBUG: topoh [th_msg.c:742]: th_unmask_ruri(): ruri [sip:192.168.99.184:5060;line=sr-1IFG
6oxISo4wSekmUolOBKVwbolIboxd6JdwS7xiUekISKPm10NH18Rz1uBZtTpG] is not encoded
```
#### SIP Traffic
![examplecall](https://user-images.githubusercontent.com/1807347/29846497-f65ed1e2-8ccb-11e7-9a16-b18cfb4365a9.png)
Example of a call that repeatedly attempts to access bogus `mask_ip` in its route because it is not detected by topoh for decoding after it is passed through a carrier which adds `:5060` to its Request URIs.
### Possible Solutions
No workarounds :(
I also don't have the option of asking carriers to change their standing policy of appending the port ":5060" but the validation efforts in the `topoh` module could be expanded to accept an undesignated port number in the URI or `topoh` could include a `mask_port` parameter so that it builds its URI to expect one in the request URI and in `Via` header fields.
When I have more time to work on the validation C code I will include some suggestions, but others may have a more informed philosophical approach.
### Additional Information
To see all changes to `th_msg.c`:
```
git diff 4.4.5 4.4.6 -- modules/topoh/th_msg.c
```
```
version: kamailio 4.4.6 (x86_64/linux) becbde
flags: STATS: Off, USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MEM, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLACKLIST, HAVE_RESOLV_RES
ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144, MAX_LISTEN 16, MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB
poll method support: poll, epoll_lt, epoll_et, sigio_rt, select.
id: becbde
compiled on 10:23:24 Jun 16 2017 with gcc 4.4.7
```
* **Operating System**:
```
CentOS release 6.8 (Final)
Linux ip-172-31-4-69.us-west-2.compute.internal 2.6.32-642.3.1.el6.x86_64 #1 SMP Tue Jul 12 18:30:56 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
```
--
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/issues/1222
Module: kamailio
Branch: 4.4
Commit: 897942d47e404a17d235c7dcb6237f024b99e3eb
URL: https://github.com/kamailio/kamailio/commit/897942d47e404a17d235c7dcb6237f0…
Author: Kamailio Dev <kamailio.dev(a)kamailio.org>
Committer: Kamailio Dev <kamailio.dev(a)kamailio.org>
Date: 2017-08-30T13:16:09+02:00
modules: readme files regenerated - topoh ... [skip ci]
---
Modified: modules/topoh/README
---
Diff: https://github.com/kamailio/kamailio/commit/897942d47e404a17d235c7dcb6237f0…
Patch: https://github.com/kamailio/kamailio/commit/897942d47e404a17d235c7dcb6237f0…
---
diff --git a/modules/topoh/README b/modules/topoh/README
index 213524545c..fb7442cc56 100644
--- a/modules/topoh/README
+++ b/modules/topoh/README
@@ -34,6 +34,7 @@ Daniel-Constantin Mierla
3.7. vparam_prefix (str)
3.8. callid_prefix (str)
3.9. sanity_checks (integer)
+ 3.10. uri_prefix_checks (integer)
List of Examples
@@ -46,6 +47,7 @@ Daniel-Constantin Mierla
1.7. Set vparam_prefix parameter
1.8. Set callid_prefix parameter
1.9. Set sanity_checks parameter
+ 1.10. Set uri_prefix_checks parameter
Chapter 1. Admin Guide
@@ -68,6 +70,7 @@ Chapter 1. Admin Guide
3.7. vparam_prefix (str)
3.8. callid_prefix (str)
3.9. sanity_checks (integer)
+ 3.10. uri_prefix_checks (integer)
1. Overview
@@ -113,6 +116,7 @@ Chapter 1. Admin Guide
3.7. vparam_prefix (str)
3.8. callid_prefix (str)
3.9. sanity_checks (integer)
+ 3.10. uri_prefix_checks (integer)
3.1. mask_key (str)
@@ -226,3 +230,22 @@ modparam("topoh", "callid_prefix", "***")
...
modparam("topoh", "sanity_checks", 1)
...
+
+3.10. uri_prefix_checks (integer)
+
+ If set to 1, topoh module will check if URIs to be decoded match the
+ expected prefix composed from mask IP and parameter name prefix. It can
+ make the topoh processing safer by avoiding to try decoding URIs which
+ were not encoded previously by topoh.
+
+ Note: do not enable this option if you have SIP devices that can alter
+ the URI values it takes from Contact or Record-Route headers (like
+ adding port 5060 when no port is in received URIs, or thet introduce
+ new parameters at unknown position).
+
+ Default value is 0.
+
+ Example 1.10. Set uri_prefix_checks parameter
+...
+modparam("topoh", "uri_prefix_checks", 1)
+...
Module: kamailio
Branch: 4.4
Commit: 4b2c51a527cb5e7810694ef64754608e8aca68a3
URL: https://github.com/kamailio/kamailio/commit/4b2c51a527cb5e7810694ef64754608…
Author: Daniel-Constantin Mierla <miconda(a)gmail.com>
Committer: Daniel-Constantin Mierla <miconda(a)gmail.com>
Date: 2017-08-30T13:07:44+02:00
topoh: documented uri_prefix_checks parameter
(cherry picked from commit ca3f2fc6dd2c94410b225d2efb82081b828ddd01)
(cherry picked from commit 826b3cab606fb51c7139fe81463aeb2409f40b67)
---
Modified: modules/topoh/doc/topoh_admin.xml
---
Diff: https://github.com/kamailio/kamailio/commit/4b2c51a527cb5e7810694ef64754608…
Patch: https://github.com/kamailio/kamailio/commit/4b2c51a527cb5e7810694ef64754608…
---
diff --git a/modules/topoh/doc/topoh_admin.xml b/modules/topoh/doc/topoh_admin.xml
index fd588cd5f3..f4f5702a06 100644
--- a/modules/topoh/doc/topoh_admin.xml
+++ b/modules/topoh/doc/topoh_admin.xml
@@ -10,9 +10,9 @@
<!-- Module User's Guide -->
<chapter>
-
+
<title>&adminguide;</title>
-
+
<section>
<title>Overview</title>
<para>
@@ -253,7 +253,34 @@ modparam("topoh", "sanity_checks", 1)
</programlisting>
</example>
</section>
-
+ <section>
+ <title><varname>uri_prefix_checks</varname> (integer)</title>
+ <para>
+ If set to 1, topoh module will check if URIs to be decoded match
+ the expected prefix composed from mask IP and parameter name prefix.
+ It can make the topoh processing safer by avoiding to try decoding
+ URIs which were not encoded previously by topoh.
+ </para>
+ <para>
+ Note: do not enable this option if you have SIP devices that can
+ alter the URI values it takes from Contact or Record-Route headers
+ (like adding port 5060 when no port is in received URIs, or
+ thet introduce new parameters at unknown position).
+ </para>
+ <para>
+ <emphasis>
+ Default value is 0.
+ </emphasis>
+ </para>
+ <example>
+ <title>Set <varname>uri_prefix_checks</varname> parameter</title>
+ <programlisting format="linespecific">
+...
+modparam("topoh", "uri_prefix_checks", 1)
+...
+</programlisting>
+ </example>
+ </section>
</section>
</chapter>