Skip to contentSkip to conversation

NFQUEUE: Fix bug with order of fanout and bypass

PATCH In review Shivani Bhardwaj 2 participants 5 messages Apr 14, 2016 – Apr 27, 2016 netfilter

lore
Share & tools
lore

Shivani Bhardwaj

author
#0 lore raw

PATCH 1/2 NFQUEUE: Fix bug with order of fanout and bypass

NFQUEUE had a bug with the ordering of fanout and bypass options which was arising due to same and odd values for flags and bypass when used together. Because of this, during bitwise ANDing of flags and NFQ_FLAG_CPU_FANOUT, the value always evaluated to false (since NFQ_FLAG_CPU_FANOUT=0x02) and led to skipping of fanout option whenever it was used before bypass because then flags would be 1.

Before this patch,

$ sudo iptables -A FORWARD -j NFQUEUE -p TCP --sport 80 --queue-balance 0:3 --queue-cpu-fanout --queue-bypass

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass

After this patch,

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass cpu-fanout

Closes bugzilla entry: http://bugzilla.netfilter.org/show_bug.cgi?id=939

Suggested-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Shivani Bhardwaj <[email protected]>
---
 extensions/libxt_NFQUEUE.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
extensions/libxt_NFQUEUE.c +1 -1
index 8115457..0b5becc 100644
@@ -99,7 +99,7 @@ static void NFQUEUE_parse_v2(struct xt_option_call *cb)
 	NFQUEUE_parse_v1(cb);
 	switch (cb->entry->id) {
 	case O_QUEUE_BYPASS:
-		info->bypass = 1;
+		info->bypass |= NFQ_FLAG_BYPASS;
 		break;
 	}
 }
Signature · 1 line
1.9.1
maintainer
#1 lore raw
Quoted 15 lines · Shivani Bhardwaj

NFQUEUE had a bug with the ordering of fanout and bypass options which was arising due to same and odd values for flags and bypass when used together. Because of this, during bitwise ANDing of flags and NFQ_FLAG_CPU_FANOUT, the value always evaluated to false (since NFQ_FLAG_CPU_FANOUT=0x02) and led to skipping of fanout option whenever it was used before bypass because then flags would be 1.

Before this patch,

$ sudo iptables -A FORWARD -j NFQUEUE -p TCP --sport 80 --queue-balance 0:3 --queue-cpu-fanout --queue-bypass

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass
Quoted 7 lines

After this patch,

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass cpu-fanout

Closes bugzilla entry: http://bugzilla.netfilter.org/show_bug.cgi?id=939

Shivani, thanks for following up on this.

Would you also update extensions/libxt_NFQUEUE.t to add a test so we make sure this regression doesn't happen ever again?

Thanks.

maintainer
#2 lore raw
Quoted 4 lines · Pablo Neira Ayuso

Shivani, thanks for following up on this.

Would you also update extensions/libxt_NFQUEUE.t to add a test so we make sure this regression doesn't happen ever again?

Just to clarify: No need to resend this patchset.

A follow up standalone patch to update the test part would be sufficient.

author
#3 lore raw
In reply to Pablo Neira Ayuso
Quoted 22 lines · Pablo Neira Ayuso
Quoted 21 lines · Shivani Bhardwaj

NFQUEUE had a bug with the ordering of fanout and bypass options which was arising due to same and odd values for flags and bypass when used together. Because of this, during bitwise ANDing of flags and NFQ_FLAG_CPU_FANOUT, the value always evaluated to false (since NFQ_FLAG_CPU_FANOUT=0x02) and led to skipping of fanout option whenever it was used before bypass because then flags would be 1.

Before this patch,

$ sudo iptables -A FORWARD -j NFQUEUE -p TCP --sport 80 --queue-balance 0:3 --queue-cpu-fanout --queue-bypass

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass

After this patch,

Chain FORWARD (policy ACCEPT)

target     prot opt source               destination
NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass cpu-fanout
Quoted 7 lines
Quoted 1 line

Closes bugzilla entry: http://bugzilla.netfilter.org/show_bug.cgi?id=939

Shivani, thanks for following up on this.

Would you also update extensions/libxt_NFQUEUE.t to add a test so we make sure this regression doesn't happen ever again?

I just did that! :) Testing and sending out the patch. Thanks.

Quoted 1 line

Thanks.

maintainer
#4 lore raw
In reply to Shivani Bhardwaj
Quoted 6 lines · Shivani Bhardwaj

NFQUEUE had a bug with the ordering of fanout and bypass options which was arising due to same and odd values for flags and bypass when used together. Because of this, during bitwise ANDing of flags and NFQ_FLAG_CPU_FANOUT, the value always evaluated to false (since NFQ_FLAG_CPU_FANOUT=0x02) and led to skipping of fanout option whenever it was used before bypass because then flags would be 1.

Applied, but please follow up with the test file update.

Thanks Shivani.