From a75f5c89cd1e11285c18c2d4b38d347cffc9e508 Mon Sep 17 00:00:00 2001 From: Thomas Ries Date: Sat, 9 Nov 2019 17:41:08 +0000 Subject: [PATCH] fix: Repetitions (T1 timer) during INVITE could cause loss of audio --- ChangeLog | 2 ++ src/dejitter.c | 2 +- src/proxy.c | 40 +++++++++++++++++++++++++++++++--------- src/rtpproxy.c | 9 +++++---- src/rtpproxy.h | 5 +++-- src/rtpproxy_relay.c | 44 ++++++++++++++++++++++++++++++-------------- src/siproxd.h | 4 ++-- 7 files changed, 74 insertions(+), 32 deletions(-) diff --git a/ChangeLog b/ChangeLog index e716f6e..9e54fff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ 0.8.3dev ======== + 09-Nov-2019: - fix: Repetitions (T1 timer) during INVITE could + cause loss of audio 10-May-2019: - fix: TCP fragment reassembly fails 17-Mar-2018: - Improved syslog output (more consistent behavior) 17-Jan-2018: - Deal with OPTION requests that have Max-Forwards=0 diff --git a/src/dejitter.c b/src/dejitter.c index e8eac70..d430008 100644 --- a/src/dejitter.c +++ b/src/dejitter.c @@ -417,7 +417,7 @@ static void send_top_of_que(int nolock) { sts = rtp_relay_stop_fwd(&callid, m->errret->direction, m->errret->media_stream_no, - nolock); + -1, nolock); if (sts != STS_SUCCESS) { /* force the streams to timeout on next occasion */ m->errret->timestamp=0; diff --git a/src/proxy.c b/src/proxy.c index 30f787c..91cde90 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -148,8 +148,8 @@ DEBUGC(DBCLASS_PROXY,"index i=%i",i); /* if this is CANCEL/BYE request, stop RTP proxying */ if (MSG_IS_BYE(request) || MSG_IS_CANCEL(request)) { /* stop the RTP proxying stream(s) */ - rtp_stop_fwd(osip_message_get_call_id(request), DIR_INCOMING); - rtp_stop_fwd(osip_message_get_call_id(request), DIR_OUTGOING); + rtp_stop_fwd(osip_message_get_call_id(request), DIR_INCOMING, -1); + rtp_stop_fwd(osip_message_get_call_id(request), DIR_OUTGOING, -1); /* check for incoming request */ } else if (MSG_IS_INVITE(request)) { @@ -221,8 +221,8 @@ sts=sip_obscure_callid(ticket); /* if this is CANCEL/BYE request, stop RTP proxying */ if (MSG_IS_BYE(request) || MSG_IS_CANCEL(request)) { /* stop the RTP proxying stream(s) */ - rtp_stop_fwd(osip_message_get_call_id(request), DIR_INCOMING); - rtp_stop_fwd(osip_message_get_call_id(request), DIR_OUTGOING); + rtp_stop_fwd(osip_message_get_call_id(request), DIR_INCOMING, -1); + rtp_stop_fwd(osip_message_get_call_id(request), DIR_OUTGOING, -1); } sts=sip_obscure_callid(ticket); @@ -487,6 +487,7 @@ int proxy_response (sip_ticket_t *ticket) { char *buffer; size_t buflen; osip_message_t *response; + int cseq; DEBUGC(DBCLASS_PROXY,"proxy_response"); @@ -550,8 +551,24 @@ sts=sip_obscure_callid(ticket); } else if ((MSG_IS_STATUS_4XX(response)) || (MSG_IS_STATUS_5XX(response)) || (MSG_IS_STATUS_6XX(response))) { - rtp_stop_fwd(osip_message_get_call_id(response), DIR_INCOMING); - rtp_stop_fwd(osip_message_get_call_id(response), DIR_OUTGOING); +//&&& in case of repetitions, this logic breaks! +// INVITE w/o auth credentials +// repeated INVITE w/o auth credentials +// 407 response to 1st INVITE +// INVITE w/ auth credentials +// 407 response to 2nd INVITE +// -> Bzzz, siproxd cancels the set-up RTP ports from INVITE(3) above +// I could: +// - simply ingore these stati (rtp relay would time-out eventually) +// - reduce rtp timeout for this stream and have if time-out quicker +// - somehow remember the CSEQ number of the last request and only do +// something if this CSEQ is equal (or higher) to the last request that changed +// the RTP relay setup +// tmp = osip_cseq_get_number(sip_msg->cseq); + cseq = atoi(osip_cseq_get_number(response->cseq)); + + rtp_stop_fwd(osip_message_get_call_id(response), DIR_INCOMING, cseq); + rtp_stop_fwd(osip_message_get_call_id(response), DIR_OUTGOING, cseq); } } /* if INVITE */ @@ -637,8 +654,11 @@ sts=sip_obscure_callid(ticket); } else if ((MSG_IS_STATUS_4XX(response)) || (MSG_IS_STATUS_5XX(response)) || (MSG_IS_STATUS_6XX(response))) { - rtp_stop_fwd(osip_message_get_call_id(response), DIR_INCOMING); - rtp_stop_fwd(osip_message_get_call_id(response), DIR_OUTGOING); +//&&& same here, repetitions break this logic + cseq = atoi(osip_cseq_get_number(response->cseq)); + + rtp_stop_fwd(osip_message_get_call_id(response), DIR_INCOMING, cseq); + rtp_stop_fwd(osip_message_get_call_id(response), DIR_OUTGOING, cseq); } } /* if INVITE */ @@ -782,6 +802,7 @@ int proxy_rewrite_invitation_body(sip_ticket_t *ticket, int direction){ int call_direction=0; int have_c_media=0; int isrtp = 0 ; + int cseq = 0; if (configuration.rtp_proxy_enable == 0) return STS_SUCCESS; @@ -1116,13 +1137,14 @@ if (configuration.debuglevel) /* * Start the RTP stream */ + cseq = atoi(osip_cseq_get_number(mymsg->cseq)); sts = rtp_start_fwd(osip_message_get_call_id(mymsg), client_id, rtp_direction, call_direction, media_stream_no, map_addr, &map_port, addr_media, msg_port, - isrtp); + isrtp, cseq); if (sts == STS_SUCCESS) { /* and rewrite the port */ diff --git a/src/rtpproxy.c b/src/rtpproxy.c index 55f2581..c1945e6 100644 --- a/src/rtpproxy.c +++ b/src/rtpproxy.c @@ -77,7 +77,7 @@ int rtp_start_fwd (osip_call_id_t *callid, client_id_t client_id, int direction, int call_direction, int media_stream_no, struct in_addr local_ipaddr, int *local_port, struct in_addr remote_ipaddr, int remote_port, - int isrtp) { + int isrtp, int cseq) { int sts=STS_FAILURE; int dejitter=0; @@ -94,7 +94,8 @@ int rtp_start_fwd (osip_call_id_t *callid, client_id_t client_id, sts = rtp_relay_start_fwd (callid, client_id, direction, call_direction, media_stream_no, local_ipaddr, local_port, - remote_ipaddr, remote_port, dejitter); + remote_ipaddr, remote_port, dejitter, + cseq); } else { ERROR("CONFIG: rtp_proxy_enable has invalid value: %d", configuration.rtp_proxy_enable); @@ -111,13 +112,13 @@ int rtp_start_fwd (osip_call_id_t *callid, client_id_t client_id, * STS_SUCCESS on success * STS_FAILURE on error */ -int rtp_stop_fwd (osip_call_id_t *callid, int direction) { +int rtp_stop_fwd (osip_call_id_t *callid, int direction, int cseq) { int sts = STS_FAILURE; if (configuration.rtp_proxy_enable == 0) { sts = STS_SUCCESS; } else if (configuration.rtp_proxy_enable == 1) { // Relay - sts = rtp_relay_stop_fwd(callid, direction, -1, LOCK_FDSET); + sts = rtp_relay_stop_fwd(callid, direction, -1, cseq, LOCK_FDSET); } else { ERROR("CONFIG: rtp_proxy_enable has invalid value: %d", configuration.rtp_proxy_enable); diff --git a/src/rtpproxy.h b/src/rtpproxy.h index ab42a1a..0c73a4f 100644 --- a/src/rtpproxy.h +++ b/src/rtpproxy.h @@ -49,6 +49,7 @@ typedef struct { char callid_number[CALLIDNUM_SIZE]; /* call ID */ char callid_host[CALLIDHOST_SIZE]; /* --"-- */ client_id_t client_id; + int cseq; int direction; /* Direction of RTP stream */ int call_direction; /* Direction of Call DIR_x */ int media_stream_no; @@ -69,9 +70,9 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, int rtp_direction, int call_direction, int media_stream_no, struct in_addr local_ipaddr, int *local_port, struct in_addr remote_ipaddr, int remote_port, - int dejitter); + int dejitter, int cseq); int rtp_relay_stop_fwd (osip_call_id_t *callid, int rtp_direction, - int media_stream_no, int nolock); + int media_stream_no, int cseq, int nolock); #define NOLOCK_FDSET 1 #define LOCK_FDSET 0 diff --git a/src/rtpproxy_relay.c b/src/rtpproxy_relay.c index f87dd99..115638a 100644 --- a/src/rtpproxy_relay.c +++ b/src/rtpproxy_relay.c @@ -391,7 +391,7 @@ static void *rtpproxy_main(void *arg) { sts = rtp_relay_stop_fwd(&callid, rtp_proxytable[i].direction, rtp_proxytable[i].media_stream_no, - NOLOCK_FDSET); + -1, NOLOCK_FDSET); if (sts != STS_SUCCESS) { /* force the streams to timeout on next occasion */ rtp_proxytable[i].timestamp=0; @@ -426,7 +426,7 @@ static void *rtpproxy_main(void *arg) { sts = rtp_relay_stop_fwd(&callid, rtp_proxytable[i].direction, rtp_proxytable[i].media_stream_no, - NOLOCK_FDSET); + -1, NOLOCK_FDSET); if (sts != STS_SUCCESS) { /* force the streams to timeout on next occasion */ rtp_proxytable[i].timestamp=0; @@ -482,7 +482,7 @@ static void *rtpproxy_main(void *arg) { * just one (unused?) has timed out. Seen with VoIPEX PBX! */ rtp_relay_stop_fwd(&callid, rtp_proxytable[i].direction, rtp_proxytable[i].media_stream_no, - NOLOCK_FDSET); + -1, NOLOCK_FDSET); } /* if */ } /* for i */ } /* if (t>...) */ @@ -512,7 +512,7 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, int rtp_direction, int call_direction, int media_stream_no, struct in_addr local_ipaddr, int *local_port, struct in_addr remote_ipaddr, - int remote_port, int dejitter) { + int remote_port, int dejitter, int cseq) { static int prev_used_port = 0; int num_ports; int i2, i, j; @@ -548,11 +548,12 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, } DEBUGC(DBCLASS_RTP,"rtp_relay_start_fwd: starting RTP proxy " - "stream for: CallID=%s@%s [Client-ID=%s] (%s,%s) #=%i", + "stream for: CallID=%s@%s [Client-ID=%s] (%s,%s) " + "cseq=%i #=%i", callid->number, callid->host, client_id.idstring, ((rtp_direction == DIR_INCOMING) ? "incoming RTP" : "outgoing RTP"), ((call_direction == DIR_INCOMING) ? "incoming Call" : "outgoing Call"), - media_stream_no); + cseq, media_stream_no); /* lock mutex */ #define return is_forbidden_in_this_code_section @@ -603,6 +604,12 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, sizeof(remote_ipaddr)); } + /* update CSEQ in proxytable if the current request has a higher one */ + if (cseq > rtp_proxytable[i].cseq) { + rtp_proxytable[i].cseq = cseq; + } + + #ifdef USE_DEJITTER /* Initialize up timecrontrol for dejitter function */ if ((configuration.rtp_input_dejitter > 0) || @@ -614,11 +621,12 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, /* return the already known local port number */ DEBUGC(DBCLASS_RTP,"RTP stream already active idx=%i (remaddr=%s, " - "remport=%i, lclport=%i, id=%s, #=%i)", + "remport=%i, lclport=%i, id=%s, cseq=%i, #=%i)", i, utils_inet_ntoa(remote_ipaddr), rtp_proxytable[i].remote_port, rtp_proxytable[i].local_port, rtp_proxytable[i].callid_number, + rtp_proxytable[i].cseq, rtp_proxytable[i].media_stream_no); *local_port=rtp_proxytable[i].local_port; sts = STS_SUCCESS; @@ -759,6 +767,7 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, /* store the passed Client-ID data */ memcpy(&rtp_proxytable[freeidx].client_id, &client_id, sizeof(client_id_t)); + rtp_proxytable[freeidx].cseq = cseq; rtp_proxytable[freeidx].direction = rtp_direction; rtp_proxytable[freeidx].call_direction = call_direction; rtp_proxytable[freeidx].media_stream_no = media_stream_no; @@ -807,12 +816,13 @@ int rtp_relay_start_fwd (osip_call_id_t *callid, client_id_t client_id, //&&& DEBUGC(DBCLASS_RTP,"rtp_relay_start_fwd: started RTP proxy " - "stream for: CallID=%s@%s [Client-ID=%s] %s #=%i idx=%i", + "stream for: CallID=%s@%s [Client-ID=%s] %s cseq=%i, " + "#=%i idx=%i", rtp_proxytable[freeidx].callid_number, rtp_proxytable[freeidx].callid_host, rtp_proxytable[freeidx].client_id.idstring, ((rtp_proxytable[freeidx].direction == DIR_INCOMING) ? "incoming RTP" : "outgoing RTP"), - rtp_proxytable[freeidx].media_stream_no, freeidx); + cseq, rtp_proxytable[freeidx].media_stream_no, freeidx); unlock_and_exit: /* unlock mutex */ @@ -829,13 +839,15 @@ unlock_and_exit: * if media_stream_no == -1, all media streams will be stopped, * otherwise only the specified one. * + * if cseq == -1, it will be ignored. + * * RETURNS * STS_SUCCESS on success * STS_FAILURE on error */ int rtp_relay_stop_fwd (osip_call_id_t *callid, int rtp_direction, - int media_stream_no, int nolock) { + int media_stream_no, int cseq, int nolock) { int i, sts; int retsts=STS_SUCCESS; int got_match=0; @@ -847,10 +859,10 @@ int rtp_relay_stop_fwd (osip_call_id_t *callid, } DEBUGC(DBCLASS_RTP,"rtp_relay_stop_fwd: stopping RTP proxy " - "stream for: %s@%s (%s) (nolock=%i)", + "stream for: %s@%s (%s), cseq=%i (nolock=%i)", callid->number, callid->host, ((rtp_direction == DIR_INCOMING) ? "incoming" : "outgoing"), - nolock); + cseq, nolock); /* * lock mutex - only if not requested to skip the lock. @@ -892,7 +904,11 @@ int rtp_relay_stop_fwd (osip_call_id_t *callid, (compare_callid(callid, &cid) == STS_SUCCESS) && (rtp_proxytable[i].direction == rtp_direction) && ((media_stream_no < 0) || - (media_stream_no == rtp_proxytable[i].media_stream_no))) { + (media_stream_no == rtp_proxytable[i].media_stream_no)) && + ((cseq < 0) || + (cseq >= rtp_proxytable[i].cseq)) + ) { + /* close RTP sockets */ if (rtp_proxytable[i].rtp_rx_sock > 0) { sts = close(rtp_proxytable[i].rtp_rx_sock); @@ -1024,7 +1040,7 @@ static void rtpproxy_kill( void ) { cid.host = rtp_proxytable[i].callid_host; sts = rtp_relay_stop_fwd(&cid, rtp_proxytable[i].direction, rtp_proxytable[i].media_stream_no, - LOCK_FDSET); + -1, LOCK_FDSET); if (sts != STS_SUCCESS) { DEBUGC(DBCLASS_RTP,"rtp_relay_stop_fwd did return error"); } diff --git a/src/siproxd.h b/src/siproxd.h index d26319f..4ad6b35 100644 --- a/src/siproxd.h +++ b/src/siproxd.h @@ -237,8 +237,8 @@ int rtp_start_fwd (osip_call_id_t *callid, client_id_t client_id, /*X*/ int direction, int call_direction, int media_stream_no, struct in_addr outbound_ipaddr, int *outboundport, struct in_addr lcl_client_ipaddr, int lcl_clientport, - int isrtp); -int rtp_stop_fwd (osip_call_id_t *callid, int direction); /*X*/ + int isrtp, int cseq); +int rtp_stop_fwd (osip_call_id_t *callid, int direction, int cseq); /*X*/ /* accessctl.c */ int accesslist_check(struct sockaddr_in from);