netfilter: remove nf_conntrack_helper sysctl and modparam toggles

__nf_ct_try_assign_helper() remains in place but it now requires a
template to configure the helper.

A toggle to disable automatic helper assignment was added by:

  a900689264 ("netfilter: nf_ct_helper: allow to disable automatic helper assignment")

in 2012 to address the issues described in "Secure use of iptables and
connection tracking helpers". Automatic conntrack helper assignment was
disabled by:

  3bb398d925 ("netfilter: nf_ct_helper: disable automatic helper assignment")

back in 2016.

This patch removes the sysctl and modparam toggles, users now have to
rely on explicit conntrack helper configuration via ruleset.

Update tools/testing/selftests/netfilter/nft_conntrack_helper.sh to
check that auto-assignment does not happen anymore.

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
Pablo Neira Ayuso 2022-08-26 08:49:16 +02:00
parent 13a9d08c29
commit b118509076
8 changed files with 37 additions and 107 deletions

View File

@ -53,8 +53,6 @@ struct nf_conntrack_net {
/* only used when new connection is allocated: */
atomic_t count;
unsigned int expect_count;
u8 sysctl_auto_assign_helper;
bool auto_assign_helper_warned;
/* only used from work queues, configuration plane, and so on: */
unsigned int users4;

View File

@ -101,7 +101,6 @@ struct netns_ct {
u8 sysctl_log_invalid; /* Log invalid packets */
u8 sysctl_events;
u8 sysctl_acct;
u8 sysctl_auto_assign_helper;
u8 sysctl_tstamp;
u8 sysctl_checksum;

View File

@ -1782,7 +1782,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
}
spin_unlock_bh(&nf_conntrack_expect_lock);
}
if (!exp)
if (!exp && tmpl)
__nf_ct_try_assign_helper(ct, tmpl, GFP_ATOMIC);
/* Other CPU might have obtained a pointer to this object before it was
@ -2068,10 +2068,6 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
if (ct->master || (help && !hlist_empty(&help->expectations)))
return;
rcu_read_lock();
__nf_ct_try_assign_helper(ct, NULL, GFP_ATOMIC);
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply);
@ -2797,7 +2793,6 @@ int nf_conntrack_init_net(struct net *net)
nf_conntrack_acct_pernet_init(net);
nf_conntrack_tstamp_pernet_init(net);
nf_conntrack_ecache_pernet_init(net);
nf_conntrack_helper_pernet_init(net);
nf_conntrack_proto_pernet_init(net);
return 0;

View File

@ -35,11 +35,6 @@ unsigned int nf_ct_helper_hsize __read_mostly;
EXPORT_SYMBOL_GPL(nf_ct_helper_hsize);
static unsigned int nf_ct_helper_count __read_mostly;
static bool nf_ct_auto_assign_helper __read_mostly = false;
module_param_named(nf_conntrack_helper, nf_ct_auto_assign_helper, bool, 0644);
MODULE_PARM_DESC(nf_conntrack_helper,
"Enable automatic conntrack helper assignment (default 0)");
static DEFINE_MUTEX(nf_ct_nat_helpers_mutex);
static struct list_head nf_ct_nat_helpers __read_mostly;
@ -51,24 +46,6 @@ static unsigned int helper_hash(const struct nf_conntrack_tuple *tuple)
(__force __u16)tuple->src.u.all) % nf_ct_helper_hsize;
}
static struct nf_conntrack_helper *
__nf_ct_helper_find(const struct nf_conntrack_tuple *tuple)
{
struct nf_conntrack_helper *helper;
struct nf_conntrack_tuple_mask mask = { .src.u.all = htons(0xFFFF) };
unsigned int h;
if (!nf_ct_helper_count)
return NULL;
h = helper_hash(tuple);
hlist_for_each_entry_rcu(helper, &nf_ct_helper_hash[h], hnode) {
if (nf_ct_tuple_src_mask_cmp(tuple, &helper->tuple, &mask))
return helper;
}
return NULL;
}
struct nf_conntrack_helper *
__nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum)
{
@ -209,33 +186,11 @@ nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp)
}
EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add);
static struct nf_conntrack_helper *
nf_ct_lookup_helper(struct nf_conn *ct, struct net *net)
{
struct nf_conntrack_net *cnet = nf_ct_pernet(net);
if (!cnet->sysctl_auto_assign_helper) {
if (cnet->auto_assign_helper_warned)
return NULL;
if (!__nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple))
return NULL;
pr_info("nf_conntrack: default automatic helper assignment "
"has been turned off for security reasons and CT-based "
"firewall rule not found. Use the iptables CT target "
"to attach helpers instead.\n");
cnet->auto_assign_helper_warned = true;
return NULL;
}
return __nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
}
int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
gfp_t flags)
{
struct nf_conntrack_helper *helper = NULL;
struct nf_conn_help *help;
struct net *net = nf_ct_net(ct);
/* We already got a helper explicitly attached. The function
* nf_conntrack_alter_reply - in case NAT is in use - asks for looking
@ -246,24 +201,22 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
if (test_bit(IPS_HELPER_BIT, &ct->status))
return 0;
if (tmpl != NULL) {
if (WARN_ON_ONCE(!tmpl))
return 0;
help = nfct_help(tmpl);
if (help != NULL) {
helper = rcu_dereference(help->helper);
set_bit(IPS_HELPER_BIT, &ct->status);
}
}
help = nfct_help(ct);
if (helper == NULL) {
helper = nf_ct_lookup_helper(ct, net);
if (helper == NULL) {
if (help)
RCU_INIT_POINTER(help->helper, NULL);
return 0;
}
}
if (help == NULL) {
help = nf_ct_helper_ext_add(ct, flags);
@ -545,19 +498,6 @@ void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat)
}
EXPORT_SYMBOL_GPL(nf_nat_helper_unregister);
void nf_ct_set_auto_assign_helper_warned(struct net *net)
{
nf_ct_pernet(net)->auto_assign_helper_warned = true;
}
EXPORT_SYMBOL_GPL(nf_ct_set_auto_assign_helper_warned);
void nf_conntrack_helper_pernet_init(struct net *net)
{
struct nf_conntrack_net *cnet = nf_ct_pernet(net);
cnet->sysctl_auto_assign_helper = nf_ct_auto_assign_helper;
}
int nf_conntrack_helper_init(void)
{
nf_ct_helper_hsize = 1; /* gets rounded up to use one page */

