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 2

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 267492 New Change
[20] 9772 lines
[+20] [+] static void add_noncodec_to_sdp(const struct sip_pvt *p, int format,
9773
				  struct sockaddr_in *sin, struct sockaddr_in *vsin, struct sockaddr_in *tsin,
9773
				  struct sockaddr_in *sin, struct sockaddr_in *vsin, struct sockaddr_in *tsin,
9774
				  struct sockaddr_in *dest, struct sockaddr_in *vdest, struct sockaddr_in *tdest)
9774
				  struct sockaddr_in *dest, struct sockaddr_in *vdest, struct sockaddr_in *tdest)
9775
{
9775
{
9776
	/* First, get our address */
9776
	/* First, get our address */
9777
	ast_rtp_instance_get_local_address(p->rtp, sin);
9777
	ast_rtp_instance_get_local_address(p->rtp, sin);
9778
	if (p->vrtp)
9778
	if ((p->ourip.sin_addr.s_addr && (sin->sin_addr.s_addr != p->ourip.sin_addr.s_addr)))

    
   
9779
		sin->sin_addr.s_addr = p->ourip.sin_addr.s_addr;

    
   
9780
	if (p->vrtp) {
9779
		ast_rtp_instance_get_local_address(p->vrtp, vsin);
9781
		ast_rtp_instance_get_local_address(p->vrtp, vsin);
9780
	if (p->trtp)
9782
		if ((p->ourip.sin_addr.s_addr && (vsin->sin_addr.s_addr != p->ourip.sin_addr.s_addr)))

    
   
9783
			vsin->sin_addr.s_addr = p->ourip.sin_addr.s_addr;

    
   
9784
	}

    
   
9785
	if (p->trtp) {
9781
		ast_rtp_instance_get_local_address(p->trtp, tsin);
9786
		ast_rtp_instance_get_local_address(p->trtp, tsin);

    
   
9787
		if ((p->ourip.sin_addr.s_addr && (tsin->sin_addr.s_addr != p->ourip.sin_addr.s_addr)))

    
   
9788
			tsin->sin_addr.s_addr = p->ourip.sin_addr.s_addr;

    
   
9789
	}
9782

    
   
9790

   
9783
	/* Now, try to figure out where we want them to send data */
9791
	/* Now, try to figure out where we want them to send data */
9784
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
9792
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
9785
	if (p->redirip.sin_addr.s_addr) {	/* If we have a redirection IP, use it */
9793
	if (p->redirip.sin_addr.s_addr) {	/* If we have a redirection IP, use it */
9786
		dest->sin_port = p->redirip.sin_port;
9794
		dest->sin_port = p->redirip.sin_port;
[+20] [20] 17731 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.