Review Board 1.7.16


Use the correct IP address in the c and o sdp lines when using an externally mapped IP address

Review Request #566 - Created March 16, 2010 and submitted

ebroad
/trunk
17044
Reviewers
asterisk-dev
Asterisk
The issue stems from a call to ast_rtp_instance_get_local_address() in chan_sip.c in get_our_media_address(). ast_rtp_instance_get_local_address() will always return the internal IP address of the Asterisk machine, which could pose a problem when using NAT.

A simple solution would be to override {v,t}sin->sin_addr in get_our_media_address() with p->ourip.sin_addr.s_addr if it is different(which is what the attached diff does), though I don't know what that might break.

Edit 1: This patch short-circuits some logic later on in get_our_media_address(). ast_rtp_instance_get_local_address() does exactly as the function name suggests, it gets the local address of the machine the Asterisk is running on, which is bad for NAT setups. Should we be calling this in get_our_media_address() or instead just use p->ourip? or should we try to stick with letting the RTP engine decide, and modifying ast_rtp_instance_get_local_address() to return our externally mapped IP when appropriate, which would break from what the function is supposed to do? Comments anyone? 
Applied to trunk, compiled, fax and voice calls made to and from Asterisk successfully.

Diff revision 3

This is not the most recent revision of the diff. The latest diff is revision 4. See what's changed.

1 2 3 4
1 2 3 4

  1. /trunk/channels/chan_sip.c: Loading...
/trunk/channels/chan_sip.c
Revision 280943 New Change
[20] 10268 lines
[+20] [+] static void get_our_media_address(struct sip_pvt *p, int needvideo, int needtext,
10269
	}
10269
	}
10270
	if (p->trtp) {
10270
	if (p->trtp) {
10271
		ast_rtp_instance_get_local_address(p->trtp, taddr);
10271
		ast_rtp_instance_get_local_address(p->trtp, taddr);
10272
	}
10272
	}
10273

    
   
10273

   

    
   
10274
	/* If our real IP differs from our internal IP, i.e. NAT, use that for media_address if media_address isn't explicitly set */

    
   
10275
	if ((ast_sockaddr_isnull(&media_address) && ast_sockaddr_cmp_addr(addr, &p->ourip))) {

    
   
10276
		ast_sockaddr_copy(&media_address, &p->ourip); 

    
   
10277
	}

    
   
10278

   
10274
	/* Now, try to figure out where we want them to send data */
10279
	/* Now, try to figure out where we want them to send data */
10275
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
10280
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
10276
	if (!ast_sockaddr_isnull(&p->redirip)) {	/* If we have a redirection IP, use it */
10281
	if (!ast_sockaddr_isnull(&p->redirip)) {	/* If we have a redirection IP, use it */
10277
		ast_sockaddr_copy(dest, &p->redirip);
10282
		ast_sockaddr_copy(dest, &p->redirip);
10278
	} else {
10283
	} else {
[+20] [20] 18223 lines
  1. /trunk/channels/chan_sip.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.