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

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

[Jozsef Kadlecsik]

Hi,

On Sun, 28 Apr 2013, Anders K. Pedersen 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.
> 
> 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.
> 
> I have tried to make it as simple as possible, but if you feel that something
> should be done differently, I will do my best to implement it.

It'd better to add it directly to ip_set_get_ip4_port, thus the fragment 
checking is not scattered everywhere. Something like this:

	...
	if (ntohs(iph->frag_off) & IP_OFFSET) {
		switch (protocol) {
		case IPPROTO_TCP:
		...
			return false;
		default:
			...

Best regards,
Jozsef
 
> --- 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-28 10:27:50.291663937 +0200
> @@ -22,13 +22,17 @@
>  /* We must handle non-linear skbs */
>  static bool
>  get_port(const struct sk_buff *skb, int protocol, unsigned int protooff,
> -	 bool src, __be16 *port, u8 *proto)
> +	 int fragoff, bool src, __be16 *port, u8 *proto)
>  {
>  	switch (protocol) {
>  	case IPPROTO_TCP: {
>  		struct tcphdr _tcph;
>  		const struct tcphdr *th;
>  
> +		if (fragoff)
> +			/* Port info not available in subsequent fragments */
> +			return false;
> +
>  		th = skb_header_pointer(skb, protooff, sizeof(_tcph), &_tcph);
>  		if (th == NULL)
>  			/* No choice either */
> @@ -41,6 +45,10 @@ get_port(const struct sk_buff *skb, int
>  		sctp_sctphdr_t _sh;
>  		const sctp_sctphdr_t *sh;
>  
> +		if (fragoff)
> +			/* Port info not available in subsequent fragments */
> +			return false;
> +
>  		sh = skb_header_pointer(skb, protooff, sizeof(_sh), &_sh);
>  		if (sh == NULL)
>  			/* No choice either */
> @@ -54,6 +62,10 @@ get_port(const struct sk_buff *skb, int
>  		struct udphdr _udph;
>  		const struct udphdr *uh;
>  
> +		if (fragoff)
> +			/* Port info not available in subsequent fragments */
> +			return false;
> +
>  		uh = skb_header_pointer(skb, protooff, sizeof(_udph), &_udph);
>  		if (uh == NULL)
>  			/* No choice either */
> @@ -66,6 +78,10 @@ get_port(const struct sk_buff *skb, int
>  		struct icmphdr _ich;
>  		const struct icmphdr *ic;
>  
> +		if (fragoff)
> +			/* Port info not available in subsequent fragments */
> +			return false;
> +
>  		ic = skb_header_pointer(skb, protooff, sizeof(_ich), &_ich);
>  		if (ic == NULL)
>  			return false;
> @@ -77,6 +93,10 @@ get_port(const struct sk_buff *skb, int
>  		struct icmp6hdr _ich;
>  		const struct icmp6hdr *ic;
>  
> +		if (fragoff)
> +			/* Port info not available in subsequent fragments */
> +			return false;
> +
>  		ic = skb_header_pointer(skb, protooff, sizeof(_ich), &_ich);
>  		if (ic == NULL)
>  			return false;
> @@ -100,12 +120,13 @@ ip_set_get_ip4_port(const struct sk_buff
>  	const struct iphdr *iph = ip_hdr(skb);
>  	unsigned int protooff = ip_hdrlen(skb);
>  	int protocol = iph->protocol;
> +	int fragoff = (ntohs(iph->frag_off) & IP_OFFSET);
>  
>  	/* See comments at tcp_match in ip_tables.c */
> -	if (protocol <= 0 || (ntohs(iph->frag_off) & IP_OFFSET))
> +	if (protocol <= 0)
>  		return false;
>  
> -	return get_port(skb, protocol, protooff, src, port, proto);
> +	return get_port(skb, protocol, protooff, fragoff, src, port, proto);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_ip4_port);
>  
> @@ -124,7 +145,7 @@ ip_set_get_ip6_port(const struct sk_buff
>  	if (protoff < 0)
>  		return false;
>  
> -	return get_port(skb, nexthdr, protoff, src, port, proto);
> +	return get_port(skb, nexthdr, protoff, 0, src, port, proto);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_ip6_port);
>  #endif
> 

-
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

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

[Anders K. Pedersen | Surftown]

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

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

[Pablo Neira Ayuso]

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

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

[Anders K. Pedersen | Surftown]

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?

-- 
Venlig hilsen / Best Regards 

Anders K. Pedersen 
Surftown A/S

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

[Jozsef Kadlecsik]

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