| Message ID | 1325597164-13459-2-git-send-email-richard@nod.at |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 3 Jan 2012 14:26:04 +0100 Richard Weinberger <richard@nod.at> wrote: > If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables > are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up. > > Signed-off-by: Richard Weinberger <richard@nod.at> I am not sure if this is a valid configuration. The setting of sysctl is saying "don't do iptables on bridge (since I won't be using it)" and then you are later doing iptables and expecting the settings as if the iptables setup was being done. Instead, you should just enable the net.bridge.bridge-nf-call-iptables sysctl. If a distro chooses to disable it then you may have to do it explicitly. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 03.01.2012 17:15, schrieb Stephen Hemminger: > On Tue, 3 Jan 2012 14:26:04 +0100 > Richard Weinberger <richard@nod.at> wrote: > >> If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables >> are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up. >> >> Signed-off-by: Richard Weinberger <richard@nod.at> > > I am not sure if this is a valid configuration. The setting of sysctl is saying > "don't do iptables on bridge (since I won't be using it)" and then you are later > doing iptables and expecting the settings as if the iptables setup was being > done. I don't think so. Also rules like this one are broken: iptables -A INPUT -i bridge0 -m physdev --physdev-in eth0 -j ... No firewalling is done on the bridge, xt_physdev is only using some meta information. At least a big fat warning would be nice that xt_physdev does not work if bridge-nf-call-iptables=0. It took me some time to figure out why my firewall rule set gone nuts on RHEL6... > Instead, you should just enable the net.bridge.bridge-nf-call-iptables sysctl. > If a distro chooses to disable it then you may have to do it explicitly. Fedora and RHEL have net.bridge.bridge-nf-call-iptables=0 per default due to KVM network performance issues. I'm sure I'm not the only user of xt_physdev on RHEL and friends. Thanks, //richard
Op 3/01/2012 18:42, Richard Weinberger schreef: > Am 03.01.2012 17:15, schrieb Stephen Hemminger: >> On Tue, 3 Jan 2012 14:26:04 +0100 >> Richard Weinberger<richard@nod.at> wrote: >> >>> If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables >>> are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up. >>> >>> Signed-off-by: Richard Weinberger<richard@nod.at> >> I am not sure if this is a valid configuration. The setting of sysctl is saying >> "don't do iptables on bridge (since I won't be using it)" and then you are later >> doing iptables and expecting the settings as if the iptables setup was being >> done. > I don't think so. > > Also rules like this one are broken: > iptables -A INPUT -i bridge0 -m physdev --physdev-in eth0 -j ... > > No firewalling is done on the bridge, xt_physdev is only using some meta > information. > > At least a big fat warning would be nice that xt_physdev does not work > if bridge-nf-call-iptables=0. > It took me some time to figure out why my firewall rule set gone nuts on > RHEL6... The documentation is probably not explicit enough, but I would keep the behavior as it is now. Setting bridge-nf-call-iptables to 0 makes iptables behave as if bridge-netfilter was not enabled at compilation. Anyway, your patch is almost certainly flawed since the fact that skb->nf_bridge can be NULL is used as part of the logic in br_netfilter.c: it indicates that bridge-nf-call-iptables was 0 when the packet was first processed by bridge-netfilter and should therefore not be given to iptables in any other netfilter hook. cheers, Bart
Am 03.01.2012 21:15, schrieb Bart De Schuymer: > The documentation is probably not explicit enough, but I would keep the > behavior as it is now. Setting bridge-nf-call-iptables to 0 makes > iptables behave as if bridge-netfilter was not enabled at compilation. > Anyway, your patch is almost certainly flawed since the fact that > skb->nf_bridge can be NULL is used as part of the logic in > br_netfilter.c: it indicates that bridge-nf-call-iptables was 0 when the > packet was first processed by bridge-netfilter and should therefore not > be given to iptables in any other netfilter hook. Thanks for the explanation! Wouldn't it make sense to check for bridge-nf-call-iptables in xt_physdev? So that the user gets warned that his iptables rule will never match... Thanks, //richard
Op 3/01/2012 21:29, Richard Weinberger schreef: > Am 03.01.2012 21:15, schrieb Bart De Schuymer: >> The documentation is probably not explicit enough, but I would keep the >> behavior as it is now. Setting bridge-nf-call-iptables to 0 makes >> iptables behave as if bridge-netfilter was not enabled at compilation. >> Anyway, your patch is almost certainly flawed since the fact that >> skb->nf_bridge can be NULL is used as part of the logic in >> br_netfilter.c: it indicates that bridge-nf-call-iptables was 0 when the >> packet was first processed by bridge-netfilter and should therefore not >> be given to iptables in any other netfilter hook. > Thanks for the explanation! > > Wouldn't it make sense to check for bridge-nf-call-iptables in xt_physdev? > So that the user gets warned that his iptables rule will never match... We don't want to introduce module dependencies between the bridge module and the iptables physdev match. We could add a message to the syslog whenever these proc settings are changed (in br_netfilter.c::brnf_sysctl_call_tables()). cheers, Bart
Am 04.01.2012 18:55, schrieb Bart De Schuymer: > Op 3/01/2012 21:29, Richard Weinberger schreef: >> Am 03.01.2012 21:15, schrieb Bart De Schuymer: >>> The documentation is probably not explicit enough, but I would keep the >>> behavior as it is now. Setting bridge-nf-call-iptables to 0 makes >>> iptables behave as if bridge-netfilter was not enabled at compilation. >>> Anyway, your patch is almost certainly flawed since the fact that >>> skb->nf_bridge can be NULL is used as part of the logic in >>> br_netfilter.c: it indicates that bridge-nf-call-iptables was 0 when the >>> packet was first processed by bridge-netfilter and should therefore not >>> be given to iptables in any other netfilter hook. >> Thanks for the explanation! >> >> Wouldn't it make sense to check for bridge-nf-call-iptables in >> xt_physdev? >> So that the user gets warned that his iptables rule will never match... > > We don't want to introduce module dependencies between the bridge module > and the iptables physdev match. CONFIG_NETFILTER_XT_MATCH_PHYSDEV depends anyway on CONFIG_BRIDGE_NETFILTER... > We could add a message to the syslog whenever these proc settings are > changed (in br_netfilter.c::brnf_sysctl_call_tables()). > Let's export brnf_call_iptables and brnf_call_ip6tables, such that physdev_mt_check() can notify the user that his iptables rule will have no effect. Thanks, //richard
Op 5/01/2012 0:13, Richard Weinberger schreef: > > Let's export brnf_call_iptables and brnf_call_ip6tables, such that > physdev_mt_check() can notify the user that his iptables rule will have > no effect. > I don't want to introduce a runtime dependency between the iptables physdev module and the bridge module. This should keep working: #modprobe bridge #modprobe xt_physdev #rmmod bridge It will stop working if you use exported symbols of the bridge module in the physdev module. Bart
Am 05.01.2012 20:50, schrieb Bart De Schuymer: > Op 5/01/2012 0:13, Richard Weinberger schreef: >> >> Let's export brnf_call_iptables and brnf_call_ip6tables, such that >> physdev_mt_check() can notify the user that his iptables rule will have >> no effect. >> > > I don't want to introduce a runtime dependency between the iptables > physdev module and the bridge module. > This should keep working: > #modprobe bridge > #modprobe xt_physdev > #rmmod bridge > It will stop working if you use exported symbols of the bridge module in > the physdev module. > IMHO this behavior would be useful. 8-) Removing bridge while xt_physdev is loaded will make some netfilter rules void. Which is not fun on a production firewall. Thanks, //richard
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index fa8b8f7..f38a8e4 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -576,10 +576,12 @@ static unsigned int br_nf_pre_routing_ipv6(unsigned int hook, struct sk_buff *skb, const struct net_device *in, const struct net_device *out, - int (*okfn)(struct sk_buff *)) + int (*okfn)(struct sk_buff *), + struct net_bridge *br) { const struct ipv6hdr *hdr; u32 pkt_len; + struct nf_bridge_info *nf_bridge; if (skb->len < sizeof(struct ipv6hdr)) return NF_DROP; @@ -606,6 +608,15 @@ static unsigned int br_nf_pre_routing_ipv6(unsigned int hook, nf_bridge_put(skb->nf_bridge); if (!nf_bridge_alloc(skb)) return NF_DROP; + + if (!brnf_call_ip6tables && !br->nf_call_ip6tables) { + nf_bridge = skb->nf_bridge; + nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING; + nf_bridge->physindev = skb->dev; + + return NF_ACCEPT; + } + if (!setup_pre_routing(skb)) return NF_DROP; @@ -629,6 +640,7 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb, { struct net_bridge_port *p; struct net_bridge *br; + struct nf_bridge_info *nf_bridge; __u32 len = nf_bridge_encap_header_len(skb); if (unlikely(!pskb_may_pull(skb, len))) @@ -641,16 +653,10 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb, if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb)) { - if (!brnf_call_ip6tables && !br->nf_call_ip6tables) - return NF_ACCEPT; - nf_bridge_pull_encap_header_rcsum(skb); - return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn); + return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn, br); } - if (!brnf_call_iptables && !br->nf_call_iptables) - return NF_ACCEPT; - if (skb->protocol != htons(ETH_P_IP) && !IS_VLAN_IP(skb) && !IS_PPPOE_IP(skb)) return NF_ACCEPT; @@ -663,6 +669,15 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb, nf_bridge_put(skb->nf_bridge); if (!nf_bridge_alloc(skb)) return NF_DROP; + + if (!brnf_call_iptables && !br->nf_call_iptables) { + nf_bridge = skb->nf_bridge; + nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING; + nf_bridge->physindev = skb->dev; + + return NF_ACCEPT; + } + if (!setup_pre_routing(skb)) return NF_DROP; store_orig_dstaddr(skb);
If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up. Signed-off-by: Richard Weinberger <richard@nod.at> --- net/bridge/br_netfilter.c | 31 +++++++++++++++++++++++-------- 1 files changed, 23 insertions(+), 8 deletions(-)