| Message ID | alpine.LSU.2.20.1702012057560.4763@cbobk.fhfr.pm |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Feb 01, 2017 at 09:01:54PM +0100, Jiri Kosina wrote: > From: Jiri Kosina <jkosina@suse.cz> > > Commit 3bb398d925 ("netfilter: nf_ct_helper: disable automatic helper > assignment") is causing behavior regressions in firewalls, as traffic > handled by conntrack helpers is now by default not passed through even > though it was before due to missing CT targets (which were not necessary > before this commit). > > The default had to be switched off due to security reasons [1] [2] and > therefore should stay the way it is, but let's be friendly to firewall > admins and issue a warning the first time we're in situation where packet > would be likely passed through with the old default but we're likely going > to drop it on the floor now. > > Rewrite the code a little bit as suggested by Linus, so that we avoid > spaghettiing the code even more -- namely the whole decision making > process regarding helper selection (either automatic or not) is being > separated, so that the whole logic can be simplified and code (condition) > duplication reduced. > > [1] https://cansecwest.com/csw12/conntrack-attack.pdf > [2] https://home.regit.org/netfilter-en/secure-use-of-helpers/ Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c index 7341adf..3457456 100644 --- a/net/netfilter/nf_conntrack_helper.c +++ b/net/netfilter/nf_conntrack_helper.c @@ -188,6 +188,25 @@ struct nf_conn_help * } EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add); +static struct nf_conntrack_helper *nf_ct_lookup_helper(struct nf_conn *ct, struct net *net) +{ + if (!net->ct.sysctl_auto_assign_helper) { + if (net->ct.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"); + net->ct.auto_assign_helper_warned = 1; + 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) { @@ -213,21 +232,14 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl, } help = nfct_help(ct); - if (net->ct.sysctl_auto_assign_helper && helper == NULL) { - helper = __nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple); - if (unlikely(!net->ct.auto_assign_helper_warned && helper)) { - pr_info("nf_conntrack: automatic helper " - "assignment is deprecated and it will " - "be removed soon. Use the iptables CT target " - "to attach helpers instead.\n"); - net->ct.auto_assign_helper_warned = true; - } - } if (helper == NULL) { - if (help) - RCU_INIT_POINTER(help->helper, NULL); - return 0; + helper = nf_ct_lookup_helper(ct, net); + if (helper == NULL) { + if (help) + RCU_INIT_POINTER(help->helper, NULL); + return 0; + } } if (help == NULL) {