On Fri, Feb 15, 2019 at 09:37:00PM +0100, Alin Năstac wrote: > Hi Pablo, > > On Fri, Feb 15, 2019 at 4:07 PM Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > > Hi Alin, > > > > On Fri, Feb 15, 2019 at 02:20:14PM +0100, Alin Năstac wrote: > > > On Fri, Feb 15, 2019 at 1:02 PM Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > [...] > > > > On Mon, Dec 24, 2018 at 08:15:19AM +0100, Alin Nastac wrote: > > > > > When enabled, the sip_external_media logic will leave SDP > > > > > payload untouched when it detects that interface towards INVITEd > > > > > party is the same with the one towards media endpoint. > > > > > > > > > > The typical scenario for this logic is when a LAN SIP agent has more > > > > > than one IP address (uses a different address for media streams than > > > > > the one used on signalling stream) and it also forwards calls to a > > > > > voice mailbox located on the WAN side. In such case sip_direct_media > > > > > must be disabled (so normal calls could be handled by the SIP > > > > > helper), but media streams that are not traversing this router must > > > > > also be excluded from address translation (e.g. call forwards). > > > > > > > > This patch got stuck in my queue right before holidays. I'm very sorry > > > > about that. > > > > > > > > Still one more question: Now that we have explicit helper assignment > > > > via rule, and assuming automatic helper assignment is deprecated > > > > (actually, disabled by default these days since it is unsecure [1]). > > > > > > > > Would it be possible to skip this via explicit ruleset policy? > > > > > > Parameters such as sip_direct_signalling and sip_external_media > > > (latter being implemented in this patch) are global switches. > > > I guess we can implement them as sip helper parameters configurable > > > through the rule that enables the helper, but I haven't found yet a > > > helper that has such parameters ("-j CT --helper xxx" rules don't > > > allow passing any additional helper parameters). Probably their > > > values will have to be stored in nf_ct_sip_master struct associated > > > with the master conntrack. > > > > Hm, I was wondering if we could restrict the rule that comes with "-j > > CT --helper xxx" to skip these flows, but we need to inspect them to > > classify them as media flow... right? Given you check for the route to > > see if the media endpoint is reachable through the same interface, I'm > > thinking if it's possible to make it from the policy side. If helper > > is the only place where we can do this, that's fine too, I'm just > > exploring :-). > > Actually almost all RELATED conntracks are media streams except the > one created at registration time with dport set to the WAN side port > of the master conntrack (usually 5060). > > However, in this patch I don't try to block media streams that > traverse the box, The goal here is to avoid mangling SDP payload that > have to be forwarded as it was transmitted by the LAN host, to allow > media streams between 2 WAN side endpoints to contact each other > directly. So you see this decision cannot be taken outside the helper > context, because it is not the kind of ACCEPT/DROP dilemma. Signalling > packet needs to be always ACCEPTed, but its SDP payload need to be > translated only when the IP address transmitted by the LAN host as its > media stream address is behind the NAT box. > > In other words, sip_external_media=1 is modifying the > sip_direct_media=0 behavior: > - master conntrack NATed, sip_direct_media=0, sip_external_media=0 : > SDP payload is always mangled by nf_nat_sip.c > - master conntrack NATed, sip_direct_media=0, sip_external_media=1 : > SDP payload is mangled when new media stream(s) will traverse the box Applied, thanks for explaining.