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 1

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 252928 New Change
[20] 8783 lines
[+20] [+] static void add_noncodec_to_sdp(const struct sip_pvt *p, int format,
8784
				  struct sockaddr_in *sin, struct sockaddr_in *vsin, struct sockaddr_in *tsin,
8784
				  struct sockaddr_in *sin, struct sockaddr_in *vsin, struct sockaddr_in *tsin,
8785
				  struct sockaddr_in *dest, struct sockaddr_in *vdest, struct sockaddr_in *tdest)
8785
				  struct sockaddr_in *dest, struct sockaddr_in *vdest, struct sockaddr_in *tdest)
8786
{
8786
{
8787
	/* First, get our address */
8787
	/* First, get our address */
8788
	ast_rtp_instance_get_local_address(p->rtp, sin);
8788
	ast_rtp_instance_get_local_address(p->rtp, sin);
8789
	if (p->vrtp)
8789
	if (p->ourip.sin_addr.s_addr && (sin->sin_addr.s_addr != p->ourip.sin_addr.s_addr))

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

    
   
8791
	if (p->vrtp) {
8790
		ast_rtp_instance_get_local_address(p->vrtp, vsin);
8792
		ast_rtp_instance_get_local_address(p->vrtp, vsin);
8791
	if (p->trtp)
8793
		if (p->ourip.sin_addr.s_addr && (vsin->sin_addr.s_addr != p->ourip.sin_addr.s_addr))

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

    
   
8795
	}

    
   
8796
	if (p->trtp) {
8792
		ast_rtp_instance_get_local_address(p->trtp, tsin);
8797
		ast_rtp_instance_get_local_address(p->trtp, tsin);

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

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

    
   
8800
	}
8793

    
   
8801

   
8794
	/* Now, try to figure out where we want them to send data */
8802
	/* Now, try to figure out where we want them to send data */
8795
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
8803
	/* Is this a re-invite to move the media out, then use the original offer from caller  */
8796
	if (p->redirip.sin_addr.s_addr) {	/* If we have a redirection IP, use it */
8804
	if (p->redirip.sin_addr.s_addr) {	/* If we have a redirection IP, use it */
8797
		dest->sin_port = p->redirip.sin_port;
8805
		dest->sin_port = p->redirip.sin_port;
[+20] [20] 16554 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.