Re: routing table lookup

2016-10-12 Thread Michal Kubecek 
On Wed, Oct 12, 2016 at 12:17:24AM +0200, Bjørnar Ness wrote:
> 
> Yeah, sortoff. But afaik rpfilter is a iptables module, and not
> available in nftables yet.
> 
> Pablo: is the "lookup in routing table from nftables" a total waste of time?

You may be interested in

  https://www.youtube.com/watch?v=wfWMPlZHQBk=19m40s

Michal Kubecek

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch] netfilter: nft_exthdr: fix error handling in nft_exthdr_init()

2016-10-12 Thread Dan Carpenter 
"err" needs to be signed for the error handling to work.

Fixes: 36b701fae12a ('netfilter: nf_tables: validate maximum value of u32 
netlink attributes')
Signed-off-by: Dan Carpenter 

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index a84cf3d..47beb3a 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -59,7 +59,8 @@ static int nft_exthdr_init(const struct nft_ctx *ctx,
   const struct nlattr * const tb[])
 {
struct nft_exthdr *priv = nft_expr_priv(expr);
-   u32 offset, len, err;
+   u32 offset, len;
+   int err;
 
if (tb[NFTA_EXTHDR_DREG] == NULL ||
tb[NFTA_EXTHDR_TYPE] == NULL ||
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch v2] netfilter: nf_tables: underflow in nft_parse_u32_check()

2016-10-12 Thread Dan Carpenter 
We don't want to allow negatives here.

Fixes: 36b701fae12a ('netfilter: nf_tables: validate maximum value of u32 
netlink attributes')
Signed-off-by: Dan Carpenter 
---
v2: cosmetic change

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b70d3ea..dd55187 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4423,7 +4423,7 @@ static int nf_tables_check_loops(const struct nft_ctx 
*ctx,
  */
 unsigned int nft_parse_u32_check(const struct nlattr *attr, int max, u32 *dest)
 {
-   int val;
+   u32 val;
 
val = ntohl(nla_get_be32(attr));
if (val > max)
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] SIP conntrack handler and TCP fragmentation

2016-10-12 Thread Florian Westphal 
Ulrich Weber  wrote:
> From reading the code this fixed the problem when Content-Length
> points to one of the next TCP fragments.

Right.

> In our case Content-Length is always 0 with a couple of SUBSCRIBE calls.
> E.g. a TCP packet starting with this will break the SIP connection tracking:
> 
> INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
> Content-Length: 0

Ugh.

I guess it makes sense to detect this and then accept for this case too.

> The previous TCP packet was accepted by SIP connection tracking
> because it had no Content-Length field.

Perhaps we should treat Content-length of 0 like "no Conent-Length
field".
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] SIP conntrack handler and TCP fragmentation

2016-10-12 Thread Ulrich Weber 
Hi Florian,

On 12.10.2016 09:52, Florian Westphal wrote:
> Ulrich Weber  wrote:
>> I know the proper solution would be TCP defragmentation
>> in the nf_conntrack_sip kernel module. However I'm not
>> sure if this is worth the effort.
> 
> I think an even better solution would be a SIP proxy that can
> inject expectations to keep datapath in kernel and only deals with
> the signalling traffic.
Yes your right, is there already one capable of doing this?
But then again, I dont want to put a proxy on an embedded box ;)

> 
>> What about just accepting unparsable TCP SIP packets?
> 
> I wonder why this patch did not fix your problem:
> 
> 3a7b21eaf4fb3c971bdb47a98f570550ddfe4471
> Author: Patrick McHardy 
> netfilter: nf_ct_sip: don't drop packets with offsets pointing outside the 
> packet
> 
> It specifically deals with this problem (l7 size larger than packet
> size).
>From reading the code this fixed the problem when Content-Length
points to one of the next TCP fragments.

In our case Content-Length is always 0 with a couple of SUBSCRIBE calls.
E.g. a TCP packet starting with this will break the SIP connection tracking:

INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
Content-Length: 0

The previous TCP packet was accepted by SIP connection tracking
because it had no Content-Length field.

Cheers
 Ulrich
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] SIP conntrack handler and TCP fragmentation

2016-10-12 Thread Ulrich Weber 
Hi all,

we had a customer with a Cisco Phone using SIP over TCP
which subscribed to 15 Phone numbers.

Since 15 subscribe calls dont fit in one packet, these
requests were fragmented in multiple TCP packets.

One of these fragments gets rejected then by nf_conntrack_sip with:
nf_ct_sip: dropping packet: cannot parse cseq

The Cisco phone will send retransmits, which got dropped too,
times out after some time, reconnects and same games
starts again...

I know the proper solution would be TCP defragmentation
in the nf_conntrack_sip kernel module. However I'm not
sure if this is worth the effort.
What about just accepting unparsable TCP SIP packets?

Cheers
 Ulrich
From 084a77a72bfc1c5c655f51618bfe885a216ce88b Mon Sep 17 00:00:00 2001
From: Ulrich Weber 
Date: Wed, 12 Oct 2016 09:27:23 +0200
Subject: [PATCH] nf_conntrack_sip: relax SIP validation for TCP

Due TCP fragmentation we cant assume full SIP messages all the time.
Therefore Accept all packets, even unparsable.

Signed-off-by: Ulrich Weber 
---
 net/netfilter/nf_conntrack_sip.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 621b81c..7700556 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1562,7 +1562,10 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 			hooks->seq_adjust(skb, protoff, tdiff);
 	}
 