View File

@ -2298,11 +2298,6 @@ ctnetlink_create_conntrack(struct net *net,
ct->status |= IPS_HELPER;
RCU_INIT_POINTER(help->helper, helper);
}
} else {
/* try an implicit helper assignation */
err = __nf_ct_try_assign_helper(ct, NULL, GFP_ATOMIC);
if (err < 0)
goto err2;
}
err = ctnetlink_setup_nat(ct, cda);

View File

@ -561,7 +561,6 @@ enum nf_ct_sysctl_index {
NF_SYSCTL_CT_LOG_INVALID,
NF_SYSCTL_CT_EXPECT_MAX,
NF_SYSCTL_CT_ACCT,
NF_SYSCTL_CT_HELPER,
#ifdef CONFIG_NF_CONNTRACK_EVENTS
NF_SYSCTL_CT_EVENTS,
#endif
@ -680,14 +679,6 @@ static struct ctl_table nf_ct_sysctl_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
[NF_SYSCTL_CT_HELPER] = {
.procname = "nf_conntrack_helper",
.maxlen = sizeof(u8),
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
#ifdef CONFIG_NF_CONNTRACK_EVENTS
[NF_SYSCTL_CT_EVENTS] = {
.procname = "nf_conntrack_events",
@ -1100,7 +1091,6 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
table[NF_SYSCTL_CT_CHECKSUM].data = &net->ct.sysctl_checksum;
table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid;
table[NF_SYSCTL_CT_ACCT].data = &net->ct.sysctl_acct;
table[NF_SYSCTL_CT_HELPER].data = &cnet->sysctl_auto_assign_helper;
#ifdef CONFIG_NF_CONNTRACK_EVENTS
table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events;
#endif

View File

@ -1089,9 +1089,6 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
if (err < 0)
goto err_put_helper;
/* Avoid the bogus warning, helper will be assigned after CT init */
nf_ct_set_auto_assign_helper_warned(ctx->net);
return 0;
err_put_helper:

View File

@ -102,26 +102,42 @@ check_for_helper()
ip netns exec ${netns} conntrack -L -f $family -p tcp --dport $port 2> /dev/null |grep -q 'helper=ftp'
if [ $? -ne 0 ] ; then
if [ $autoassign -eq 0 ] ;then
echo "FAIL: ${netns} did not show attached helper $message" 1>&2
ret=1
else
echo "PASS: ${netns} did not show attached helper $message" 1>&2
fi
else
if [ $autoassign -eq 0 ] ;then
echo "PASS: ${netns} connection on port $port has ftp helper attached" 1>&2
else
echo "FAIL: ${netns} connection on port $port has ftp helper attached" 1>&2
ret=1
fi
fi
echo "PASS: ${netns} connection on port $port has ftp helper attached" 1>&2
return 0
}
test_helper()
{
local port=$1
local msg=$2
local autoassign=$2
if [ $autoassign -eq 0 ] ;then
msg="set via ruleset"
else
msg="auto-assign"
fi
sleep 3 | ip netns exec ${ns2} nc -w 2 -l -p $port > /dev/null &
sleep 1 | ip netns exec ${ns1} nc -w 2 10.0.1.2 $port > /dev/null &
sleep 1
check_for_helper "$ns1" "ip $msg" $port
check_for_helper "$ns2" "ip $msg" $port
check_for_helper "$ns1" "ip $msg" $port $autoassign
check_for_helper "$ns2" "ip $msg" $port $autoassign
wait
@ -173,9 +189,9 @@ if [ $? -ne 0 ];then
fi
fi
test_helper 2121 "set via ruleset"
ip netns exec ${ns1} sysctl -q 'net.netfilter.nf_conntrack_helper=1'
ip netns exec ${ns2} sysctl -q 'net.netfilter.nf_conntrack_helper=1'
test_helper 21 "auto-assign"
test_helper 2121 0
ip netns exec ${ns1} sysctl -qe 'net.netfilter.nf_conntrack_helper=1'
ip netns exec ${ns2} sysctl -qe 'net.netfilter.nf_conntrack_helper=1'
test_helper 21 1
exit $ret