Direct Media calls within private network sometimes get one way audio

When endpoints with direct_media enabled, behind a firewall (Asterisk on a
separate network) and were bridged sometimes Asterisk would send the ip
address of the firewall in the sdp to one of the phones in the reinvite
resulting in one way audio. When sending the reinvite Asterisk will retrieve
the media address from the associated rtp instance, but if frames were being
read this can be overwritten with another address (in this case the
firewall's).  This patch ensures that Asterisk uses the original device
address when using direct media.

ASTERISK-24563
Reported by: Steve Pitts
Review: https://reviewboard.asterisk.org/r/4216/
........

Merged revisions 429195 from http://svn.asterisk.org/svn/asterisk/branches/12


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/13@429196 65c4cc65-6c06-0410-ace0-fbb531ad65f3

4 files changed, 109 insertions, 18 deletions

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index ab03580ad..ee55fd38d 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -23062,7 +23062,7 @@ static void handle_response_invite(struct sip_pvt *p, int resp, const char *rest
 		} else if (!reinvite) {
 			struct ast_sockaddr remote_address = {{0,}};
 
-			ast_rtp_instance_get_remote_address(p->rtp, &remote_address);
+			ast_rtp_instance_get_requested_target_address(p->rtp, &remote_address);
 			if (ast_sockaddr_isnull(&remote_address) || (!ast_strlen_zero(p->theirprovtag) && strcmp(p->theirtag, p->theirprovtag))) {
 				ast_log(LOG_WARNING, "Received response: \"200 OK\" from '%s' without SDP\n", p->relatedpeer->name);
 				ast_set_flag(&p->flags[0], SIP_PENDINGBYE);
@@ -32291,7 +32291,7 @@ static int sip_allow_anyrtp_remote(struct ast_channel *chan1, struct ast_rtp_ins
 	if (ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA)) {
 		struct ast_sockaddr us = { { 0, }, }, them = { { 0, }, };
 
-		ast_rtp_instance_get_remote_address(instance, &them);
+		ast_rtp_instance_get_requested_target_address(instance, &them);
 		ast_rtp_instance_get_local_address(instance, &us);
 
 		if (ast_apply_acl(acl, &them, "SIP Direct Media ACL: ") == AST_SENSE_DENY) {
diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index db5cd34ed..a1a17da43 100644
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -876,6 +876,40 @@ int ast_rtp_instance_write(struct ast_rtp_instance *instance, struct ast_frame *
 struct ast_frame *ast_rtp_instance_read(struct ast_rtp_instance *instance, int rtcp);
 
 /*!
+ * \brief Set the incoming source address of the remote endpoint that we are sending RTP to
+ *
+ * This sets the incoming source address the engine is sending RTP to. Usually this
+ * will be the same as the requested target address, however in the case where
+ * the engine "learns" the address (for instance, symmetric RTP enabled) this
+ * will then contain the learned address.
+ *
+ * \param instance The RTP instance to change the address on
+ * \param address Address to set it to
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ */
+int ast_rtp_instance_set_incoming_source_address(struct ast_rtp_instance *instance,
+						 const struct ast_sockaddr *address);
+
+/*!
+ * \brief Set the requested target address of the remote endpoint
+ *
+ * This should always be the address of the remote endpoint. Consequently, this can differ
+ * from the address the engine is sending RTP to.  However, usually they will be the same
+ * except in some circumstances (for instance when the engine "learns" the address if
+ * symmetric RTP is enabled).
+ *
+ * \param instance The RTP instance to change the address on
+ * \param address Address to set it to
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ */
+int ast_rtp_instance_set_requested_target_address(struct ast_rtp_instance *instance,
+						  const struct ast_sockaddr *address);
+
+/*!
  * \brief Set the address of the remote endpoint that we are sending RTP to
  *
  * \param instance The RTP instance to change the address on
@@ -895,7 +929,8 @@ struct ast_frame *ast_rtp_instance_read(struct ast_rtp_instance *instance, int r
  *
  * \since 1.8
  */
-int ast_rtp_instance_set_remote_address(struct ast_rtp_instance *instance, const struct ast_sockaddr *address);
+#define ast_rtp_instance_set_remote_address(instance, address) \
+	ast_rtp_instance_set_requested_target_address((instance), (address));
 
 /*!
  * \brief Set the address that we are expecting to receive RTP on
@@ -963,6 +998,32 @@ void ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance, struc
 int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
 
 /*!
+ * \brief Get the incoming source address of the remote endpoint
+ *
+ * This returns the remote address the engine is sending RTP to. Usually this
+ * will be the same as the requested target address, however in the case where
+ * the engine "learns" the address (for instance, symmetric RTP enabled) this
+ * will then contain the learned address.
+ *
+ * \param instance The instance that we want to get the incoming source address for
+ * \param address A structure to put the address into
+ */
+void ast_rtp_instance_get_incoming_source_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+
+/*!
+ * \brief Get the requested target address of the remote endpoint
+ *
+ * This returns the explicitly set address of a remote endpoint. Meaning this won't change unless
+ * specifically told to change. In most cases this should be the same as the incoming source
+ * address, except in cases where the engine "learns" the address in which case this and the
+ * incoming source address might differ.
+ *
+ * \param instance The instance that we want to get the requested target address for
+ * \param address A structure to put the address into
+ */
+void ast_rtp_instance_get_requested_target_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+
+/*!
  * \brief Get the address of the remote endpoint that we are sending RTP to
  *
  * \param instance The instance that we want to get the remote address for
@@ -980,7 +1041,20 @@ int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance
  *
  * \since 1.8
  */
-void ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+#define ast_rtp_instance_get_remote_address(instance, address) \
+	ast_rtp_instance_get_incoming_source_address((instance), (address));
+
+/*!
+ * \brief Get the requested target address of the remote endpoint and
+ *        compare it to the given address
+ *
+ * \param instance The instance that we want to get the remote address for
+ * \param address An initialized address that may be overwritten addresses differ
+ *
+ * \retval 0 address was not changed
+ * \retval 1 address was changed
+ */
+int ast_rtp_instance_get_and_cmp_requested_target_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
 
 /*!
  * \brief Get the address of the remote endpoint that we are sending RTP to, comparing its address to another
@@ -1003,8 +1077,8 @@ void ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance, stru
  *
  * \since 1.8
  */
-
-int ast_rtp_instance_get_and_cmp_remote_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+#define ast_rtp_instance_get_and_cmp_remote_address(instance, address) \
+	ast_rtp_instance_get_and_cmp_requested_target_address((instance), (address));
 
 /*!
  * \brief Set the value of an RTP instance extended property
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index df04c1571..11fe985cb 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -170,8 +170,10 @@ struct ast_rtp_instance {
 	int properties[AST_RTP_PROPERTY_MAX];
 	/*! Address that we are expecting RTP to come in to */
 	struct ast_sockaddr local_address;
+	/*! The original source address */
+	struct ast_sockaddr requested_target_address;
 	/*! Address that we are sending RTP to */
-	struct ast_sockaddr remote_address;
+	struct ast_sockaddr incoming_source_address;
 	/*! Instance that we are bridged to if doing remote or local bridging */
 	struct ast_rtp_instance *bridged;
 	/*! Payload and packetization information */
@@ -467,20 +469,28 @@ int ast_rtp_instance_set_local_address(struct ast_rtp_instance *instance,
 	return 0;
 }
 
-int ast_rtp_instance_set_remote_address(struct ast_rtp_instance *instance,
-		const struct ast_sockaddr *address)
+int ast_rtp_instance_set_incoming_source_address(struct ast_rtp_instance *instance,
+						 const struct ast_sockaddr *address)
 {
-	ast_sockaddr_copy(&instance->remote_address, address);
+	ast_sockaddr_copy(&instance->incoming_source_address, address);
 
 	/* moo */
 
 	if (instance->engine->remote_address_set) {
-		instance->engine->remote_address_set(instance, &instance->remote_address);
+		instance->engine->remote_address_set(instance, &instance->incoming_source_address);
 	}
 
 	return 0;
 }
 
+int ast_rtp_instance_set_requested_target_address(struct ast_rtp_instance *instance,
+						  const struct ast_sockaddr *address)
+{
+	ast_sockaddr_copy(&instance->requested_target_address, address);
+
+	return ast_rtp_instance_set_incoming_source_address(instance, address);
+}
+
 int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance,
 		struct ast_sockaddr *address)
 {
@@ -498,21 +508,27 @@ void ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance,
 	ast_sockaddr_copy(address, &instance->local_address);
 }
 
-int ast_rtp_instance_get_and_cmp_remote_address(struct ast_rtp_instance *instance,
+int ast_rtp_instance_get_and_cmp_requested_target_address(struct ast_rtp_instance *instance,
 		struct ast_sockaddr *address)
 {
-	if (ast_sockaddr_cmp(address, &instance->remote_address) != 0) {
-		ast_sockaddr_copy(address, &instance->remote_address);
+	if (ast_sockaddr_cmp(address, &instance->requested_target_address) != 0) {
+		ast_sockaddr_copy(address, &instance->requested_target_address);
 		return 1;
 	}
 
 	return 0;
 }
 
-void ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance,
-		struct ast_sockaddr *address)
+void ast_rtp_instance_get_incoming_source_address(struct ast_rtp_instance *instance,
+						  struct ast_sockaddr *address)
+{
+	ast_sockaddr_copy(address, &instance->incoming_source_address);
+}
+
+void ast_rtp_instance_get_requested_target_address(struct ast_rtp_instance *instance,
+						   struct ast_sockaddr *address)
 {
-	ast_sockaddr_copy(address, &instance->remote_address);
+	ast_sockaddr_copy(address, &instance->requested_target_address);
 }
 
 void ast_rtp_instance_set_extended_prop(struct ast_rtp_instance *instance, int property, void *value)
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 818f1c05b..1d7ccd9dc 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4332,7 +4332,8 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc
 	/* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */
 	if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {
 		if (ast_sockaddr_cmp(&remote_address, &addr)) {
-			ast_rtp_instance_set_remote_address(instance, &addr);
+			/* do not update the originally given address, but only the remote */
+			ast_rtp_instance_set_incoming_source_address(instance, &addr);
 			ast_sockaddr_copy(&remote_address, &addr);
 			if (rtp->rtcp) {
 				ast_sockaddr_copy(&rtp->rtcp->them, &addr);