-	return ret;
+	/* Due TCP fragmentation we cant assume full SIP messages all the time.
+	 * Therefore Accept all packets, even unparsable.
+	 */
+	return NF_ACCEPT;
 }
 
 static int sip_help_udp(struct sk_buff *skb, unsigned int protoff,
-- 
2.7.4

[patch] netfilter: nf_tables: underflow in nft_parse_u32_check()

2016-10-12 Thread Dan Carpenter 
We don't want to allow negatives here.

Fixes: 36b701fae12a ('netfilter: nf_tables: validate maximum value of u32 
netlink attributes')
Signed-off-by: Dan Carpenter 

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b70d3ea..dd55187 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4423,7 +4423,7 @@ static int nf_tables_check_loops(const struct nft_ctx 
*ctx,
  */
 unsigned int nft_parse_u32_check(const struct nlattr *attr, int max, u32 *dest)
 {
-   int val;
+   uint val;
 
val = ntohl(nla_get_be32(attr));
if (val > max)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] SIP conntrack handler and TCP fragmentation

2016-10-12 Thread Florian Westphal 
Ulrich Weber  wrote:
> we had a customer with a Cisco Phone using SIP over TCP
> which subscribed to 15 Phone numbers.
> 
> Since 15 subscribe calls dont fit in one packet, these
> requests were fragmented in multiple TCP packets.
> 
> One of these fragments gets rejected then by nf_conntrack_sip with:
> nf_ct_sip: dropping packet: cannot parse cseq
> 
> The Cisco phone will send retransmits, which got dropped too,
> times out after some time, reconnects and same games
> starts again...
> 
> I know the proper solution would be TCP defragmentation
> in the nf_conntrack_sip kernel module. However I'm not
> sure if this is worth the effort.

I think an even better solution would be a SIP proxy that can
inject expectations to keep datapath in kernel and only deals with
the signalling traffic.

> What about just accepting unparsable TCP SIP packets?

I wonder why this patch did not fix your problem:

3a7b21eaf4fb3c971bdb47a98f570550ddfe4471
Author: Patrick McHardy 
netfilter: nf_ct_sip: don't drop packets with offsets pointing outside the 
packet

It specifically deals with this problem (l7 size larger than packet
size).

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf] netfilter: nft_hash: add missing NFTA_HASH_OFFSET's nla_policy

2016-10-12 Thread Liping Zhang 
From: Liping Zhang 

Missing the nla_policy description will also miss the validation check
in kernel.

Fixes: 70ca767ea1b2 ("netfilter: nft_hash: Add hash offset value")
Signed-off-by: Liping Zhang 
---
 net/netfilter/nft_hash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 09473b4..baf694d 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -44,6 +44,7 @@ static const struct nla_policy nft_hash_policy[NFTA_HASH_MAX 
+ 1] = {
[NFTA_HASH_LEN] = { .type = NLA_U32 },
[NFTA_HASH_MODULUS] = { .type = NLA_U32 },
[NFTA_HASH_SEED]= { .type = NLA_U32 },
+   [NFTA_HASH_OFFSET]  = { .type = NLA_U32 },
 };
 
 static int nft_hash_init(const struct nft_ctx *ctx,
-- 
2.5.5


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf] netfilter: xt_ipcomp: add "ip[6]t_ipcomp" module alias name

2016-10-12 Thread Liping Zhang 
From: Liping Zhang 

Otherwise, user cannot add related rules if xt_ipcomp.ko is not loaded:
  # iptables -A OUTPUT -p 108 -m ipcomp --ipcompspi 1
  iptables: No chain/target/match by that name.

Signed-off-by: Liping Zhang 
---
 net/netfilter/xt_ipcomp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/xt_ipcomp.c b/net/netfilter/xt_ipcomp.c
index 89d5310..000e703 100644
--- a/net/netfilter/xt_ipcomp.c
+++ b/net/netfilter/xt_ipcomp.c
@@ -26,6 +26,8 @@
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Fan Du ");
 MODULE_DESCRIPTION("Xtables: IPv4/6 IPsec-IPComp SPI match");
+MODULE_ALIAS("ipt_ipcomp");
+MODULE_ALIAS("ip6t_ipcomp");
 
 /* Returns 1 if the spi is matched by the range, 0 otherwise */
 static inline bool
-- 
2.5.5


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: routing table lookup

2016-10-12 Thread Bjørnar Ness 
2016-10-12 8:19 GMT+02:00 Michal Kubecek :
> On Wed, Oct 12, 2016 at 12:17:24AM +0200, Bjørnar Ness wrote:
>>
>> Yeah, sortoff. But afaik rpfilter is a iptables module, and not
>> available in nftables yet.
>>
>> Pablo: is the "lookup in routing table from nftables" a total waste of time?
>
> You may be interested in
>
>   https://www.youtube.com/watch?v=wfWMPlZHQBk=19m40s

Thanks, Michal, this is interesting, but not exactly what I am looking
for. This fib module
would as far as I can tell follow the routing from rules -> table ->
decision, which will need
both a src and dst address. What I want is to skip the rule matching,
and check directly in
a table, that way we only need a single address, and the following
should potentially work
from prerouting:

ip saddr rt_table 10 drop

comments?

-- 
Bj(/)rnar
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] SIP conntrack handler and TCP fragmentation

2016-10-12 Thread Ulrich Weber 
On 12.10.2016 13:41, Florian Westphal wrote:
> Ulrich Weber  wrote:
>> From reading the code this fixed the problem when Content-Length
>> points to one of the next TCP fragments.
> 
> Right.
> 
>> In our case Content-Length is always 0 with a couple of SUBSCRIBE calls.
>> E.g. a TCP packet starting with this will break the SIP connection tracking:
>>
>> INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
>> Content-Length: 0
> 
> Ugh.
> 
> I guess it makes sense to detect this and then accept for this case too.
> 
>> The previous TCP packet was accepted by SIP connection tracking
>> because it had no Content-Length field.
> 
> Perhaps we should treat Content-length of 0 like "no Conent-Length
> field".
> 
Ahh, I found the root of the problem ;)

Since the payload doesnt start with SIP/2.0, its interpreted as
a response. Starting with INVITE its interpreted as an INVITE
request. Since there is no CSeq header found, its dropped then as invalid.

Possible proper solutions would be either
a) add trailing space to sip_handlers commands
b) check for trailing SIP/2.0 in request line

Do you think a or b would break existing setups?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] netfilter: nf_tables: underflow in nft_parse_u32_check()

2016-10-12 Thread Liping Zhang 
2016-10-12 14:08 GMT+08:00 Dan Carpenter :
> We don't want to allow negatives here.
>
> Fixes: 36b701fae12a ('netfilter: nf_tables: validate maximum value of u32 
> netlink attributes')
> Signed-off-by: Dan Carpenter 
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index b70d3ea..dd55187 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -4423,7 +4423,7 @@ static int nf_tables_check_loops(const struct nft_ctx 
> *ctx,
>   */

I think it's better if you can convert it to follows:

>  unsigned int nft_parse_u32_check(const struct nlattr *attr, int max, u32 
> *dest)

int nft_parse_u32_check(const struct nlattr *attr, u32 max, u32 *dest)

>  {
> -   int val;
> +   uint val;

u32 val;

>
> val = ntohl(nla_get_be32(attr));
> if (val > max)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html