Hi Alin,
On Tue, Dec 11, 2018 at 08:27:45AM +0100, Alin Năstac wrote:
> On Tue, Dec 11, 2018 at 6:41 AM Alin Năstac <alin.nastac@xxxxxxxxx> wrote:
[...]
> > > Alin Nastac said:
> > > > The scenario fixed by this patch involves a regular SIP call, but
> > > > one that requires port translation for the RTP conntrack. For
> > > > instance, suppose you have 2 SIP agents in the LAN, both connected
> > > > to the same SIP proxy:
> > > > - agent S1 starts first and its REGISTER phase will create a
> > > > permanent expected conntrack on dport 5060 for allowing SIP
> > > > packets to be forwarded to S1 regardless of their source IP
> > > > address or port
> > > > - on agent S2 registration, its permanent expected conntrack will
> > > > confict with the S1 signalling expected conntrack, so it will be
> > > > translated to port 1024
> > > >
> > > > When S1 initiates a call using RTP/RTCP port range 1024-1025, SIP
> > > > helper will find that port 1024 is taken over by S2's signalling
> > > > expected conntrack, so it translates it to port range 1026-1027. All
> > > > goes well if the RTP conntrack is initiated by a packet originated
> > > > from the SIP proxy, but when the first RTP packet is sent by S1
> > > > (usually the peer that initiates the call is the one that sends the
> > > > first RTP packet), it is sent towards SIP proxy with unmodified
> > > > source port (1024 iso 1026).
> > > -- end of quote --
[...]
> > > > @@ -316,6 +317,9 @@ static void nf_nat_sip_seq_adjust(struct sk_buff *skb, unsigned int protoff,
> > > > static void nf_nat_sip_expected(struct nf_conn *ct,
> > > > struct nf_conntrack_expect *exp)
> > > > {
> > > > + struct nf_conn_help *help = nfct_help(ct->master);
> > > > + struct nf_conntrack_expect *pair_exp;
> > > > + int range_set_for_snat = 0;
> > > > struct nf_nat_range2 range;
> > > >
> > > > /* This must be a fresh one. */
> > > > @@ -327,15 +331,38 @@ static void nf_nat_sip_expected(struct nf_conn *ct,
> > > > range.min_addr = range.max_addr = exp->saved_addr;
> > > > nf_nat_setup_info(ct, &range, NF_NAT_MANIP_DST);
> > > >
> > > > - /* Change src to where master sends to, but only if the connection
> > > > - * actually came from the same source. */
> > > > - if (nf_inet_addr_cmp(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3,
> > > > + /* Do SRC manip according with the parameters found in the
> > > > + * paired expected conntrack. */
> > > > + spin_lock_bh(&nf_conntrack_expect_lock);
> > > > + hlist_for_each_entry(pair_exp, &help->expectations, lnode) {
> > > > + if (pair_exp->tuple.src.l3num == nf_ct_l3num(ct) &&
> > > > + pair_exp->tuple.dst.protonum == ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum &&
> > > > + nf_inet_addr_cmp(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3, &pair_exp->saved_addr) &&
> > > > + ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u.all == pair_exp->saved_proto.all) {
> > >
> > > Let me rephrase the scenario with my own words, I'm assuming that:
> > >
> > > #1 S1 agent creates expectation via REGISTER: SIP proxy (SP) -> S1.
> > > using dport 5060 for the expectation.
> > > #2 S2 agent creates expectation via REGISTER: SP -> S2 with dport 1024
> > > for the expectation.
> > > #3 S1 creates expectation via INVITE to SP: SP -> S1 for RTP/RCTP traffic.
> > > As you said "all goes well if RTP conntrack is initiated by a
> > > packet originated from the SP", which makes sense since this
> > > traffic matches the expectation.
> > > #4 However, if first RTP packet is coming from S1, it does not match
> > > any expectation, so it goes through the firewall with no mangling -
> > > as you described. But, in such case, given we do not match an
> > > expectation, nf_nat_sip_expected() is not called.
> > >
> > > So what am I missing to exercise your new code from nf_nat_sip_expected()?
> >
> > An INVITE creates an expectation for the RTP packets sent by the party
> > that receives the INVITE, while the reply to the INVITE creates
> > another expectation for the RTP packets sent by the call initiator.
> > These expectations are designed to perform the necessary DNAT, so the
> > expectations created by SIP packets sent from LAN to WAN are not doing
> > actually any translation (source IP address is translated anyway by
> > the MASQUERADE/SNAT rule). However, when RTP port of the LAN client
> > requires translation (source port that is, e.g. from 1024 to 1026),
> > the current version of nf_nat_sip_expected doesn't deliver it.
> >
> > There is no problem when the WAN party is the one that sends the first
> > RTP packet because the LAN party's RTP packets will match the
> > conntrack created by the WAN party's RTP packets and therefore the
> > source port of the RTP packets sent from the LAN will be translated
> > correctly.
> >
> > However, when LAN client sends the first RTP packet, there is no
> > conntrack created already so there will be no source port translation.
> > And this is where my patch intervenes... It looks for the expectation
> > created by the INVITE's reply (where all the necessary info is stored)
> > and perform the SNAT accordingly.
>
> Correction: It looks for the expectation created by the INVITE (where
> all the necessary info is stored) and perform the SNAT accordingly.
>
> As I said earlier, the LAN RTP packets are hitting the expectation
> created by the peer's reply to the INVITE.
Thanks a lot for explaining.
One thing we could probably do is to add nf_nat_rtp_rtcp_expected()
instead of using the generic nf_nat_sip_expected() which is also
called for the REGISTER case. I think we only need this code below for
the RTP/RTCP case as you explained.
> > > > + range.flags = (NF_NAT_RANGE_MAP_IPS | NF_NAT_RANGE_PROTO_SPECIFIED);
> > > > + range.min_proto.all = range.max_proto.all = pair_exp->tuple.dst.u.all;
> > > > + range.min_addr = range.max_addr = pair_exp->tuple.dst.u3;
> > > > + range_set_for_snat = 1;
> > > > + break;
> > > > + }
> > > > + }
> > > > + spin_unlock_bh(&nf_conntrack_expect_lock);
> > > > +
> > > > + /* When no paired expected conntrack has been found, change src to
> > > > + * where master sends to, but only if the connection actually came
> > > > + * from the same source. */
> > > > + if (!range_set_for_snat &&
> > > > + nf_inet_addr_cmp(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.u3,
> > > > &ct->master->tuplehash[exp->dir].tuple.src.u3)) {
> > > > range.flags = NF_NAT_RANGE_MAP_IPS;
> > > > range.min_addr = range.max_addr
> > > > = ct->master->tuplehash[!exp->dir].tuple.dst.u3;
> > > > - nf_nat_setup_info(ct, &range, NF_NAT_MANIP_SRC);
> > > > + range_set_for_snat = 1;
> > > > }
> > > > +
> > > > + /* Perform SRC manip. */
> > > > + if (range_set_for_snat)
> > > > + nf_nat_setup_info(ct, &range, NF_NAT_MANIP_SRC);
> > > > }
> > > >
> > > > static unsigned int nf_nat_sip_expect(struct sk_buff *skb, unsigned int protoff,
> > > > --
> > > > 2.7.4
> > > >
