Re: [PATCH] netfilter: nf_nat_sip: fix RTP/RTCP source port translations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pablo,
On Wed, Dec 12, 2018 at 5:13 PM Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> 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.

That's true, but signalling expectations do not have a paired
expectation anyway.
If you want to exclude this logic for them, it is enough to condition
it with exp->helper != nfct_help(ct)->helper.

What variant do you prefer?

>
> > > > > +                     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
> > > > >




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux