diff mbox

ebtables-restore 2.0.10-4 IPv6 rule loading bug

Message ID 20130702012723.D62F440000C68@1xj.tpn.terra.com
State Awaiting Upstream
Headers show

Commit Message

tdthp@terra.com.br July 2, 2013, 1:27 a.m. UTC 
Hi

So, I worked on this today, and it seems to be really an initialization problem. Here's my patch for 2.0.10-4. (Is there a better way to submit it? I'm new here.)

Do you have a test suit I could run to make sure I didn't break anything?

thanks,
-- Luís Fernando














Em Seg 1/07/13 04:33, Luis Fernando <tdthp@terra.com.br> escreveu:
Hello

I believe I found a bug in ebtables-restore.
I have the following set of rules (saved with ebtables-save):
# Generated by ebtables-save v1.0 on Tue Jun 25 16:15:00 BRT 2013
*filter
:INPUT ACCEPT
:FORWARD DROP
:OUTPUT ACCEPT
:INPUT-VLAN29 DROP
:OUTPUT-VLAN29 DROP
:OUTPUT-VLAN33 DROP
:OUTPUT-VLAN34 DROP
:OUTPUT-VLAN35 DROP
-A FORWARD -p IPv4 --ip-proto udp --ip-sport 67:68 --log-level info --log-prefix "dhcp_spoofing_denied" --log-ip -j DROP
-A FORWARD -p IPv6 --ip6-proto udp --ip6-sport 547 --log-level info --log-prefix "dhcp6_spoofing_denied" --log-ip -j DROP
-A FORWARD -p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type router-advertisement --log-level info --log-prefix "ra6_spoofing_denied" -j DROP
-A FORWARD -i eth1.29 -j INPUT-VLAN29
-A FORWARD -o eth1.29 -j OUTPUT-VLAN29
-A FORWARD -o eth2 -j ACCEPT
-A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
-A INPUT-VLAN29 -p IPv4 -s 0:50:56:0:0:15 -i eth1.29 --ip-src 10.133.145.120 -j RETURN
-A OUTPUT-VLAN29 -p IPv6 -d 0:50:56:0:0:15 -o eth1.29 --ip6-dst ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j ACCEPT
-A OUTPUT-VLAN29 -p IPv4 -d 0:50:56:0:0:15 -o eth1.29 --ip-dst 10.133.145.120 -j ACCEPT

When I load them manually (after flushing/deleting/creating the chains), they work. If I ebtables-restore them and then run the following commands, they also work:
ebtables -D INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
ebtables -I INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN

