Commit 0bfcb7b7 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso
Browse files

netfilter: xtables: avoid NFPROTO_UNSPEC where needed 



syzbot managed to call xt_cluster match via ebtables:

 WARNING: CPU: 0 PID: 11 at net/netfilter/xt_cluster.c:72 xt_cluster_mt+0x196/0x780
 [..]
 ebt_do_table+0x174b/0x2a40

Module registers to NFPROTO_UNSPEC, but it assumes ipv4/ipv6 packet
processing.  As this is only useful to restrict locally terminating
TCP/UDP traffic, register this for ipv4 and ipv6 family only.

Pablo points out that this is a general issue, direct users of the
set/getsockopt interface can call into targets/matches that were only
intended for use with ip(6)tables.

Check all UNSPEC matches and targets for similar issues:

- matches and targets are fine except if they assume skb_network_header()
  is valid -- this is only true when called from inet layer: ip(6) stack
  pulls the ip/ipv6 header into linear data area.
- targets that return XT_CONTINUE or other xtables verdicts must be
  restricted too, they are incompatbile with the ebtables traverser, e.g.
  EBT_CONTINUE is a completely different value than XT_CONTINUE.

Most matches/targets are changed to register for NFPROTO_IPV4/IPV6, as
they are provided for use by ip(6)tables.

The MARK target is also used by arptables, so register for NFPROTO_ARP too.

While at it, bail out if connbytes fails to enable the corresponding
conntrack family.

This change passes the selftests in iptables.git.

Reported-by: default avatar <syzbot+256c348558aa5cf611a9@syzkaller.appspotmail.com>
Closes: https://lore.kernel.org/netfilter-devel/66fec2e2.050a0220.9ec68.0047.GAE@google.com/


Fixes: 0269ea49 ("netfilter: xtables: add cluster match")
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Co-developed-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 983e35ce

net/netfilter/xt_CHECKSUM.c

+23 −10
Original line number Diff line number Diff line
@@ -63,24 +63,37 @@ static int checksum_tg_check(const struct xt_tgchk_param *par) 
	return 0;

}



static struct xt_target checksum_tg_reg __read_mostly = {

static struct xt_target checksum_tg_reg[] __read_mostly = {

	{

		.name		= "CHECKSUM",

		.family		= NFPROTO_IPV4,

		.target		= checksum_tg,

		.targetsize	= sizeof(struct xt_CHECKSUM_info),

		.table		= "mangle",

		.checkentry	= checksum_tg_check,

		.me		= THIS_MODULE,

	},

#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)

	{

		.name		= "CHECKSUM",

	.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV6,

		.target		= checksum_tg,

		.targetsize	= sizeof(struct xt_CHECKSUM_info),

		.table		= "mangle",

		.checkentry	= checksum_tg_check,

		.me		= THIS_MODULE,

	},

#endif

};



static int __init checksum_tg_init(void)

{

	return xt_register_target(&checksum_tg_reg);

	return xt_register_targets(checksum_tg_reg, ARRAY_SIZE(checksum_tg_reg));

}



static void __exit checksum_tg_exit(void)

{

	xt_unregister_target(&checksum_tg_reg);

	xt_unregister_targets(checksum_tg_reg, ARRAY_SIZE(checksum_tg_reg));

}



module_init(checksum_tg_init);

net/netfilter/xt_CLASSIFY.c

+14 −2
Original line number Diff line number Diff line
@@ -38,7 +38,7 @@ static struct xt_target classify_tg_reg[] __read_mostly = { 
	{

		.name       = "CLASSIFY",

		.revision   = 0,

		.family     = NFPROTO_UNSPEC,

		.family     = NFPROTO_IPV4,

		.hooks      = (1 << NF_INET_LOCAL_OUT) | (1 << NF_INET_FORWARD) |

			      (1 << NF_INET_POST_ROUTING),

		.target     = classify_tg,
@@ -54,6 +54,18 @@ static struct xt_target classify_tg_reg[] __read_mostly = { 
		.targetsize = sizeof(struct xt_classify_target_info),

		.me         = THIS_MODULE,

	},

#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)

	{

		.name       = "CLASSIFY",

		.revision   = 0,

		.family     = NFPROTO_IPV6,

		.hooks      = (1 << NF_INET_LOCAL_OUT) | (1 << NF_INET_FORWARD) |

			      (1 << NF_INET_POST_ROUTING),

		.target     = classify_tg,

		.targetsize = sizeof(struct xt_classify_target_info),

		.me         = THIS_MODULE,

	},

#endif

};



static int __init classify_tg_init(void)

net/netfilter/xt_CONNSECMARK.c

+25 −11
Original line number Diff line number Diff line
@@ -114,25 +114,39 @@ static void connsecmark_tg_destroy(const struct xt_tgdtor_param *par) 
	nf_ct_netns_put(par->net, par->family);

}



static struct xt_target connsecmark_tg_reg __read_mostly = {

static struct xt_target connsecmark_tg_reg[] __read_mostly = {

