From 235b6c030b847889c1aa55989cca481cde60d30e Mon Sep 17 00:00:00 2001 From: Andy Newlands Date: Thu, 18 Aug 2022 16:39:38 +0100 Subject: [PATCH] Only hangup after too many SRTP errors if SWITCH_RTP_FLAG_SRTP_HANGUP_ON_ERROR. Remove fix-specific diagnostics --- src/switch_rtp.c | 161 ++++++++++++++++++++++------------------------- 1 file changed, 75 insertions(+), 86 deletions(-) diff --git a/src/switch_rtp.c b/src/switch_rtp.c index 848925269c..9d1af0c6be 100644 --- a/src/switch_rtp.c +++ b/src/switch_rtp.c @@ -649,13 +649,6 @@ static handle_rfc2833_result_t handle_rfc2833(switch_rtp_t *rtp_session, switch_ in_digit_seq = ntohs((uint16_t) rtp_session->last_rtp_hdr.seq); ts = htonl(rtp_session->last_rtp_hdr.ts); -#ifdef DEBUG_2833 - if (key == 0) { - if ((channel = switch_core_session_get_channel(rtp_session->session)) != NULL) - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_WARNING, "Invalid DTMF characters detected on Call-ID: %s, DTMF: 0x%02x (actual: 0x%02x) Duration: %u\n", switch_channel_get_variable(channel, "call_uuid"), key, packet[0], duration); - } -#endif - if (rtp_session->flags[SWITCH_RTP_FLAG_PASS_RFC2833]) { if (end) { @@ -5891,88 +5884,84 @@ static switch_size_t do_flush(switch_rtp_t *rtp_session, int force, switch_size_ switch_socket_recvfrom(rtp_session->from_addr, rtp_session->sock_input, 0, (void *) &rtp_session->recv_msg, &bytes); #ifdef ENABLE_SRTP + // Ensure we decrypt BEFORE attempting to decode DTMF payload + // (the following code was copied, verbatim (apart from the "goto" destination), from read_rtp_packet() + switch_mutex_lock(rtp_session->ice_mutex); + if (rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV] && rtp_session->has_rtp && + (check_recv_payload(rtp_session) || + rtp_session->last_rtp_hdr.pt == rtp_session->recv_te || + rtp_session->last_rtp_hdr.pt == rtp_session->cng_pt)) { + //if (rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV] && (!rtp_session->ice.ice_user || rtp_session->has_rtp)) { + int sbytes = (int) *bytes; + srtp_err_status_t stat = 0; - if (rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV] && rtp_session->has_rtp && - (check_recv_payload(rtp_session) || - rtp_session->last_rtp_hdr.pt == rtp_session->recv_te || - rtp_session->last_rtp_hdr.pt == rtp_session->cng_pt)) { - int sbytes = (int) bytes; - srtp_err_status_t stat = 0; + if (rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV_RESET] || !rtp_session->recv_ctx[rtp_session->srtp_idx_rtp]) { + switch_rtp_clear_flag(rtp_session, SWITCH_RTP_FLAG_SECURE_RECV_RESET); + srtp_dealloc(rtp_session->recv_ctx[rtp_session->srtp_idx_rtp]); + rtp_session->recv_ctx[rtp_session->srtp_idx_rtp] = NULL; + if ((stat = srtp_create(&rtp_session->recv_ctx[rtp_session->srtp_idx_rtp], + &rtp_session->recv_policy[rtp_session->srtp_idx_rtp])) || !rtp_session->recv_ctx[rtp_session->srtp_idx_rtp]) { - if (rtp_session->recv_ctx[rtp_session->srtp_idx_rtp]) { + rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV] = 0; + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_ERROR, "Error! RE-Activating Secure RTP RECV\n"); + rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV] = 0; + switch_mutex_unlock(rtp_session->ice_mutex); + return SWITCH_STATUS_FALSE; + } else { -#ifdef DEBUG_SRTP_FLUSH - if (sbytes) { - q = 0; - bytes_buf[0] = 0; - for(i = 0; sbytes && i < sbytes; i++) { - sprintf(bytes_buf + q, "0x%02x ", ((u_char *)(&rtp_session->recv_msg))[i]); - q = strlen(bytes_buf); - } - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "do_flush loop BEFORE decryption: bytes = %d\nData: %s\n", sbytes, bytes_buf); - } -#endif + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_INFO, "RE-Activating Secure RTP RECV\n"); + rtp_session->srtp_errs[rtp_session->srtp_idx_rtp] = 0; + } + } - // Ensure we properly decrypt SRPT before reading DTMF - if (!rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV_MKI]) { - stat = srtp_unprotect(rtp_session->recv_ctx[rtp_session->srtp_idx_rtp], &rtp_session->recv_msg.header, &sbytes); - } else { - stat = srtp_unprotect_mki(rtp_session->recv_ctx[rtp_session->srtp_idx_rtp], &rtp_session->recv_msg.header, &sbytes, 1); - } + if (!(*flags & SFF_PLC) && rtp_session->recv_ctx[rtp_session->srtp_idx_rtp]) { + if (!rtp_session->flags[SWITCH_RTP_FLAG_SECURE_RECV_MKI]) { + stat = srtp_unprotect(rtp_session->recv_ctx[rtp_session->srtp_idx_rtp], &rtp_session->recv_msg.header, &sbytes); + } else { + stat = srtp_unprotect_mki(rtp_session->recv_ctx[rtp_session->srtp_idx_rtp], &rtp_session->recv_msg.header, &sbytes, 1); + } -#ifdef DEBUG_SRTP_FLUSH - if (sbytes) { - q = 0; - bytes_buf[0] = 0; - for(i = 0; sbytes && i < sbytes; i++) { - sprintf(bytes_buf + q, "0x%02x ", ((u_char *)(&rtp_session->recv_msg))[i]); - q = strlen(bytes_buf); - } - switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "do_flush loop AFTER decryption: bytes = %d\nData: %s\n", sbytes, bytes_buf); - } -#endif + if (rtp_session->flags[SWITCH_RTP_FLAG_NACK] && stat == srtp_err_status_replay_fail) { + /* false alarm nack */ + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_DEBUG1, "REPLAY ERR, FALSE NACK\n"); + sbytes = 0; + *bytes = 0; + if (rtp_session->stats.rtcp.pkt_count) { + rtp_session->stats.rtcp.period_pkt_count--; + rtp_session->stats.rtcp.pkt_count--; + } + switch_mutex_unlock(rtp_session->ice_mutex); + goto done_srtp; + } + } - if (rtp_session->flags[SWITCH_RTP_FLAG_NACK] && stat == srtp_err_status_replay_fail) { - /* false alarm nack */ - switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_DEBUG1, "REPLAY ERR, FALSE NACK\n"); - sbytes = 0; - bytes = 0; - if (rtp_session->stats.rtcp.pkt_count) { - rtp_session->stats.rtcp.period_pkt_count--; - rtp_session->stats.rtcp.pkt_count--; - } - goto done_srtp; - } - } + if (stat && rtp_session->recv_msg.header.pt != rtp_session->recv_te && rtp_session->recv_msg.header.pt != rtp_session->cng_pt) { + int errs = ++rtp_session->srtp_errs[rtp_session->srtp_idx_rtp]; + if (rtp_session->flags[SWITCH_RTP_FLAG_SRTP_HANGUP_ON_ERROR] && stat != srtp_err_status_replay_old) { + char *msg; + switch_srtp_err_to_txt(stat, &msg); + if (errs >= MAX_SRTP_ERRS) { + switch_channel_t *channel = switch_core_session_get_channel(rtp_session->session); + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, + "SRTP %s unprotect failed with code %d (%s) %ld bytes %d errors\n", + rtp_type(rtp_session), stat, msg, (long)*bytes, errs); + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, + "Ending call due to SRTP error\n"); + switch_channel_hangup(channel, SWITCH_CAUSE_SRTP_READ_ERROR); + } else if (errs >= WARN_SRTP_ERRS && !(errs % WARN_SRTP_ERRS)) { + switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, + "SRTP %s unprotect failed with code %d (%s) %ld bytes %d errors\n", + rtp_type(rtp_session), stat, msg, (long)*bytes, errs); + } + } + sbytes = 0; + } else { + rtp_session->srtp_errs[rtp_session->srtp_idx_rtp] = 0; + } - if (stat && rtp_session->recv_msg.header.pt != rtp_session->recv_te && rtp_session->recv_msg.header.pt != rtp_session->cng_pt) { - int errs = ++rtp_session->srtp_errs[rtp_session->srtp_idx_rtp]; - if (stat != 10) { - char *msg; - if (stat == srtp_err_status_replay_fail) msg="replay check failed (in do_flush)"; - else if (stat == srtp_err_status_auth_fail) msg="auth check failed (in do_flush)"; - else msg=""; - if (errs >= MAX_SRTP_ERRS) { - switch_channel_t *channel = switch_core_session_get_channel(rtp_session->session); - switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, - "SRTP %s unprotect failed (in do_flush) with code %d (%s) %ld bytes %d errors\n", - rtp_type(rtp_session), stat, msg, (long)bytes, errs); - switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, - "Ending call due to SRTP error (in do_flush)\n"); - switch_channel_hangup(channel, SWITCH_CAUSE_SRTP_READ_ERROR); - } else if (errs >= WARN_SRTP_ERRS && !(errs % WARN_SRTP_ERRS)) { - switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING, - "SRTP %s unprotect failed (in do_flush) with code %d (%s) %ld bytes %d errors\n", - rtp_type(rtp_session), stat, msg, (long)bytes, errs); - } - } - sbytes = 0; - } else { - rtp_session->srtp_errs[rtp_session->srtp_idx_rtp] = 0; - } - - bytes = sbytes; - } + *bytes = sbytes; + } + switch_mutex_unlock(rtp_session->ice_mutex); done_srtp: #endif @@ -8874,8 +8863,8 @@ static int rtp_common_write(switch_rtp_t *rtp_session, switch_rtp_clear_flag(rtp_session, SWITCH_RTP_FLAG_SECURE_SEND_RESET); srtp_dealloc(rtp_session->send_ctx[rtp_session->srtp_idx_rtp]); rtp_session->send_ctx[rtp_session->srtp_idx_rtp] = NULL; - if ((stat = srtp_create(&rtp_session->send_ctx[rtp_session->srtp_idx_rtp], - &rtp_session->send_policy[rtp_session->srtp_idx_rtp])) || !rtp_session->send_ctx[rtp_session->srtp_idx_rtp]) { + if (srtp_create(&rtp_session->send_ctx[rtp_session->srtp_idx_rtp], + &rtp_session->send_policy[rtp_session->srtp_idx_rtp]) || !rtp_session->send_ctx[rtp_session->srtp_idx_rtp]) { switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_ERROR, "Error! RE-Activating %s Secure RTP SEND\n", rtp_type(rtp_session)); rtp_session->flags[SWITCH_RTP_FLAG_SECURE_SEND] = 0; @@ -9470,8 +9459,8 @@ SWITCH_DECLARE(switch_status_t) switch_rtp_write_raw(switch_rtp_t *rtp_session, switch_rtp_clear_flag(rtp_session, SWITCH_RTP_FLAG_SECURE_SEND_RESET); srtp_dealloc(rtp_session->send_ctx[rtp_session->srtp_idx_rtp]); rtp_session->send_ctx[rtp_session->srtp_idx_rtp] = NULL; - if ((stat = srtp_create(&rtp_session->send_ctx[rtp_session->srtp_idx_rtp], - &rtp_session->send_policy[rtp_session->srtp_idx_rtp])) || !rtp_session->send_ctx[rtp_session->srtp_idx_rtp]) { + if (srtp_create(&rtp_session->send_ctx[rtp_session->srtp_idx_rtp], + &rtp_session->send_policy[rtp_session->srtp_idx_rtp]) || !rtp_session->send_ctx[rtp_session->srtp_idx_rtp]) { switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_ERROR, "Error! RE-Activating Secure RTP SEND\n"); rtp_session->flags[SWITCH_RTP_FLAG_SECURE_SEND] = 0; status = SWITCH_STATUS_FALSE;