[Asterisk-code-review] res rtp asterisk: Revert DTLS negotiation changes. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_rtp_asterisk: Revert DTLS negotiation changes.
......................................................................


res_rtp_asterisk: Revert DTLS negotiation changes.

Due to locking issues within pjnath these changes are being
reverted until pjnath can be changed.

ASTERISK-25645

Revert "res_rtp_asterisk.c: Fix DTLS negotiation delays."

This reverts commit 24ae124e4f7310cfa64c187b944b2ffc060da28d.

Change-Id: I2986cfb2c43dc14455c1bcaf92c3804f9da49705

Revert "res_rtp_asterisk: Resolve further timing issues with DTLS negotiation"

This reverts commit 965a0eee46d24321f74c244e23c5a5f45e67e12b.

Change-Id: Ie68fafde27dad4b03cb7a1e27ce2a8502c3f7bbe
---
M res/res_rtp_asterisk.c
1 file changed, 9 insertions(+), 50 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 3f65f13..85d997f 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -444,7 +444,6 @@
 #ifdef HAVE_OPENSSL_SRTP
 static int ast_rtp_activate(struct ast_rtp_instance *instance);
 static void dtls_srtp_check_pending(struct ast_rtp_instance *instance, struct ast_rtp *rtp, int rtcp);
-static void dtls_srtp_flush_pending(struct ast_rtp_instance *instance, struct ast_rtp *rtp);
 static void dtls_srtp_start_timeout_timer(struct ast_rtp_instance *instance, struct ast_rtp *rtp, int rtcp);
 static void dtls_srtp_stop_timeout_timer(struct ast_rtp_instance *instance, struct ast_rtp *rtp, int rtcp);
 #endif
@@ -1684,20 +1683,15 @@
 		if (rtp->rtcp) {
 			update_address_with_ice_candidate(rtp, AST_RTP_ICE_COMPONENT_RTCP, &rtp->rtcp->them);
 		}
-
-#ifdef HAVE_OPENSSL_SRTP
-		if (rtp->dtls.dtls_setup != AST_RTP_DTLS_SETUP_PASSIVE) {
-			dtls_perform_handshake(instance, &rtp->dtls, 0);
-		}
-		else {
-			dtls_srtp_flush_pending(instance, rtp); /* this flushes pending BIO for both rtp & rtcp as needed. */
-		}
-
-		if (rtp->rtcp && rtp->rtcp->dtls.dtls_setup != AST_RTP_DTLS_SETUP_PASSIVE) {
-			dtls_perform_handshake(instance, &rtp->rtcp->dtls, 1);
-		}
-#endif
 	}
+ 
+#ifdef HAVE_OPENSSL_SRTP
+	dtls_perform_handshake(instance, &rtp->dtls, 0);
+
+	if (rtp->rtcp) {
+		dtls_perform_handshake(instance, &rtp->rtcp->dtls, 1);
+	}
+#endif
 
 	if (!strictrtp) {
 		return;
@@ -1890,23 +1884,6 @@
 	struct dtls_details *dtls = !rtcp ? &rtp->dtls : &rtp->rtcp->dtls;
 
 	AST_SCHED_DEL_UNREF(rtp->sched, dtls->timeout_timer, ao2_ref(instance, -1));
-}
-
-static void dtls_srtp_flush_pending(struct ast_rtp_instance *instance, struct ast_rtp *rtp)
-{
-	struct dtls_details *dtls;
-
-	dtls = &rtp->dtls;
-	ast_mutex_lock(&dtls->lock);
-	dtls_srtp_check_pending(instance, rtp, 0);
-	ast_mutex_unlock(&dtls->lock);
-
-	if (rtp->rtcp) {
-		dtls = &rtp->rtcp->dtls;
-		ast_mutex_lock(&dtls->lock);
-		dtls_srtp_check_pending(instance, rtp, 1);
-		ast_mutex_unlock(&dtls->lock);
-	}
 }
 
 static void dtls_srtp_check_pending(struct ast_rtp_instance *instance, struct ast_rtp *rtp, int rtcp)
@@ -2141,8 +2118,6 @@
 			SSL_set_accept_state(dtls->ssl);
 		}
 
-		ast_mutex_lock(&dtls->lock);
-
 		dtls_srtp_check_pending(instance, rtp, rtcp);
 
 		BIO_write(dtls->read_bio, buf, len);
@@ -2153,7 +2128,6 @@
 			unsigned long error = ERR_get_error();
 			ast_log(LOG_ERROR, "DTLS failure occurred on RTP instance '%p' due to reason '%s', terminating\n",
 				instance, ERR_reason_error_string(error));
-			ast_mutex_unlock(&dtls->lock);
 			return -1;
 		}
 
@@ -2170,8 +2144,6 @@
 			/* Since we've sent additional traffic start the timeout timer for retransmission */
 			dtls_srtp_start_timeout_timer(instance, rtp, rtcp);
 		}
-
-		ast_mutex_unlock(&dtls->lock);
 
 		return res;
 	}
@@ -4861,20 +4833,7 @@
 		rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno);
 	}
 
-#ifdef HAVE_OPENSSL_SRTP
-	/* Trigger pending outbound DTLS packets received before the address was set.  Avoid unnecessary locking
-	 * by checking if we're passive. Without this, we only send the pending packets once a new SSL packet is
-	 * received in __rtp_recvfrom.  If rtp->ice, this is instead done on_ice_complete
-	 */
-#ifdef HAVE_PJPROJECT
-	if (rtp->ice) {
-		return;
-	}
-#endif
-	if (rtp->dtls.dtls_setup == AST_RTP_DTLS_SETUP_PASSIVE) {
-		dtls_srtp_flush_pending(instance, rtp);
-	}
-#endif
+	return;
 }
 
 /*! \brief Write t140 redundacy frame

-- 
To view, visit https://gerrit.asterisk.org/1976
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie68fafde27dad4b03cb7a1e27ce2a8502c3f7bbe
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>