On Thu, Mar 14, 2019 at 4:27 AM xiao ruizhu <katrina.xiaorz@xxxxxxxxx> 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.xiaorz@xxxxxxxxx>
> ---
> 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.nastac@xxxxxxxxx>
