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 e9a8350..80f50fd 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 4f39bf7..75e53fd 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 b081892..8ba5b22 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 24d0576..8e943ef 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 3f5c504..b2fe655 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 a715304..b594cd2 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 eda5fe4..d7209d1 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 1eb5590..d24bfa9 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);