res_rtp_asterisk: Fix issues with ICE renegotiation.

When re-inviting to add more streams it is possible for
the role of existing ICE sessions to be changed to the
incorrect value. This results in subsequent refreshes
within the sessions getting a role conflict and the ICE
session breaking down. This change only sets the role to
be the new value if an ICE renegotiation is actually
going to happen, otherwise the existing role is preserved.

As well if we encounter a situation where a unidirectional
ICE negotiation happens and the other side does not send us
candidates we will not store any information for sending
traffic, even though we know where they are reachable. This
change fixes this by using the source of the ICE traffic
itself as the target if no candidates are known and we
receive some ICE traffic.

ASTERISK-27088

Change-Id: I71228181e358917fcefc3100fad21b2fc02a59a9

1 files changed, 18 insertions, 7 deletions

diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index f6317f5f5..67140732a 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -319,6 +319,7 @@ struct ast_rtp {
 	ast_cond_t cond;            /*!< ICE/TURN condition for signaling */
 
 	struct ice_wrap *ice;       /*!< ao2 wrapped ICE session */
+	enum ast_rtp_ice_role role; /*!< Our role in ICE negotiation */
 	pj_turn_sock *turn_rtp;     /*!< RTP TURN relay */
 	pj_turn_sock *turn_rtcp;    /*!< RTCP TURN relay */
 	pj_turn_state_t turn_state; /*!< Current state of the TURN relay session */
@@ -681,7 +682,6 @@ static void ice_wrap_dtor(void *vdoomed)
 static int ice_reset_session(struct ast_rtp_instance *instance)
 {
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-	pj_ice_sess_role role = rtp->ice->real_ice->role;
 	int res;
 
 	ast_debug(3, "Resetting ICE for RTP instance '%p'\n", instance);
@@ -693,8 +693,9 @@ static int ice_reset_session(struct ast_rtp_instance *instance)
 	ast_debug(3, "Recreating ICE session %s (%d) for RTP instance '%p'\n", ast_sockaddr_stringify(&rtp->ice_original_rtp_addr), rtp->ice_port, instance);
 	res = ice_create(instance, &rtp->ice_original_rtp_addr, rtp->ice_port, 1);
 	if (!res) {
-		/* Preserve the role that the old ICE session used */
-		pj_ice_sess_change_role(rtp->ice->real_ice, role);
+		/* Use the current expected role for the ICE session */
+		pj_ice_sess_change_role(rtp->ice->real_ice, rtp->role == AST_RTP_ICE_ROLE_CONTROLLED ?
+			PJ_ICE_SESS_ROLE_CONTROLLED : PJ_ICE_SESS_ROLE_CONTROLLING);
 	}
 
 	/* If we only have one component now, and we previously set up TURN for RTCP,
@@ -765,6 +766,8 @@ static void ast_rtp_ice_start(struct ast_rtp_instance *instance)
 		ast_debug(3, "Proposed == active candidates for RTP instance '%p'\n", instance);
 		ao2_cleanup(rtp->ice_proposed_remote_candidates);
 		rtp->ice_proposed_remote_candidates = NULL;
+		/* If this ICE session is being preserved then go back to the role it currently is */
+		rtp->role = rtp->ice->real_ice->role;
 		return;
 	}
 
@@ -938,10 +941,7 @@ static void ast_rtp_ice_set_role(struct ast_rtp_instance *instance, enum ast_rtp
 		return;
 	}
 
-	pj_thread_register_check();
-
-	pj_ice_sess_change_role(rtp->ice->real_ice, role == AST_RTP_ICE_ROLE_CONTROLLED ?
-		PJ_ICE_SESS_ROLE_CONTROLLED : PJ_ICE_SESS_ROLE_CONTROLLING);
+	rtp->role = role;
 }
 
 /*! \pre instance is locked */
@@ -2522,6 +2522,17 @@ static int __rtp_recvfrom(struct ast_rtp_instance *instance, void *buf, size_t s
 			return -1;
 		}
 		if (!rtp->passthrough) {
+			/* If a unidirectional ICE negotiation occurs then lock on to the source of the
+			 * ICE traffic and use it as the target. This will occur if the remote side only
+			 * wants to receive media but never send to us.
+			 */
+			if (!rtp->ice_active_remote_candidates && !rtp->ice_proposed_remote_candidates) {
+				if (rtcp) {
+					ast_sockaddr_copy(&rtp->rtcp->them, sa);
+				} else {
+					ast_rtp_instance_set_remote_address(instance, sa);
+				}
+			}
 			return 0;
 		}
 		rtp->passthrough = 0;