diff mbox

[V2] netfilter: ipset: support package fragments for IPv4 protos without ports

Message ID 1367431355.17468.193.camel@homer.cohaesio.com
State Awaiting Upstream
Headers show

Commit Message

Anders K. Pedersen | Surftown May 1, 2013, 6:02 p.m. UTC
From: Anders K. Pedersen <akp@surftown.com>

Enable ipset port set types to match IPv4 package fragments for
protocols that doesn't have ports (or the port information isn't
supported by ipset).

For example this allows a hash:ip,port ipset containing the entry
192.168.0.1,gre:0 to match all package fragments for PPTP VPN tunnels
to/from the host. Without this patch only the first package fragment
(with fragment offset 0) was matched, while subsequent fragments wasn't.

This is not possible for IPv6, where the protocol is in the fragmented
part of the package unlike IPv4, where the protocol is in the IP header.

IPPROTO_ICMPV6 is deliberately not included, because it isn't relevant
for IPv4.

Signed-off-by: Anders K. Pedersen <akp@surftown.com>

---

The patch was implemented and tested on linux-3.8.10 and I have verified
that it applies cleanly to current linux.git and nf-next.git.

Now implemented directly in ip_set_get_ip4_port() as suggested. I
originally hadn't done this to avoid duplicating the protocol list from
get_port(), but this is clearly simpler.

Best regards,
Anders K. Pedersen
Surftown A/S



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

Comments

Pablo Neira Ayuso May 1, 2013, 10:23 p.m. UTC | #1
On Wed, May 01, 2013 at 08:02:35PM +0200, Anders K. Pedersen | Surftown wrote:
> From: Anders K. Pedersen <akp@surftown.com>
> 
> Enable ipset port set types to match IPv4 package fragments for
> protocols that doesn't have ports (or the port information isn't
> supported by ipset).
> 
> For example this allows a hash:ip,port ipset containing the entry
> 192.168.0.1,gre:0 to match all package fragments for PPTP VPN tunnels
> to/from the host. Without this patch only the first package fragment
> (with fragment offset 0) was matched, while subsequent fragments wasn't.
> 
> This is not possible for IPv6, where the protocol is in the fragmented
> part of the package unlike IPv4, where the protocol is in the IP header.
> 
> IPPROTO_ICMPV6 is deliberately not included, because it isn't relevant
> for IPv4.
> 
> Signed-off-by: Anders K. Pedersen <akp@surftown.com>
> 
> ---
> 
> The patch was implemented and tested on linux-3.8.10 and I have verified
> that it applies cleanly to current linux.git and nf-next.git.
> 
> Now implemented directly in ip_set_get_ip4_port() as suggested. I
> originally hadn't done this to avoid duplicating the protocol list from
> get_port(), but this is clearly simpler.
> 
> Best regards,
> Anders K. Pedersen
> Surftown A/S
> 
> --- linux-3.8.10/net/netfilter/ipset/ip_set_getport.c.orig	2013-02-19 00:58:34.000000000 +0100
> +++ linux-3.8.10/net/netfilter/ipset/ip_set_getport.c	2013-04-30 12:41:52.550817989 +0200
> @@ -102,9 +102,25 @@ ip_set_get_ip4_port(const struct sk_buff
>  	int protocol = iph->protocol;
>  
>  	/* See comments at tcp_match in ip_tables.c */
> -	if (protocol <= 0 || (ntohs(iph->frag_off) & IP_OFFSET))
> +	if (protocol <= 0)
>  		return false;
>  
> +	if (ntohs(iph->frag_off) & IP_OFFSET)
> +		switch (protocol) {
> +		case IPPROTO_TCP:
> +		case IPPROTO_SCTP:
> +		case IPPROTO_UDP:
> +		case IPPROTO_UDPLITE:
> +		case IPPROTO_ICMP:
> +			/* Port info not available for fragment offset > 0 */
> +			return false;

You can probably use proto_ports_offset for this?

> +		default:
> +			/* Other protocols doesn't have ports,
> +			   so we can match fragments */
> +			*proto = protocol;
> +			return true;
> +		}
> +
>  	return get_port(skb, protocol, protooff, src, port, proto);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_ip4_port);
> 
> 
> --
> 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
--
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
Anders K. Pedersen | Surftown May 2, 2013, 11:48 a.m. UTC | #2
On Thu, 2013-05-02 at 00:23 +0200, Pablo Neira Ayuso wrote:
> On Wed, May 01, 2013 at 08:02:35PM +0200, Anders K. Pedersen | Surftown wrote:
> > +	if (ntohs(iph->frag_off) & IP_OFFSET)
> > +		switch (protocol) {
> > +		case IPPROTO_TCP:
> > +		case IPPROTO_SCTP:
> > +		case IPPROTO_UDP:
> > +		case IPPROTO_UDPLITE:
> > +		case IPPROTO_ICMP:
> > +			/* Port info not available for fragment offset > 0 */
> > +			return false;
> 
> You can probably use proto_ports_offset for this?

I'm not sure that is a good idea. The protocols in proto_ports_offset()
doesn't match what ipset supports:

static inline int proto_ports_offset(int proto)
{
	switch (proto) {
	case IPPROTO_TCP:
	case IPPROTO_UDP:
	case IPPROTO_DCCP:
	case IPPROTO_ESP:	/* SPI */
	case IPPROTO_SCTP:
	case IPPROTO_UDPLITE:
		return 0;
	case IPPROTO_AH:	/* SPI */
		return 4;
	default:
		return -EINVAL;
	}
}

Ports for DCCP, ESP, and AH aren't supported by ipset. I could add that
support, but I don't think it makes sense for ipset to match on SPI for
ESP and AH?
Jozsef Kadlecsik May 3, 2013, 9:20 p.m. UTC | #3
On Wed, 1 May 2013, Anders K. Pedersen | Surftown wrote:

> From: Anders K. Pedersen <akp@surftown.com>
> 
> Enable ipset port set types to match IPv4 package fragments for
> protocols that doesn't have ports (or the port information isn't
> supported by ipset).
> 
> For example this allows a hash:ip,port ipset containing the entry
> 192.168.0.1,gre:0 to match all package fragments for PPTP VPN tunnels
> to/from the host. Without this patch only the first package fragment
> (with fragment offset 0) was matched, while subsequent fragments wasn't.
> 
> This is not possible for IPv6, where the protocol is in the fragmented
> part of the package unlike IPv4, where the protocol is in the IP header.
> 
> IPPROTO_ICMPV6 is deliberately not included, because it isn't relevant
> for IPv4.
> 
> Signed-off-by: Anders K. Pedersen <akp@surftown.com>

