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/

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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index a2312e7745b7b1523f9fad79b75fd40192663a6d..a3d8bdcffd6dc5fc3611b6cf0dd528820598121a 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -23323,7 +23323,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);
@@ -32550,7 +32550,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 81e530e85b94aec1496a632a118242e48ca2b0bb..e687da5205362c0e31d8db2572889e89c5c8a801 100644 (file)
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -854,6 +854,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
  *
@@ -874,7 +908,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
@@ -941,6 +976,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
  *
@@ -959,7 +1020,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
@@ -982,8 +1056,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 3a114e3d7abcbf9de5890c10c181429895b2b767..9c633094a2b0ea26a582bf2e9f931eb065d0b5c3 100644 (file)
--- 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 */
@@ -443,20 +445,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)
 {
@@ -474,21 +484,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 b986aee6724d52a637cf6cc8d3dbadb6c91a00d2..c1ce86493bd85326107bd84f09d816c638a7be4d 100644 (file)
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -4321,7 +4321,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);