netfilter: nf_conntrack_expect: restore helper propagation via expectation

A recent series to fix expectations broke helper propagation via
expectation, this mechanism is used by the sip and h323 helper. This
also propagates the conntrack helper to expected connections. I changed
semantics of exp->helper which now tells us the actual helper that
created the expectation.

Add an explicit assign_helper field to expectations for this purpose
and update helpers to use it.

Restore this feature for userspace conntrack helper via ctnetlink
nfqueue integration so it is again possible to attach a helper to an
expectation, where it makes sense. This is not restored via ctnetlink
expectation creation as there is no client for such feature. Use the
expectation layer 4 protocol number for the helper lookup for
consistency.

Make sure the expectation using this helper propagation mechanism also
go away when the helper is unregistered.

Fixes: 9c42bc9db90a ("netfilter: nf_conntrack_expect: honor expectation helper field")
Fixes: 917b61fa2042 ("netfilter: ctnetlink: ignore explicit helper on new expectations")
Reported-by: Ilya Maximets <i.maximets@ovn.org>
Tested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/netfilter/nf_conntrack_expect.h b/include/net/netfilter/nf_conntrack_expect.h
index e9a8350e7ccfb030eb79951f9ac1dc457325a040..80f50fd0f7ad270768ccde689aae2bf5556b24e4 100644 (file)
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -45,9 +45,12 @@ struct nf_conntrack_expect {
        void (*expectfn)(struct nf_conn *new,
                         struct nf_conntrack_expect *this);
 
-       /* Helper to assign to new connection */
+       /* Helper that created this expectation */
        struct nf_conntrack_helper __rcu *helper;
 
+       /* Helper to assign to new connection */
+       struct nf_conntrack_helper __rcu *assign_helper;
+
        /* The conntrack of the master connection */
        struct nf_conn *master;
 

diff --git a/net/netfilter/nf_conntrack_broadcast.c b/net/netfilter/nf_conntrack_broadcast.c
index 4f39bf7c843f2df0a55443f187a17eb3c5d6fc12..75e53fde6b29748dc75c4d3a5cd5f16e3021c064 100644 (file)
--- a/net/netfilter/nf_conntrack_broadcast.c
+++ b/net/netfilter/nf_conntrack_broadcast.c
@@ -72,6 +72,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb,
        exp->flags                = NF_CT_EXPECT_PERMANENT;
        exp->class                = NF_CT_EXPECT_CLASS_DEFAULT;
        rcu_assign_pointer(exp->helper, helper);
+       rcu_assign_pointer(exp->assign_helper, NULL);
        write_pnet(&exp->net, net);
 #ifdef CONFIG_NF_CONNTRACK_ZONES
        exp->zone = ct->zone;

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index b081892263201c9be48b9350f420eae1d1a2ab1f..8ba5b22a1eef2fcc5cba2b5a3e54072a6fac6165 100644 (file)
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1811,14 +1811,17 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
                spin_lock_bh(&nf_conntrack_expect_lock);
                exp = nf_ct_find_expectation(net, zone, tuple, !tmpl || nf_ct_is_confirmed(tmpl));
                if (exp) {
+                       struct nf_conntrack_helper *assign_helper;
+
                        /* Welcome, Mr. Bond.  We've been expecting you... */
                        __set_bit(IPS_EXPECTED_BIT, &ct->status);
                        /* exp->master safe, refcnt bumped in nf_ct_find_expectation */
                        ct->master = exp->master;
-                       if (exp->helper) {
+                       assign_helper = rcu_dereference(exp->assign_helper);
+                       if (assign_helper) {
                                help = nf_ct_helper_ext_add(ct, GFP_ATOMIC);
                                if (help)
-                                       rcu_assign_pointer(help->helper, exp->helper);
+                                       rcu_assign_pointer(help->helper, assign_helper);
                        }
 
 #ifdef CONFIG_NF_CONNTRACK_MARK

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 24d0576d84b7f60f3bcec7333ff2bfae1b4158b2..8e943efbdf0a5292b0dc202f901eeecab7c66ccb 100644 (file)
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -344,6 +344,7 @@ void nf_ct_expect_init(struct nf_conntrack_expect *exp, unsigned int class,
                helper = rcu_dereference(help->helper);
 
        rcu_assign_pointer(exp->helper, helper);
+       rcu_assign_pointer(exp->assign_helper, NULL);
        write_pnet(&exp->net, net);
 #ifdef CONFIG_NF_CONNTRACK_ZONES
        exp->zone = ct->zone;

diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
index 3f5c50455b716ac49299ff9f56994a222d5cabcb..b2fe6554b9cf434b7dd6eb2becd8d6c802ac8940 100644 (file)
--- a/net/netfilter/nf_conntrack_h323_main.c
+++ b/net/netfilter/nf_conntrack_h323_main.c
@@ -643,7 +643,7 @@ static int expect_h245(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3,
                          &ct->tuplehash[!dir].tuple.dst.u3,
                          IPPROTO_TCP, NULL, &port);
-       rcu_assign_pointer(exp->helper, &nf_conntrack_helper_h245);
+       rcu_assign_pointer(exp->assign_helper, &nf_conntrack_helper_h245);
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
        if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
@@ -767,7 +767,7 @@ static int expect_callforwarding(struct sk_buff *skb,
        nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, nf_ct_l3num(ct),
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
-       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
+       rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_q931);
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
        if (memcmp(&ct->tuplehash[dir].tuple.src.u3,
@@ -1234,7 +1234,7 @@ static int expect_q931(struct sk_buff *skb, struct nf_conn *ct,
                                &ct->tuplehash[!dir].tuple.src.u3 : NULL,
                          &ct->tuplehash[!dir].tuple.dst.u3,
                          IPPROTO_TCP, NULL, &port);
-       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
+       rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_q931);
        exp->flags = NF_CT_EXPECT_PERMANENT;    /* Accept multiple calls */
 
        nathook = rcu_dereference(nfct_h323_nat_hook);
@@ -1306,7 +1306,7 @@ static int process_gcf(struct sk_buff *skb, struct nf_conn *ct,
        nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, nf_ct_l3num(ct),
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_UDP, NULL, &port);
-       rcu_assign_pointer(exp->helper, nf_conntrack_helper_ras);
+       rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_ras);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect RAS ");
@@ -1523,7 +1523,7 @@ static int process_acf(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
        exp->flags = NF_CT_EXPECT_PERMANENT;
-       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
+       rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_q931);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect Q.931 ");
@@ -1577,7 +1577,7 @@ static int process_lcf(struct sk_buff *skb, struct nf_conn *ct,
                          &ct->tuplehash[!dir].tuple.src.u3, &addr,
                          IPPROTO_TCP, NULL, &port);
        exp->flags = NF_CT_EXPECT_PERMANENT;
-       rcu_assign_pointer(exp->helper, nf_conntrack_helper_q931);
+       rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_q931);
 
        if (nf_ct_expect_related(exp, 0) == 0) {
                pr_debug("nf_ct_ras: expect Q.931 ");

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index a715304a53d8c2b9e7a54bc8073a9a7aff1ad318..b594cd244fe1d459adc24ca70b618a9d09b67648 100644 (file)
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -400,6 +400,11 @@ static bool expect_iter_me(struct nf_conntrack_expect *exp, void *data)
 
        this = rcu_dereference_protected(exp->helper,
                                         lockdep_is_held(&nf_conntrack_expect_lock));
+       if (this == me)
+               return true;
+
+       this = rcu_dereference_protected(exp->assign_helper,
+                                        lockdep_is_held(&nf_conntrack_expect_lock));
        return this == me;
 }
 

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index eda5fe4a75c827ce9f902a132e2652af8aff30cc..d7209d1241114581663dfbfa3fcba254db90a617 100644 (file)
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2634,6 +2634,7 @@ static const struct nla_policy exp_nla_policy[CTA_EXPECT_MAX+1] = {
 
 static struct nf_conntrack_expect *
 ctnetlink_alloc_expect(const struct nlattr *const cda[], struct nf_conn *ct,
+                      const struct nf_conntrack_helper *assign_helper,
                       struct nf_conntrack_tuple *tuple,
                       struct nf_conntrack_tuple *mask);
 
@@ -2860,6 +2861,7 @@ static int
 ctnetlink_glue_attach_expect(const struct nlattr *attr, struct nf_conn *ct,
                             u32 portid, u32 report)
 {
+       struct nf_conntrack_helper *assign_helper = NULL;
        struct nlattr *cda[CTA_EXPECT_MAX+1];
        struct nf_conntrack_tuple tuple, mask;
        struct nf_conntrack_expect *exp;
@@ -2875,8 +2877,18 @@ ctnetlink_glue_attach_expect(const struct nlattr *attr, struct nf_conn *ct,
        if (err < 0)
                return err;
 
+       if (cda[CTA_EXPECT_HELP_NAME]) {
+               const char *helpname = nla_data(cda[CTA_EXPECT_HELP_NAME]);
+
+               assign_helper = __nf_conntrack_helper_find(helpname,
+                                                          nf_ct_l3num(ct),
+                                                          tuple.dst.protonum);
+               if (!assign_helper)
+                       return -EOPNOTSUPP;
+       }
+
        exp = ctnetlink_alloc_expect((const struct nlattr * const *)cda, ct,
-                                    &tuple, &mask);
+                                    assign_helper, &tuple, &mask);
        if (IS_ERR(exp))
                return PTR_ERR(exp);
 
@@ -3515,6 +3527,7 @@ ctnetlink_parse_expect_nat(const struct nlattr *attr,
 
 static struct nf_conntrack_expect *
 ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
+                      const struct nf_conntrack_helper *assign_helper,
                       struct nf_conntrack_tuple *tuple,
                       struct nf_conntrack_tuple *mask)
 {
@@ -3568,6 +3581,7 @@ ctnetlink_alloc_expect(const struct nlattr * const cda[], struct nf_conn *ct,
        exp->zone = ct->zone;
 #endif
        rcu_assign_pointer(exp->helper, helper);
+       rcu_assign_pointer(exp->assign_helper, assign_helper);
        exp->tuple = *tuple;
        exp->mask.src.u3 = mask->src.u3;
        exp->mask.src.u.all = mask->src.u.all;
@@ -3623,7 +3637,7 @@ ctnetlink_create_expect(struct net *net,
        ct = nf_ct_tuplehash_to_ctrack(h);
 
        rcu_read_lock();
-       exp = ctnetlink_alloc_expect(cda, ct, &tuple, &mask);
+       exp = ctnetlink_alloc_expect(cda, ct, NULL, &tuple, &mask);
        if (IS_ERR(exp)) {
                err = PTR_ERR(exp);
                goto err_rcu;

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 1eb55907d470d612cafd60dbba8862305e21276e..d24bfa9e823448fbbeb906896f666c7f9cdc512c 100644 (file)
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1383,7 +1383,7 @@ static int process_register_request(struct sk_buff *skb, unsigned int protoff,
        nf_ct_expect_init(exp, SIP_EXPECT_SIGNALLING, nf_ct_l3num(ct),
                          saddr, &daddr, proto, NULL, &port);
        exp->timeout.expires = sip_timeout * HZ;
-       rcu_assign_pointer(exp->helper, helper);
+       rcu_assign_pointer(exp->assign_helper, helper);
        exp->flags = NF_CT_EXPECT_PERMANENT | NF_CT_EXPECT_INACTIVE;
 
        hooks = rcu_dereference(nf_nat_sip_hooks);