[Home]

Summary:ASTERISK-22920: Crash while Forwarding from TLS extension with CHANNEL args secure_bridge_media and secure_bridge_signaling
Reporter:Shlomi Gutman (voicenter)Labels:patch
Date Opened:2013-11-27 06:44:34.000-0600Date Closed:2020-04-29 13:06:23
Priority:MajorRegression?
Status:Closed/CompleteComponents:Resources/res_srtp
Versions:1.8.14.0 1.8.24.0 11.2.2 11.5.0 11.6.0 11.7.0 13.18.4 Frequency of
Occurrence		Constant
Related
Issues:
Environment:CentOS release 5.8 (Final) kernel 2.6.18-308.24.1.el5 64bit, libsrtp 1.4.2(compiled manually) with 1.8.14 with and without patch (https://issues.asterisk.org/jira/browse/ASTERISK-18345) Debian GNU/Linux 7 (wheezy) kenrel 3.2.0-4-amd64 (3.2.51-1 64bit), with above patch on 11.5.0 and without patch on 1.8.24.0 11.7.0-rc1 11.6.0 with libsrtp 1.4.4 (from debian repo), self compiled 1.4.2, as well as 1.4.4 self compiled and self compiled with patch ( http://srtp.cvs.sourceforge.net/viewvc/srtp/srtp/crypto/replay/rdb.c?r1=1.4&r2=1.5) as mentioned on https://issues.asterisk.org/jira/browse/ASTERISK-16665 2 phones were tested snom 710 and fanvil C62 Attachments:( 0) A_channel.patch
( 1) B_local.patch
( 2) backtrace_ldd.log
( 3) debug.log
( 4) exten_incoming.conf
( 5) extension_realtime.info
( 6) gdb.log
( 7) ldd.log
( 8) rnewton_backtrace.txt
( 9) rnewton_sip.txt
(10) sip.conf
Description:Steps to reproduce:
1)Asterisk with self signed certificates or GoDaddy certificates
2)Extension connected with TLS transport (behind NAT in our case)
3)Route incoming call to that extension, while forward call from it without answering (302 - FORWARD)
4)Crash

I know that this bug may be related to srtp, but as we see it was not developed and maintained for a long time and as asterisk srtp based on itץ
I think at least it should crash the call only, but not whole asterisk.
Comments:By: Shlomi Gutman (voicenter) 2013-11-27 06:47:32.279-0600

ran asterisk in gdb
last parts of CLI with debug on before crash, bt, bt full, tread apply all bt data there accordingly
By: Matt Jordan (mjordan) 2013-11-27 08:41:11.514-0600

Please attach:

# A {{sip.conf}} that contains the peer configuration
# A sample {{extensions.conf}} that reproduces the problem
By: Shlomi Gutman (voicenter) 2013-11-27 10:52:35.167-0600

extension is from mysql (REALTIME)
so "select * from sip_Devices where name = 'NAME' \G" was saved to extension_realtime.info

exten_incoming.conf
is the dialplan before call, OPTION_SRTP is 1 so it does go to sub and ends up in sub-set-secure-on
which would set :
same => n,Set(CHANNEL(secure_bridge_signaling)=1)
same => n,Set(CHANNEL(secure_bridge_media)=1)
By: Shlomi Gutman (voicenter) 2013-11-27 11:08:58.623-0600

I actually succeed to stop the crashes on one of the machines.
I'm not sure what was the reason to resolve, i have other machine that crashes, so i'll try to find out what resolved it.
The steps were (with results):
1)Compile https://github.com/cisco/libsrtp/tree/feature-openssl as libsrtp (on debian), recompile asterisk 11.6.0 (still there were crashes)
2)Installed binutils-dev, compiled asterisk with DONT_OPTIMIZE and BETTER_BACKTRACE to provide more information (no crashes anymore, Murphy law, isn't it :) )
3)Compiled again without DONT_OPTIMIZE and BETTER_BACKTRACE (no crashes anymore)

I'll provide more info one i'll start to run tests on Centos machine.

I'm not sure if it's related but :

After there were no crashes the forward wouldn't work btw, as it would failt to create channel with "cause 58" problem is in SRTP (only TLS works), after checking code i found out that the problem is in line 5862 :
{code}
      for (i = 0; i < 2; i++) {
             if (ops[i][1]) {
                     if (ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0)) {
                               /* We require a security feature, but the channel won't provide it */
                             return -1;
                     }
             } else {
                       /* We don't care if we can't clear the option on a channel that doesn't support it */
                       ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0);
             }
       }
{code}

It would return -1, commenting if and using
ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0);
in all cases would solve the problem for forwards, but i'm not sure if it can/will break other things.

