[asterisk-dev] [Code Review]: Pimp SIP Nat

[Mark Michelson]

> On March 4, 2013, 12:39 p.m., Mark Michelson wrote:
> > /team/group/pimp_my_sip/res/res_sip_nat.c, lines 178-191
> > <https://reviewboard.asterisk.org/r/2363/diff/2/?file=33727#file33727line178>
> >
> >     I don't particularly like this block. Mainly, I don't think this will scale well when people want support for other types of bodies that contain addresses in them. This code also will fail to update SDP if the SDP is part of a multi-part body.
> >     
> >     This should be replaced with something similar to the SDP handler code in res_sip_session.c. Modules that handle message bodies that contain address information register themselves with the core. Here, we iterate over each part of the body, and pass the body part off to the handlers that know how to rewrite addresses. If the handler returns successful, it rewrote the address. If it returns unsuccessful, then it doesn't know how to handle this type of body so move on to the next one.

I've just thought of another potential issue here. Obviously we don't have direct media support in the new SIP work yet, but when we do, SDP address re-writing will have to not try to rewrite remote addresses that are for directmedia. We'll only want to overwrite our local address with the external IP. This is another potentially good reason to leave address rewriting in the body of a SIP message out of the NAT module since the NAT module would be forced to know of such nuances as direct media addresses.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2363/#review7977
-----------------------------------------------------------


On March 4, 2013, 9:41 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2363/
> -----------------------------------------------------------
> 
> (Updated March 4, 2013, 9:41 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This change adds a new module, res_sip_nat, which performs rewriting of outgoing messages for cases where address information needs to be replaced when Asterisk is behind NAT. It also performs rewriting of incoming messages for cases where the remote party may be behind NAT. This change also adds ICE support.
> 
> 
> Diffs
> -----
> 
>   /team/group/pimp_my_sip/include/asterisk/res_sip.h 382367 
>   /team/group/pimp_my_sip/res/res_sip/config_transport.c 382367 
>   /team/group/pimp_my_sip/res/res_sip/sip_configuration.c 382367 
>   /team/group/pimp_my_sip/res/res_sip_nat.c PRE-CREATION 
>   /team/group/pimp_my_sip/res/res_sip_sdp_audio.c 382367 
> 
> Diff: https://reviewboard.asterisk.org/r/2363/diff
> 
> 
> Testing
> -------
> 
> Tested Asterisk behind NAT with proper configuration, as well as devices behind NAT. Also tested ICE negotiation.
> 
> 
> Thanks,
> 
> Joshua
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130304/f5bcaafa/attachment.htm>