| From 84cce3a6e0c32c66f8efdcede419826016144e7f Mon Sep 17 00:00:00 2001 |
| From: Jiri Kosina <jkosina@suse.cz> |
| Date: Wed, 1 Feb 2017 21:01:54 +0100 |
| Subject: [PATCH] netfilter: nf_ct_helper: warn when not applying default |
| helper assignment |
| |
| commit dfe75ff8ca74f54b0fa5a326a1aa9afa485ed802 upstream. |
| |
| 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/ |
| |
| Signed-off-by: Jiri Kosina <jkosina@suse.cz> |
| Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c |
| index b989b81ac156..183afa681d37 100644 |
| --- a/net/netfilter/nf_conntrack_helper.c |
| +++ b/net/netfilter/nf_conntrack_helper.c |
| @@ -183,6 +183,26 @@ nf_ct_helper_ext_add(struct nf_conn *ct, |
| } |
| 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) |
| { |
| @@ -209,21 +229,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); |
| - goto out; |
| + helper = nf_ct_lookup_helper(ct, net); |
| + if (helper == NULL) { |
| + if (help) |
| + RCU_INIT_POINTER(help->helper, NULL); |
| + goto out; |
| + } |
| } |
| |
| if (help == NULL) { |
| -- |
| 2.12.0 |
| |