	{

		.name       = "CONNSECMARK",

		.revision   = 0,

		.family     = NFPROTO_IPV4,

		.checkentry = connsecmark_tg_check,

		.destroy    = connsecmark_tg_destroy,

		.target     = connsecmark_tg,

		.targetsize = sizeof(struct xt_connsecmark_target_info),

		.me         = THIS_MODULE,

	},

#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)

	{

		.name       = "CONNSECMARK",

		.revision   = 0,

	.family     = NFPROTO_UNSPEC,

		.family     = NFPROTO_IPV6,

		.checkentry = connsecmark_tg_check,

		.destroy    = connsecmark_tg_destroy,

		.target     = connsecmark_tg,

		.targetsize = sizeof(struct xt_connsecmark_target_info),

		.me         = THIS_MODULE,

	},

#endif

};



static int __init connsecmark_tg_init(void)

{

	return xt_register_target(&connsecmark_tg_reg);

	return xt_register_targets(connsecmark_tg_reg, ARRAY_SIZE(connsecmark_tg_reg));

}



static void __exit connsecmark_tg_exit(void)

{

	xt_unregister_target(&connsecmark_tg_reg);

	xt_unregister_targets(connsecmark_tg_reg, ARRAY_SIZE(connsecmark_tg_reg));

}



module_init(connsecmark_tg_init);

net/netfilter/xt_CT.c

+69 −37
Original line number Diff line number Diff line
@@ -313,10 +313,30 @@ static void xt_ct_tg_destroy_v1(const struct xt_tgdtor_param *par) 
	xt_ct_tg_destroy(par, par->targinfo);

}



static unsigned int

notrack_tg(struct sk_buff *skb, const struct xt_action_param *par)

{

	/* Previously seen (loopback)? Ignore. */

	if (skb->_nfct != 0)

		return XT_CONTINUE;



	nf_ct_set(skb, NULL, IP_CT_UNTRACKED);



	return XT_CONTINUE;

}



static struct xt_target xt_ct_tg_reg[] __read_mostly = {

	{

		.name		= "NOTRACK",

		.revision	= 0,

		.family		= NFPROTO_IPV4,

		.target		= notrack_tg,

		.table		= "raw",

		.me		= THIS_MODULE,

	},