I applied your patch in the ipset git tree, thanks.

Best regards,
Jozsef
 
> ---
> 
> The patch was implemented and tested on linux-3.8.10 and I have verified
> that it applies cleanly to current linux.git and nf-next.git.
> 
> Now implemented directly in ip_set_get_ip4_port() as suggested. I
> originally hadn't done this to avoid duplicating the protocol list from
> get_port(), but this is clearly simpler.
> 
> Best regards,
> Anders K. Pedersen
> Surftown A/S
> 
> --- linux-3.8.10/net/netfilter/ipset/ip_set_getport.c.orig	2013-02-19 00:58:34.000000000 +0100
> +++ linux-3.8.10/net/netfilter/ipset/ip_set_getport.c	2013-04-30 12:41:52.550817989 +0200
> @@ -102,9 +102,25 @@ ip_set_get_ip4_port(const struct sk_buff
>  	int protocol = iph->protocol;
>  
>  	/* See comments at tcp_match in ip_tables.c */
> -	if (protocol <= 0 || (ntohs(iph->frag_off) & IP_OFFSET))
> +	if (protocol <= 0)
>  		return false;
>  
> +	if (ntohs(iph->frag_off) & IP_OFFSET)
> +		switch (protocol) {
> +		case IPPROTO_TCP:
> +		case IPPROTO_SCTP:
> +		case IPPROTO_UDP:
> +		case IPPROTO_UDPLITE:
> +		case IPPROTO_ICMP:
> +			/* Port info not available for fragment offset > 0 */
> +			return false;
> +		default:
> +			/* Other protocols doesn't have ports,
> +			   so we can match fragments */
> +			*proto = protocol;
> +			return true;
> +		}
> +
>  	return get_port(skb, protocol, protooff, src, port, proto);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_ip4_port);
> 
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary
--
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 mbox

Patch

--- linux-3.8.10/net/netfilter/ipset/ip_set_getport.c.orig	2013-02-19 00:58:34.000000000 +0100
+++ linux-3.8.10/net/netfilter/ipset/ip_set_getport.c	2013-04-30 12:41:52.550817989 +0200
@@ -102,9 +102,25 @@  ip_set_get_ip4_port(const struct sk_buff
 	int protocol = iph->protocol;
 
 	/* See comments at tcp_match in ip_tables.c */
-	if (protocol <= 0 || (ntohs(iph->frag_off) & IP_OFFSET))
+	if (protocol <= 0)
 		return false;
 
+	if (ntohs(iph->frag_off) & IP_OFFSET)
+		switch (protocol) {
+		case IPPROTO_TCP:
+		case IPPROTO_SCTP:
+		case IPPROTO_UDP:
+		case IPPROTO_UDPLITE:
+		case IPPROTO_ICMP:
+			/* Port info not available for fragment offset > 0 */
+			return false;
+		default:
+			/* Other protocols doesn't have ports,
+			   so we can match fragments */
+			*proto = protocol;
+			return true;
+		}
+
 	return get_port(skb, protocol, protooff, src, port, proto);
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ip4_port);