Is there any other informauion i can provide? And i'll keep updating here once i have more results with tests.
By: Shlomi Gutman (voicenter) 2013-11-27 11:18:27.430-0600

forgot to mention that i used  patch (https://issues.asterisk.org/jira/secure/attachment/43792/tls_read.patch) from bug ASTERISK-18345 to resolve the outgoing calls, though not sure if it's relevant.
By: Shlomi Gutman (voicenter) 2013-12-01 03:04:42.013-0600

Ha, i gathered new info from other server.
Centos one(info in envirement info of bug)
asterisk 1.8.14.0

ldd.log is after i installed it i got more info, and as i see it goes to pointer, that was probably freed before and thus it fails.
(usr/sbin/asterisk: munmap_chunk(): invalid pointer: XXXXXXXXXXXXXX ****)
backtrace_ldd.log is info from core

debug.log is when i set debug 10, though i get less info from ldd so you have ldd.log with backtrace and it's from other crash.

On this system it's 100% crashes.


By: Rusty Newton (rnewton) 2013-12-30 19:45:11.573-0600

@Shlomi,

I'm a little confused. It sounds like you were able to resolve the crashes in 11.6.0. Can you reproduce them in that version or any version after?

Are you saying now that the only issue is the forwarding itself doesn't work?
By: Shlomi Gutman (voicenter) 2014-01-07 04:26:35.213-0600

@Rusty Newton,
hey
After many tries and versions of srtp lib(as explained above) i resolved the issue on one server, but unfortunately as i had no time i was unable to write down all the steps/patches and versions.
For now i think it's still unresolved and crash is 100% reproducible ( as i tried different versions of asterisk and libsrtp on other platform after and was unable to resolve).

About the forward not working - on server i did resolved the crash forward didn't work (at least it didn't crash), but was resolved by solution proposed in few comments above.

I think the main priority here is not to resolve forward but to prevent crashes.




