NOTRACK removal breaks working configurations

NOTRACK removal breaks working configurations

[Florian Westphal]

Pablo,

please consider reverting

commit 965505015beccc4ec900798070165875b8e8dccf
Author: Cong Wang <xiyou.wangcong@gmail.com>
Subject: netfilter: remove xt_NOTRACK

It breaks working netfilter configurations.
At the very least, NOTRACK should have printk'd

BIG FAT REMOVAL WARNING

for a year or so.  Which it didn't do.

Now, someone updating from 3.6 to 3.7 will get iptables-restore failure.

Re: NOTRACK removal breaks working configurations

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Hi Florian,

On Thu, Dec 20, 2012 at 11:23:58AM +0100, Florian Westphal wrote:
> Pablo,
> 
> please consider reverting
> 
> commit 965505015beccc4ec900798070165875b8e8dccf
> Author: Cong Wang <xiyou.wangcong@gmail.com>
> Subject: netfilter: remove xt_NOTRACK
> 
> It breaks working netfilter configurations.
> At the very least, NOTRACK should have printk'd
> 
> BIG FAT REMOVAL WARNING
> 
> for a year or so.  Which it didn't do.

This was announced in Documentation/feature-removal-schedule.txt and
the aliasing infrastructure was added to iptables but I agree in that
it was agressive since I think not many users have checked that file /
they may no have upgrade iptables to latest.

Can you see any problem with the patch attached?