I noticed, though, that when I just use ebtables-restore, this rule was not being matched. After comparing a lot of --atomic-save dumps (working vs not-working), and reading a bit about ebtables structs, I found out that when I use restore, this is what is stored in the kernel (I'll color it to try to make it look easier for you):
http://i.imgur.com/E4t6f2o.png

When I run the commands manually, this is the result:
http://i.imgur.com/IJgqwKB.png

The colors (accordingly to ebt_ip6_info @ include/linux/netfilter_bridge/ebt_ip6.h ) represents:

BLUE -> source IPv6 address
RED -> dest IPv6 address
CYAN -> source IPv6 mask
ORANGE -> dest IPv6 mask

As you can see (bold), however, in the restored version there's trash in the source IPv6 address and mask. I'd put my money on a zeroing problem, but I couldn't find it so far.

There's a workaround for this issue, which is specifying an ip6-dst with mask 0:
removing -A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
adding -A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff --ip6-dst ::/:: -j RETURN

This probably forces the initialization of the dest ipv6 fields to zero.
Another interesting fact is that if the INPUT-VLAN29 IPv4 rule is commented out, this problem doesn't happen (maybe indicating that this rule is invading the memory of the IPv6 one).
I also noticed that if you comment most of the rules in this file, it will work correctly (that's why I had kept the OUTPUT-VLAN33/34/35 for instance).
So maybe it's related to the chain creation/replacing.

Any ideas on promising debugging I could do?

thanks,
-- Luís Fernando

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the  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

Comments

Bart De Schuymer July 2, 2013, 9:31 p.m. UTC | #1 
I think you found a kernel regression bug, introduced by the commit 
0898f99a dating back from 2010-03-08: it shouldn't have removed the 
check on info->bitmask & EBT_IP6_DEST and info->bitmask & EBT_IP6_SOURCE.
See: 
https://github.com/torvalds/linux/commit/0898f99a267f89a7dc72cc687955f17613a711b8
Can you reintroduce those checks in the kernel code (similar to what's 
done in the ebt_ip.c code), verify and submit the fix?
Your userspace patch is fine as a workaround. But I think it's best to 
patch the kernel to fix compatibility with the released ebtables 
versions. I'll also commit the userspace patch.

No test suite available, sorry...

Thanks for the effort,

Bart

Op 2/07/2013 3:27, tdthp@terra.com.br schreef:
> Hi
>
> So, I worked on this today, and it seems to be really an initialization problem. Here's my patch for 2.0.10-4. (Is there a better way to submit it? I'm new here.)
>
> Do you have a test suit I could run to make sure I didn't break anything?
>
> thanks,
> -- Luís Fernando
>
>
>
>
>
> --- extensions/ebt_ip6.c    2011-12-15 18:02:47.000000000 -0200
> +++ extensions/ebt_ip6_fixed.c    2013-07-01 21:55:56.000000000 -0300
> @@ -312,6 +312,10 @@
>   
>       ipinfo->invflags = 0;
>       ipinfo->bitmask = 0;
> +    memset(ipinfo->saddr.s6_addr, 0, sizeof(ipinfo->saddr.s6_addr));
> +    memset(ipinfo->smsk.s6_addr, 0, sizeof(ipinfo->smsk.s6_addr));
> +    memset(ipinfo->daddr.s6_addr, 0, sizeof(ipinfo->daddr.s6_addr));
> +    memset(ipinfo->dmsk.s6_addr, 0, sizeof(ipinfo->dmsk.s6_addr));
>   }
>   
>   #define OPT_SOURCE 0x01
>
>
>
>
>
>
>
>
>
> Em Seg 1/07/13 04:33, Luis Fernando <tdthp@terra.com.br> escreveu:
> Hello
>
> I believe I found a bug in ebtables-restore.
> I have the following set of rules (saved with ebtables-save):
> # Generated by ebtables-save v1.0 on Tue Jun 25 16:15:00 BRT 2013
> *filter
> :INPUT ACCEPT
> :FORWARD DROP
> :OUTPUT ACCEPT
> :INPUT-VLAN29 DROP
> :OUTPUT-VLAN29 DROP
> :OUTPUT-VLAN33 DROP
> :OUTPUT-VLAN34 DROP
> :OUTPUT-VLAN35 DROP
> -A FORWARD -p IPv4 --ip-proto udp --ip-sport 67:68 --log-level info --log-prefix "dhcp_spoofing_denied" --log-ip -j DROP
> -A FORWARD -p IPv6 --ip6-proto udp --ip6-sport 547 --log-level info --log-prefix "dhcp6_spoofing_denied" --log-ip -j DROP
> -A FORWARD -p IPv6 --ip6-proto ipv6-icmp --ip6-icmp-type router-advertisement --log-level info --log-prefix "ra6_spoofing_denied" -j DROP
> -A FORWARD -i eth1.29 -j INPUT-VLAN29
> -A FORWARD -o eth1.29 -j OUTPUT-VLAN29
> -A FORWARD -o eth2 -j ACCEPT
> -A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
> -A INPUT-VLAN29 -p IPv4 -s 0:50:56:0:0:15 -i eth1.29 --ip-src 10.133.145.120 -j RETURN
> -A OUTPUT-VLAN29 -p IPv6 -d 0:50:56:0:0:15 -o eth1.29 --ip6-dst ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j ACCEPT
> -A OUTPUT-VLAN29 -p IPv4 -d 0:50:56:0:0:15 -o eth1.29 --ip-dst 10.133.145.120 -j ACCEPT
>
> When I load them manually (after flushing/deleting/creating the chains), they work. If I ebtables-restore them and then run the following commands, they also work:
> ebtables -D INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
> ebtables -I INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
>
> I noticed, though, that when I just use ebtables-restore, this rule was not being matched. After comparing a lot of --atomic-save dumps (working vs not-working), and reading a bit about ebtables structs, I found out that when I use restore, this is what is stored in the kernel (I'll color it to try to make it look easier for you):
> http://i.imgur.com/E4t6f2o.png
>
> When I run the commands manually, this is the result:
> http://i.imgur.com/IJgqwKB.png
>
> The colors (accordingly to ebt_ip6_info @ include/linux/netfilter_bridge/ebt_ip6.h ) represents:
>
> BLUE -> source IPv6 address
> RED -> dest IPv6 address
> CYAN -> source IPv6 mask
> ORANGE -> dest IPv6 mask
>
> As you can see (bold), however, in the restored version there's trash in the source IPv6 address and mask. I'd put my money on a zeroing problem, but I couldn't find it so far.
>
> There's a workaround for this issue, which is specifying an ip6-dst with mask 0:
> removing -A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff -j RETURN
> adding -A INPUT-VLAN29 -p IPv6 -s 0:50:56:0:0:15 -i eth1.29 --ip6-src ::250:56ff:fe00:15/::ffff:ffff:ffff:ffff --ip6-dst ::/:: -j RETURN
>
> This probably forces the initialization of the dest ipv6 fields to zero.
> Another interesting fact is that if the INPUT-VLAN29 IPv4 rule is commented out, this problem doesn't happen (maybe indicating that this rule is invading the memory of the IPv6 one).
> I also noticed that if you comment most of the rules in this file, it will work correctly (that's why I had kept the OUTPUT-VLAN33/34/35 for instance).
> So maybe it's related to the chain creation/replacing.
>
> Any ideas on promising debugging I could do?
>
> thanks,
> -- Luís Fernando
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the  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
>

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

--- extensions/ebt_ip6.c    2011-12-15 18:02:47.000000000 -0200
+++ extensions/ebt_ip6_fixed.c    2013-07-01 21:55:56.000000000 -0300
@@ -312,6 +312,10 @@ 
 
     ipinfo->invflags = 0;
     ipinfo->bitmask = 0;
+    memset(ipinfo->saddr.s6_addr, 0, sizeof(ipinfo->saddr.s6_addr));
+    memset(ipinfo->smsk.s6_addr, 0, sizeof(ipinfo->smsk.s6_addr));
+    memset(ipinfo->daddr.s6_addr, 0, sizeof(ipinfo->daddr.s6_addr));
+    memset(ipinfo->dmsk.s6_addr, 0, sizeof(ipinfo->dmsk.s6_addr));
 }
 
 #define OPT_SOURCE 0x01