Btw as i tried to get deeper into the problem(more debugging) as far as i understand(correct me if i'm wrong) the crash happens when there is try to free "chan->tech_pvt":
channel.c -> function ast_channel_destructor:
if (chan->tech_pvt) {
               ast_log(LOG_WARNING, "Channel '%s' may not have been hung up properly\n", chan->name);
               ast_free(chan->tech_pvt);
       }

(Sorry for not sending line num as it seems to be different in different version of asterisk)

As far as i understand this is pointer that keeps data for the channel and in our case it might hold parts of srtp data and as i understand as there is no other place in code we free this data before taht line, it might be that it comes from libsrtp where it can free? reallocate? the data that our pointer pointing to?(correct me if i'm wrong)
By: Shlomi Gutman (voicenter) 2014-01-07 11:08:17.952-0600

I think i found the problem and solution but not really sure if it's the best option and if it's should be done that way(new to asterisk code, new to C).
I tried to detect if the channel is secure and just not run ast_free on chan->tech_pvt but i failed as it seems to be always be unsecure.
Then i remembered that there was warning saying( which would be related to the second problem we talked about) :
" WARNING[20339][C-00000000]: channel.c:5952 ast_request: Setting security requirements failed"
I decided to try to use the fix i used before:
{code}
--- channel.c.org       2014-01-07 18:58:45.396465980 +0200
+++ channel.c   2014-01-07 19:00:13.704244067 +0200
@@ -5855,15 +5855,15 @@ static int set_security_requirements(con
       ast_channel_unlock(r);

       for (i = 0; i < 2; i++) {
-               if (ops[i][1]) {
-                       if (ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0)) {
+//             if (ops[i][1]) {
+//                     if (ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0)) {
                               /* We require a security feature, but the channel won't provide it */
-                               return -1;
-                       }
-               } else {
+//                             return -1;
+//                     }
+//             } else {
                       /* We don't care if we can't clear the option on a channel that doesn't support it */
                       ast_channel_setoption(out, ops[i][0], &ops[i][1], sizeof(ops[i][1]), 0);
-               }
+//             }
       }

       return 0;
{code}

But i'm not sure that this wouldn't break anything else. If it wouldn't break srtp support in any specific places or it will break forward from secured call to regular extension and so on...
So it need to be reviewed and debugged, i'll try to update once i'll have more info.

P.S.
Would like to know if anyone can reproduce crash? as it's 100% reproducable for me for few versions (including 11.7.0) and several distros(debian/centos) and even different hw and i don't see too much activity/comments here.



By: Rusty Newton (rnewton) 2014-01-29 19:09:39.490-0600

Well.. Apparently most of the freely available soft phones that support TLS and SRTP don't also support call forwarding or call transfers with a 302, so it wasn't quick to reproduce.

Fortunately a colleague had a Snom 370 setup and was able to reproduce your issue. We reproduced it with optimizations, so the backtrace is pretty much the same as yours. Tomorrow he'll try to get one without optimizations. I'll attach the very minimal sip.conf required to help any developer that reproduces this.

The key to reproduction is the dialplan:

{noformat}
same => n,Set(CHANNEL(secure_bridge_signaling)=1)
same => n,Set(CHANNEL(secure_bridge_media)=1)
{noformat}
Without this, the forward works fine.

With the options set, the crash occurs. We didn't narrow it down to one option or the other.

By: Rusty Newton (rnewton) 2014-01-29 19:21:28.158-0600

Attaching rnewton_backtrace.txt and rnewton_sip.txt
By: Shlomi Gutman (voicenter) 2014-01-30 03:26:45.262-0600

As well as with only TLS (no srtp) there would be no crash, but the forward would not work as well.
Oh and the patch above fixed crash and forward (did you try it as well to approve?).
By: Joerg Sonnenberger (joerg) 2015-10-22 18:36:38.813-0500

I can confirm that the problem still happen with 11.20 and that the patch actually stops it.
By: Alexander Traud (traud) 2020-04-27 11:06:21.615-0500

First of all, thank you for reporting this issue. Without reports like this, everyone would consume Asterisk and not contribute something back to the community.

[~voicenter], like you, I am an external contributor. However, because of a bit of spare time, I look into the remaining chan_sip/TLS and SDES-sRTP issues. I was able to reproduce your setup and the crash. It simply needs a phone which allows forwarding via SIP status 302 like a recent [Snom|https://service.snom.com/display/wiki/Call+Forwarding].

The provided logs contain two important puzzle pieces: “Channel 'Local/…' may not have been hung up properly”. That is the last log entry before the crash. (1) Asterisk tried to release that channel. (2) The channel type is not ‘SIP’ as usual but ‘Local’ because of the call forwarding. Local uses ao2_ref (reference counting) when it comes to releasing. Therefore, the wrong release function was called – it worked for {{chan_sip}} but not in general.

Long story short, the crash was fixed with [GERRIT-3725|https://gerrit.asterisk.org/3725]. Consequently, ASTERISK-26306 was a duplicate of this issue report. I simply missed it. Thanks to the comment from Richard Mudgett, a core member of the Asterisk team, the crash is fixed since Asterisk 11.24.0 and Asterisk 13.12.0.

I wonder why nobody increased the severity to ‘critical’ here. An external entity, a SIP phone attached to Asterisk is able to crash Asterisk, depending on the configuration of the phone. Yeek!

However, your issue report (and your proposed patch) is about more. It is about a working {{secure_bridge_media}} even in case of call forwarding. Therefore, the solution to ASTERISK-26306 just decreased the severity of your report from critical to major. Consequently, I had to look deeper into your proposed patch. I went for a conservative approach and changed just Local Proxy and not all channel drivers, because: There are channel drivers which do not support these items like the new SIP channel driver PJSIP. That should be written somewhere as a warning.

Anyway, I attached both alternatives, your approach {{A_channel.patch}} and my approach {{B_local.patch}}. Perhaps the Asterisk team finds more (local) channel drivers, which can be covered, for example, changing {{ast_unreal_setoption}} in core_unreal. I do not know. That would be a middle course between your and my approach. I submitted my approach for review. Hope you like it, Shlomi (and everyone else watching this issue). Otherwise, please, comment!
By: Friendly Automation (friendly-automation) 2020-04-29 13:06:24.912-0500

Change 14345 merged by Joshua Colp:
core_local: Local calls are always secure.

[https://gerrit.asterisk.org/c/asterisk/+/14345|https://gerrit.asterisk.org/c/asterisk/+/14345]
By: Friendly Automation (friendly-automation) 2020-04-29 13:07:49.435-0500

Change 14343 merged by Joshua Colp:
core_local: Local calls are always secure.

[https://gerrit.asterisk.org/c/asterisk/+/14343|https://gerrit.asterisk.org/c/asterisk/+/14343]
By: Friendly Automation (friendly-automation) 2020-04-29 13:08:11.617-0500

Change 14344 merged by Joshua Colp:
core_local: Local calls are always secure.

[https://gerrit.asterisk.org/c/asterisk/+/14344|https://gerrit.asterisk.org/c/asterisk/+/14344]
By: Friendly Automation (friendly-automation) 2020-04-29 13:09:35.696-0500

Change 14327 merged by Joshua Colp:
core_local: Local calls are always secure.

[https://gerrit.asterisk.org/c/asterisk/+/14327|https://gerrit.asterisk.org/c/asterisk/+/14327]