[-- Attachment #2: 0001-netfilter-xt_CT-recover-NOTRACK-target-support.patch --]
[-- Type: text/x-diff, Size: 4201 bytes --]

>From 3d990849b0e4b0e4db93195a39e468968e5a10ac Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 20 Dec 2012 12:26:22 +0100
Subject: [PATCH] netfilter: xt_CT: recover NOTRACK target support

Florian Westphal reported that the removal of the NOTRACK target
(9655050 netfilter: remove xt_NOTRACK) is breaking some existing
setups.

That removal was scheduled for removal since long time ago as
described in Documentation/feature-removal-schedule.txt

What:  xt_NOTRACK
Files: net/netfilter/xt_NOTRACK.c
When:  April 2011
Why:   Superseded by xt_CT

Still, people may have not notice / may have decided to stick to an
old iptables version. I agree with him in that some more conservative
approach by spotting some printk to warn users for some time is less
agressive.

Current iptables 1.4.16.3 already contains the aliasing support
that makes it point to the CT target, so upgrading would fix it.
Still, the policy so far has been to avoid pushing our users to
upgrade.

As a solution, this patch recovers the NOTRACK target inside the CT
target and it now spots a warning.

Reported-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netns/x_tables.h |    1 +
 net/netfilter/Kconfig        |    4 ++++
 net/netfilter/xt_CT.c        |   50 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/include/net/netns/x_tables.h b/include/net/netns/x_tables.h
index 591db7d..c24060e 100644
--- a/include/net/netns/x_tables.h
+++ b/include/net/netns/x_tables.h
@@ -8,6 +8,7 @@ struct ebt_table;
 
 struct netns_xt {
 	struct list_head tables[NFPROTO_NUMPROTO];
+	bool notrack_deprecated_warning;
 #if defined(CONFIG_BRIDGE_NF_EBTABLES) || \
     defined(CONFIG_BRIDGE_NF_EBTABLES_MODULE)
 	struct ebt_table *broute_table;
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index fefa514..390f96c 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -680,6 +680,10 @@ config NETFILTER_XT_TARGET_NFQUEUE
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config NETFILTER_XT_TARGET_NOTRACK
+	tristate  '"NOTRACK" target support (DEPRECATED)'
+	select NETFILTER_XT_TARGET_CT
+
 config NETFILTER_XT_TARGET_RATEEST
 	tristate '"RATEEST" target support'
 	depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index ae7f5da..aeeaf5c 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -377,14 +377,60 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = {
 	},
 };
 
+static unsigned int
+notrack_tg(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	/* Previously seen (loopback)? Ignore. */
+	if (skb->nfct != NULL)
+		return XT_CONTINUE;
+
+	skb->nfct = &nf_ct_untracked_get()->ct_general;
+	skb->nfctinfo = IP_CT_NEW;
+	nf_conntrack_get(skb->nfct);
+
+	return XT_CONTINUE;
+}
+
+static int notrack_chk(const struct xt_tgchk_param *par)
+{
+	if (!par->net->xt.notrack_deprecated_warning) {
+		pr_info("netfilter: NOTRACK target is deprecated, "
+			"use CT instead or upgrade iptables\n");
+		par->net->xt.notrack_deprecated_warning = true;
+	}
+	return 0;
+}
+
+static struct xt_target notrack_tg_reg __read_mostly = {
+	.name		= "NOTRACK",
+	.revision	= 0,
+	.family		= NFPROTO_UNSPEC,
+	.checkentry	= notrack_chk,
+	.target		= notrack_tg,
+	.table		= "raw",
+	.me		= THIS_MODULE,
+};
+
 static int __init xt_ct_tg_init(void)
 {
-	return xt_register_targets(xt_ct_tg_reg, ARRAY_SIZE(xt_ct_tg_reg));
+	int ret;
+
+	ret = xt_register_target(&notrack_tg_reg);
+	if (ret < 0)
+		return ret;
+
+	ret = xt_register_targets(xt_ct_tg_reg, ARRAY_SIZE(xt_ct_tg_reg));
+	if (ret < 0) {
+		xt_unregister_target(&notrack_tg_reg);
+		return ret;
+	}
+	return 0;
 }
 
 static void __exit xt_ct_tg_exit(void)
 {
 	xt_unregister_targets(xt_ct_tg_reg, ARRAY_SIZE(xt_ct_tg_reg));
+	xt_unregister_target(&notrack_tg_reg);
 }
 
 module_init(xt_ct_tg_init);
@@ -394,3 +440,5 @@ MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Xtables: connection tracking target");
 MODULE_ALIAS("ipt_CT");
 MODULE_ALIAS("ip6t_CT");
+MODULE_ALIAS("ipt_NOTRACK");
+MODULE_ALIAS("ip6t_NOTRACK");
-- 
1.7.10.4

Re: NOTRACK removal breaks working configurations

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > commit 965505015beccc4ec900798070165875b8e8dccf
> > Author: Cong Wang <xiyou.wangcong@gmail.com>
> > Subject: netfilter: remove xt_NOTRACK
> > 
> > It breaks working netfilter configurations.
> > At the very least, NOTRACK should have printk'd
> > 
> > BIG FAT REMOVAL WARNING
> > 
> > for a year or so.  Which it didn't do.
> 
> This was announced in Documentation/feature-removal-schedule.txt and
> the aliasing infrastructure was added to iptables

I know.

> it was agressive since I think not many users have checked that file /
> they may no have upgrade iptables to latest.

Right.

> Can you see any problem with the patch attached?

No.  The patch works.
[   21.870092] xt_CT: netfilter: NOTRACK target is deprecated, use CT instead or upgrade iptables

Even better than a revert.

Thanks,
Florian

Re: NOTRACK removal breaks working configurations

[Pablo Neira Ayuso]

On Thu, Dec 20, 2012 at 01:28:39PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > commit 965505015beccc4ec900798070165875b8e8dccf
> > > Author: Cong Wang <xiyou.wangcong@gmail.com>
> > > Subject: netfilter: remove xt_NOTRACK
> > > 
> > > It breaks working netfilter configurations.
> > > At the very least, NOTRACK should have printk'd
> > > 
> > > BIG FAT REMOVAL WARNING
> > > 
> > > for a year or so.  Which it didn't do.
> > 
> > This was announced in Documentation/feature-removal-schedule.txt and
> > the aliasing infrastructure was added to iptables
> 
> I know.
> 
> > it was agressive since I think not many users have checked that file /
> > they may no have upgrade iptables to latest.
> 
> Right.
> 
> > Can you see any problem with the patch attached?
> 
> No.  The patch works.
> [   21.870092] xt_CT: netfilter: NOTRACK target is deprecated, use CT instead or upgrade iptables
> 
> Even better than a revert.

Thanks, I'll pass this to David in the next batch and then ask for
-stable submission.