	{

		.name		= "CT",

		.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV4,

		.targetsize	= sizeof(struct xt_ct_target_info),

		.usersize	= offsetof(struct xt_ct_target_info, ct),

		.checkentry	= xt_ct_tg_check_v0,
@@ -327,7 +347,7 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = { 
	},

	{

		.name		= "CT",

		.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV4,

		.revision	= 1,

		.targetsize	= sizeof(struct xt_ct_target_info_v1),

		.usersize	= offsetof(struct xt_ct_target_info, ct),
@@ -339,7 +359,7 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = { 
	},

	{

		.name		= "CT",

		.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV4,

		.revision	= 2,

		.targetsize	= sizeof(struct xt_ct_target_info_v1),

		.usersize	= offsetof(struct xt_ct_target_info, ct),
@@ -349,49 +369,61 @@ static struct xt_target xt_ct_tg_reg[] __read_mostly = { 
		.table		= "raw",

		.me		= THIS_MODULE,

	},

};



static unsigned int

notrack_tg(struct sk_buff *skb, const struct xt_action_param *par)

#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)

	{

	/* Previously seen (loopback)? Ignore. */

	if (skb->_nfct != 0)

		return XT_CONTINUE;



	nf_ct_set(skb, NULL, IP_CT_UNTRACKED);



	return XT_CONTINUE;

}



static struct xt_target notrack_tg_reg __read_mostly = {

		.name		= "NOTRACK",

		.revision	= 0,

	.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV6,

		.target		= notrack_tg,

		.table		= "raw",

		.me		= THIS_MODULE,

	},

	{

		.name		= "CT",

		.family		= NFPROTO_IPV6,

		.targetsize	= sizeof(struct xt_ct_target_info),

		.usersize	= offsetof(struct xt_ct_target_info, ct),

		.checkentry	= xt_ct_tg_check_v0,

		.destroy	= xt_ct_tg_destroy_v0,

		.target		= xt_ct_target_v0,

		.table		= "raw",

		.me		= THIS_MODULE,

	},

	{

		.name		= "CT",

		.family		= NFPROTO_IPV6,

		.revision	= 1,

		.targetsize	= sizeof(struct xt_ct_target_info_v1),

		.usersize	= offsetof(struct xt_ct_target_info, ct),

		.checkentry	= xt_ct_tg_check_v1,

		.destroy	= xt_ct_tg_destroy_v1,

		.target		= xt_ct_target_v1,

		.table		= "raw",

		.me		= THIS_MODULE,

	},

	{

		.name		= "CT",

		.family		= NFPROTO_IPV6,

		.revision	= 2,

		.targetsize	= sizeof(struct xt_ct_target_info_v1),

		.usersize	= offsetof(struct xt_ct_target_info, ct),

		.checkentry	= xt_ct_tg_check_v2,

		.destroy	= xt_ct_tg_destroy_v1,

		.target		= xt_ct_target_v1,

		.table		= "raw",

		.me		= THIS_MODULE,

	},

#endif

};



static int __init xt_ct_tg_init(void)

{

	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;

	return xt_register_targets(xt_ct_tg_reg, ARRAY_SIZE(xt_ct_tg_reg));

}



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);

net/netfilter/xt_IDLETIMER.c

+40 −19
Original line number Diff line number Diff line
@@ -459,7 +459,7 @@ static void idletimer_tg_destroy_v1(const struct xt_tgdtor_param *par) 
static struct xt_target idletimer_tg[] __read_mostly = {

	{

		.name		= "IDLETIMER",

	.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV4,

		.target		= idletimer_tg_target,

		.targetsize     = sizeof(struct idletimer_tg_info),

		.usersize	= offsetof(struct idletimer_tg_info, timer),
@@ -469,7 +469,7 @@ static struct xt_target idletimer_tg[] __read_mostly = { 
	},

	{

		.name		= "IDLETIMER",

	.family		= NFPROTO_UNSPEC,

		.family		= NFPROTO_IPV4,

		.revision	= 1,

		.target		= idletimer_tg_target_v1,

		.targetsize     = sizeof(struct idletimer_tg_info_v1),
@@ -478,8 +478,29 @@ static struct xt_target idletimer_tg[] __read_mostly = { 
		.destroy        = idletimer_tg_destroy_v1,

		.me		= THIS_MODULE,

	},





#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)

	{

		.name		= "IDLETIMER",

		.family		= NFPROTO_IPV6,

		.target		= idletimer_tg_target,

		.targetsize     = sizeof(struct idletimer_tg_info),

		.usersize	= offsetof(struct idletimer_tg_info, timer),

		.checkentry	= idletimer_tg_checkentry,

		.destroy        = idletimer_tg_destroy,

		.me		= THIS_MODULE,

	},

	{

		.name		= "IDLETIMER",

		.family		= NFPROTO_IPV6,

		.revision	= 1,

		.target		= idletimer_tg_target_v1,

		.targetsize     = sizeof(struct idletimer_tg_info_v1),

		.usersize	= offsetof(struct idletimer_tg_info_v1, timer),

		.checkentry	= idletimer_tg_checkentry_v1,

		.destroy        = idletimer_tg_destroy_v1,

		.me		= THIS_MODULE,

	},

#endif

};



static struct class *idletimer_tg_class;