[Acked] [Precise][CVE-2014-8160] netfilter: conntrack: disable generic tracking for known protocols

[Andy Whitcroft]

On Tue, Jan 20, 2015 at 02:41:59PM +0000, Luis Henriques wrote:
> From: Florian Westphal <fw at strlen.de>
> 
> Given following iptables ruleset:
> 
> -P FORWARD DROP
> -A FORWARD -m sctp --dport 9 -j ACCEPT
> -A FORWARD -p tcp --dport 80 -j ACCEPT
> -A FORWARD -p tcp -m conntrack -m state ESTABLISHED,RELATED -j ACCEPT
> 
> One would assume that this allows SCTP on port 9 and TCP on port 80.
> Unfortunately, if the SCTP conntrack module is not loaded, this allows
> *all* SCTP communication, to pass though, i.e. -p sctp -j ACCEPT,
> which we think is a security issue.
> 
> This is because on the first SCTP packet on port 9, we create a dummy
> "generic l4" conntrack entry without any port information (since
> conntrack doesn't know how to extract this information).
> 
> All subsequent packets that are unknown will then be in established
> state since they will fallback to proto_generic and will match the
> 'generic' entry.
> 
> Our originally proposed version [1] completely disabled generic protocol
> tracking, but Jozsef suggests to not track protocols for which a more
> suitable helper is available, hence we now mitigate the issue for in
> tree known ct protocol helpers only, so that at least NAT and direction
> information will still be preserved for others.
> 
>  [1] http://www.spinics.net/lists/netfilter-devel/msg33430.html
> 
> Joint work with Daniel Borkmann.
> 
> Signed-off-by: Florian Westphal <fw at strlen.de>
> Signed-off-by: Daniel Borkmann <dborkman at redhat.com>
> Acked-by: Jozsef Kadlecsik <kadlec at blackhole.kfki.hu>
> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> (backported from commit db29a9508a9246e77087c5531e45b2c88ec6988b)
> CVE-2014-8160
> BugLink: http://bugs.launchpad.net/bugs/XXX
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  net/netfilter/nf_conntrack_proto_generic.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index e2091d0c7a2f..53bf12abbe05 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -14,6 +14,30 @@
>  
>  static unsigned int nf_ct_generic_timeout __read_mostly = 600*HZ;
>  
> +static bool nf_generic_should_process(u8 proto)
> +{
> +	switch (proto) {
> +#ifdef CONFIG_NF_CT_PROTO_SCTP_MODULE
> +	case IPPROTO_SCTP:
> +		return false;
> +#endif
> +#ifdef CONFIG_NF_CT_PROTO_DCCP_MODULE
> +	case IPPROTO_DCCP:
> +		return false;
> +#endif
> +#ifdef CONFIG_NF_CT_PROTO_GRE_MODULE
> +	case IPPROTO_GRE:
> +		return false;
> +#endif
> +#ifdef CONFIG_NF_CT_PROTO_UDPLITE_MODULE
> +	case IPPROTO_UDPLITE:
> +		return false;
> +#endif
> +	default:
> +		return true;
> +	}
> +}
> +
>  static bool generic_pkt_to_tuple(const struct sk_buff *skb,
>  				 unsigned int dataoff,
>  				 struct nf_conntrack_tuple *tuple)
> @@ -56,7 +80,7 @@ static int packet(struct nf_conn *ct,
>  static bool new(struct nf_conn *ct, const struct sk_buff *skb,
>  		unsigned int dataoff)
>  {
> -	return true;
> +	return nf_generic_should_process(nf_ct_protonum(ct));
>  }
>  
>  #ifdef CONFIG_SYSCTL

Looks to do what is expected:

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw