On Thu, Mar 14, 2019 at 4:27 AM xiao ruizhu <katrina.xia...@gmail.com> wrote:
>
> An unexpected RTCP expectation clash happens in the following scenario.
>
>            |      INVITE SDP (g711A g739 telephone-event)      |
>       5060 |<--------------------------------------------------|5060
>            |                 100 Trying                        |
>       5060 |-------------------------------------------------->|5060
>   S        |  183 Session Progress SDP (g711A telephone-event) |
>       5060 |-------------------------------------------------->|5060
>   E        |                    PRACK                          |      C
>      50601 |<--------------------------------------------------|5060
>   R        |                  200 OK (PRACK)                   |      P
>      50601 |-------------------------------------------------->|5060
>   V        |                  200 OK (INVITE)                  |      E
>       5060 |-------------------------------------------------->|5060
>   E        |                      ACK                          |
>      50601 |<--------------------------------------------------|5060
>   R        |                                                   |
>            |<------------------ RTP stream ------------------->|
>            |                                                   |
>            |                 INVITE SDP (t38)                  |
>      50601 |-------------------------------------------------->|5060
>
> With a certain configuration in the CPE, SIP messages "183 with SDP" and
> "re-INVITE with SDP t38" in the scenario will trigger the ALG to create
> expectations for RTP and RTCP.
>
> It is okay to create RTP&RTCP expectations for "183", whose master
> connection source port is 5060, and destination port is 5060.
>
> In this "183" message, port in Contact header changes to 50601 (from the
> original 5060). So the following requests e.g. PRACK are sent to port
> 50601. It has a different master connection (let call 2nd master
> connection) from the original INVITE (let call 1st master connection) due
> to the port difference.
>
> When "re-INVITE with SDP t38" arrives to create RTP&RTCP expectations,
> current ALG implementation will firstly check whether there is a RTP
> expectation with the same tuples already exists for a different master
> connection. If yes, it will skip RTP&RTCP expects creation; otherwise it
> will create a new RTP&RTCP expectation pairs.
>
> In the scenario above, there is RTP stream but no RTCP stream for the 1st
> master connection, so the RTP expectation created upon "183" is cleared,
> and RTCP expectation created for the 1st master connection retains.
>
> Basing on current ALG implementation, it only checks RTP expectation
> existence but not for RTCP. So when "re-INVITE with SDP t38" arrives, it
> will continue to create a new RTP&RTCP expectation pairs for the 2nd master
> connection, which will result in a detection of expectation clash for RTCP
> (same tuples and different master), and then a failure in processing of
> that re-INVITE.
>
> The fixing here is to check both RTP and RTCP expectation existence before
> creation. When there is an old expectation with same tuples for a different
> master, release the old expectation. Then a new one will be created for the
> current master.
>
> Signed-off-by: xiao ruizhu <katrina.xia...@gmail.com>
> ---
> Changes in v2:
> - add a comment on release_conflicting_expect functionality
> - move local variable errp to the beginning of the block
> v1:
> - original patch
> ---
>  net/netfilter/nf_conntrack_sip.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_sip.c 
> b/net/netfilter/nf_conntrack_sip.c
> index f067c6b..4398a53 100644
> --- a/net/netfilter/nf_conntrack_sip.c
> +++ b/net/netfilter/nf_conntrack_sip.c
> @@ -799,6 +799,23 @@ static int ct_sip_parse_sdp_addr(const struct nf_conn 
> *ct, const char *dptr,
>         return 1;
>  }
>
> +/* Remove conflicting expect created by sip helper for another master
> + * conntrack, most likely related to this master.
> + */
> +static void release_conflicting_expect(const struct nf_conn *ct,
> +                                      const struct nf_conntrack_expect 
> *expect,
> +                                      const enum sip_expectation_classes 
> class)
> +{
> +       struct nf_conntrack_expect *exp;
> +       struct net *net = nf_ct_net(ct);
> +
> +       exp = __nf_ct_expect_find(net, nf_ct_zone(ct), &expect->tuple);
> +       if (exp && exp->master != ct &&
> +           nfct_help(exp->master)->helper == nfct_help(ct)->helper &&
> +           exp->class == class)
> +               nf_ct_unexpect_related(exp);
> +}
> +
>  static int refresh_signalling_expectation(struct nf_conn *ct,
>                                           union nf_inet_addr *addr,
>                                           u8 proto, __be16 port,
> @@ -980,11 +997,16 @@ static int set_expected_rtp_rtcp(struct sk_buff *skb, 
> unsigned int protoff,
>                                        datalen, rtp_exp, rtcp_exp,
>                                        mediaoff, medialen, daddr);
>         else {
> +               int errp;
> +
> +               release_conflicting_expect(ct, rtp_exp, class);
> +               release_conflicting_expect(ct, rtcp_exp, class);
> +
>                 /* -EALREADY handling works around end-points that send
>                  * SDP messages with identical port but different media type,
>                  * we pretend expectation was set up.
>                  */
> -               int errp = nf_ct_expect_related(rtp_exp);
> +               errp = nf_ct_expect_related(rtp_exp);
>
>                 if (errp == 0 || errp == -EALREADY) {
>                         int errcp = nf_ct_expect_related(rtcp_exp);
> --
> 1.9.1
>

Acked-by: Alin Nastac <alin.nas...@gmail.com>

Reply via email to