Commit 05a5474e authored by Jakub Kicinski's avatar Jakub Kicinski

Merge git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf

Florian Westphal says:

====================
netfilter: bug fixes for net

1. Fix IP address check in irc DCC conntrack helper, this should check
   the opposite direction rather than the destination address of the
   packets' direction, from David Leadbeater.

2. bridge netfilter needs to drop dst references, from Harsh Modi.
   This was fine back in the day the code was originally written,
   but nowadays various tunnels can pre-set metadata dsts on packets.

3. Remove nf_conntrack_helper sysctl and the modparam toggle, users
   need to explicitily assign the helpers to use via nftables or
   iptables.  Conntrack helpers, by design, may be used to add dynamic
   port redirections to internal machines, so its necessary to restrict
   which hosts/peers are allowed to use them.
   It was discovered that improper checking in the irc DCC helper makes
   it possible to trigger the 'please do dynamic port forward'
   from outside by embedding a 'DCC' in a PING request; if the client
   echos that back a expectation/port forward gets added.
   The auto-assign-for-everything mechanism has been in "please don't do this"
   territory since 2012.  From Pablo.

4. Fix a memory leak in the netdev hook error unwind path, also from Pablo.

* git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf:
  netfilter: nf_conntrack_irc: Fix forged IP logic
  netfilter: nf_tables: clean up hook list when offload flags check fails
  netfilter: br_netfilter: Drop dst references before setting.
  netfilter: remove nf_conntrack_helper sysctl and modparam toggles
====================

Link: https://lore.kernel.org/r/20220901071238.3044-1-fw@strlen.deSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents e7506d34 0efe125c
...@@ -53,8 +53,6 @@ struct nf_conntrack_net { ...@@ -53,8 +53,6 @@ struct nf_conntrack_net {
/* only used when new connection is allocated: */ /* only used when new connection is allocated: */
atomic_t count; atomic_t count;
unsigned int expect_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: */ /* only used from work queues, configuration plane, and so on: */
unsigned int users4; unsigned int users4;
......
...@@ -101,7 +101,6 @@ struct netns_ct { ...@@ -101,7 +101,6 @@ struct netns_ct {
u8 sysctl_log_invalid; /* Log invalid packets */ u8 sysctl_log_invalid; /* Log invalid packets */
u8 sysctl_events; u8 sysctl_events;
u8 sysctl_acct; u8 sysctl_acct;
u8 sysctl_auto_assign_helper;
u8 sysctl_tstamp; u8 sysctl_tstamp;
u8 sysctl_checksum; u8 sysctl_checksum;
......
...@@ -384,6 +384,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_ ...@@ -384,6 +384,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
/* - Bridged-and-DNAT'ed traffic doesn't /* - Bridged-and-DNAT'ed traffic doesn't
* require ip_forwarding. */ * require ip_forwarding. */
if (rt->dst.dev == dev) { if (rt->dst.dev == dev) {
skb_dst_drop(skb);
skb_dst_set(skb, &rt->dst); skb_dst_set(skb, &rt->dst);
goto bridged_dnat; goto bridged_dnat;
} }
...@@ -413,6 +414,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_ ...@@ -413,6 +414,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
kfree_skb(skb); kfree_skb(skb);
return 0; return 0;
} }
skb_dst_drop(skb);
skb_dst_set_noref(skb, &rt->dst); skb_dst_set_noref(skb, &rt->dst);
} }
......
...@@ -197,6 +197,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc ...@@ -197,6 +197,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
kfree_skb(skb); kfree_skb(skb);
return 0; return 0;
} }
skb_dst_drop(skb);
skb_dst_set_noref(skb, &rt->dst); skb_dst_set_noref(skb, &rt->dst);
} }
......
...@@ -1782,7 +1782,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl, ...@@ -1782,7 +1782,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
} }
spin_unlock_bh(&nf_conntrack_expect_lock); spin_unlock_bh(&nf_conntrack_expect_lock);
} }
if (!exp) if (!exp && tmpl)
__nf_ct_try_assign_helper(ct, tmpl, GFP_ATOMIC); __nf_ct_try_assign_helper(ct, tmpl, GFP_ATOMIC);
/* Other CPU might have obtained a pointer to this object before it was /* 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, ...@@ -2068,10 +2068,6 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply; ct->tuplehash[IP_CT_DIR_REPLY].tuple = *newreply;
if (ct->master || (help && !hlist_empty(&help->expectations))) if (ct->master || (help && !hlist_empty(&help->expectations)))
return; return;
rcu_read_lock();
__nf_ct_try_assign_helper(ct, NULL, GFP_ATOMIC);
rcu_read_unlock();
} }
EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply); EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply);
...@@ -2797,7 +2793,6 @@ int nf_conntrack_init_net(struct net *net) ...@@ -2797,7 +2793,6 @@ int nf_conntrack_init_net(struct net *net)
nf_conntrack_acct_pernet_init(net); nf_conntrack_acct_pernet_init(net);
nf_conntrack_tstamp_pernet_init(net); nf_conntrack_tstamp_pernet_init(net);
nf_conntrack_ecache_pernet_init(net); nf_conntrack_ecache_pernet_init(net);
nf_conntrack_helper_pernet_init(net);
nf_conntrack_proto_pernet_init(net); nf_conntrack_proto_pernet_init(net);
return 0; return 0;
......
...@@ -35,11 +35,6 @@ unsigned int nf_ct_helper_hsize __read_mostly; ...@@ -35,11 +35,6 @@ unsigned int nf_ct_helper_hsize __read_mostly;
EXPORT_SYMBOL_GPL(nf_ct_helper_hsize); EXPORT_SYMBOL_GPL(nf_ct_helper_hsize);
static unsigned int nf_ct_helper_count __read_mostly; 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 DEFINE_MUTEX(nf_ct_nat_helpers_mutex);
static struct list_head nf_ct_nat_helpers __read_mostly; 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) ...@@ -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; (__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 * struct nf_conntrack_helper *
__nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum) __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) ...@@ -209,33 +186,11 @@ nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp)
} }
EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add); 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, int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
gfp_t flags) gfp_t flags)
{ {
struct nf_conntrack_helper *helper = NULL; struct nf_conntrack_helper *helper = NULL;
struct nf_conn_help *help; struct nf_conn_help *help;
struct net *net = nf_ct_net(ct);
/* We already got a helper explicitly attached. The function /* We already got a helper explicitly attached. The function
* nf_conntrack_alter_reply - in case NAT is in use - asks for looking * nf_conntrack_alter_reply - in case NAT is in use - asks for looking
...@@ -246,23 +201,21 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl, ...@@ -246,23 +201,21 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
if (test_bit(IPS_HELPER_BIT, &ct->status)) if (test_bit(IPS_HELPER_BIT, &ct->status))
return 0; return 0;
if (tmpl != NULL) { if (WARN_ON_ONCE(!tmpl))
help = nfct_help(tmpl); return 0;
if (help != NULL) {
helper = rcu_dereference(help->helper); help = nfct_help(tmpl);
set_bit(IPS_HELPER_BIT, &ct->status); if (help != NULL) {
} helper = rcu_dereference(help->helper);
set_bit(IPS_HELPER_BIT, &ct->status);
} }
help = nfct_help(ct); help = nfct_help(ct);
if (helper == NULL) { if (helper == NULL) {
helper = nf_ct_lookup_helper(ct, net); if (help)
if (helper == NULL) { RCU_INIT_POINTER(help->helper, NULL);
if (help) return 0;
RCU_INIT_POINTER(help->helper, NULL);
return 0;
}
} }
if (help == NULL) { if (help == NULL) {
...@@ -545,19 +498,6 @@ void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat) ...@@ -545,19 +498,6 @@ void nf_nat_helper_unregister(struct nf_conntrack_nat_helper *nat)
} }
EXPORT_SYMBOL_GPL(nf_nat_helper_unregister); 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) int nf_conntrack_helper_init(void)
{ {
nf_ct_helper_hsize = 1; /* gets rounded up to use one page */ nf_ct_helper_hsize = 1; /* gets rounded up to use one page */
......
...@@ -194,8 +194,9 @@ static int help(struct sk_buff *skb, unsigned int protoff, ...@@ -194,8 +194,9 @@ static int help(struct sk_buff *skb, unsigned int protoff,
/* dcc_ip can be the internal OR external (NAT'ed) IP */ /* dcc_ip can be the internal OR external (NAT'ed) IP */
tuple = &ct->tuplehash[dir].tuple; tuple = &ct->tuplehash[dir].tuple;
if (tuple->src.u3.ip != dcc_ip && if ((tuple->src.u3.ip != dcc_ip &&
tuple->dst.u3.ip != dcc_ip) { ct->tuplehash[!dir].tuple.dst.u3.ip != dcc_ip) ||
dcc_port == 0) {
net_warn_ratelimited("Forged DCC command from %pI4: %pI4:%u\n", net_warn_ratelimited("Forged DCC command from %pI4: %pI4:%u\n",
&tuple->src.u3.ip, &tuple->src.u3.ip,
&dcc_ip, dcc_port); &dcc_ip, dcc_port);
......
...@@ -2298,11 +2298,6 @@ ctnetlink_create_conntrack(struct net *net, ...@@ -2298,11 +2298,6 @@ ctnetlink_create_conntrack(struct net *net,
ct->status |= IPS_HELPER; ct->status |= IPS_HELPER;
RCU_INIT_POINTER(help->helper, 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); err = ctnetlink_setup_nat(ct, cda);
......
...@@ -561,7 +561,6 @@ enum nf_ct_sysctl_index { ...@@ -561,7 +561,6 @@ enum nf_ct_sysctl_index {
NF_SYSCTL_CT_LOG_INVALID, NF_SYSCTL_CT_LOG_INVALID,
NF_SYSCTL_CT_EXPECT_MAX, NF_SYSCTL_CT_EXPECT_MAX,
NF_SYSCTL_CT_ACCT, NF_SYSCTL_CT_ACCT,
NF_SYSCTL_CT_HELPER,
#ifdef CONFIG_NF_CONNTRACK_EVENTS #ifdef CONFIG_NF_CONNTRACK_EVENTS
NF_SYSCTL_CT_EVENTS, NF_SYSCTL_CT_EVENTS,
#endif #endif
...@@ -680,14 +679,6 @@ static struct ctl_table nf_ct_sysctl_table[] = { ...@@ -680,14 +679,6 @@ static struct ctl_table nf_ct_sysctl_table[] = {
.extra1 = SYSCTL_ZERO, .extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE, .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 #ifdef CONFIG_NF_CONNTRACK_EVENTS
[NF_SYSCTL_CT_EVENTS] = { [NF_SYSCTL_CT_EVENTS] = {
.procname = "nf_conntrack_events", .procname = "nf_conntrack_events",
...@@ -1100,7 +1091,6 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net) ...@@ -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_CHECKSUM].data = &net->ct.sysctl_checksum;
table[NF_SYSCTL_CT_LOG_INVALID].data = &net->ct.sysctl_log_invalid; 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_ACCT].data = &net->ct.sysctl_acct;
table[NF_SYSCTL_CT_HELPER].data = &cnet->sysctl_auto_assign_helper;
#ifdef CONFIG_NF_CONNTRACK_EVENTS #ifdef CONFIG_NF_CONNTRACK_EVENTS
table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events; table[NF_SYSCTL_CT_EVENTS].data = &net->ct.sysctl_events;
#endif #endif
......
...@@ -2166,8 +2166,10 @@ static int nft_basechain_init(struct nft_base_chain *basechain, u8 family, ...@@ -2166,8 +2166,10 @@ static int nft_basechain_init(struct nft_base_chain *basechain, u8 family,
chain->flags |= NFT_CHAIN_BASE | flags; chain->flags |= NFT_CHAIN_BASE | flags;
basechain->policy = NF_ACCEPT; basechain->policy = NF_ACCEPT;
if (chain->flags & NFT_CHAIN_HW_OFFLOAD && if (chain->flags & NFT_CHAIN_HW_OFFLOAD &&
!nft_chain_offload_support(basechain)) !nft_chain_offload_support(basechain)) {
list_splice_init(&basechain->hook_list, &hook->list);
return -EOPNOTSUPP; return -EOPNOTSUPP;
}
flow_block_init(&basechain->flow_block); flow_block_init(&basechain->flow_block);
......
...@@ -1089,9 +1089,6 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx, ...@@ -1089,9 +1089,6 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
if (err < 0) if (err < 0)
goto err_put_helper; 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; return 0;
err_put_helper: err_put_helper:
......
...@@ -102,26 +102,42 @@ check_for_helper() ...@@ -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' ip netns exec ${netns} conntrack -L -f $family -p tcp --dport $port 2> /dev/null |grep -q 'helper=ftp'
if [ $? -ne 0 ] ; then if [ $? -ne 0 ] ; then
echo "FAIL: ${netns} did not show attached helper $message" 1>&2 if [ $autoassign -eq 0 ] ;then
ret=1 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 fi
echo "PASS: ${netns} connection on port $port has ftp helper attached" 1>&2
return 0 return 0
} }
test_helper() test_helper()
{ {
local port=$1 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 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 | ip netns exec ${ns1} nc -w 2 10.0.1.2 $port > /dev/null &
sleep 1 sleep 1
check_for_helper "$ns1" "ip $msg" $port check_for_helper "$ns1" "ip $msg" $port $autoassign
check_for_helper "$ns2" "ip $msg" $port check_for_helper "$ns2" "ip $msg" $port $autoassign
wait wait
...@@ -173,9 +189,9 @@ if [ $? -ne 0 ];then ...@@ -173,9 +189,9 @@ if [ $? -ne 0 ];then
fi fi
fi fi
test_helper 2121 "set via ruleset" test_helper 2121 0
ip netns exec ${ns1} sysctl -q 'net.netfilter.nf_conntrack_helper=1' ip netns exec ${ns1} sysctl -qe 'net.netfilter.nf_conntrack_helper=1'
ip netns exec ${ns2} sysctl -q 'net.netfilter.nf_conntrack_helper=1' ip netns exec ${ns2} sysctl -qe 'net.netfilter.nf_conntrack_helper=1'
test_helper 21 "auto-assign" test_helper 21 1
exit $ret exit $ret
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment