lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Mar 2019 12:59:48 +0100 From: Florian Westphal <fw@...len.de> To: Neil Horman <nhorman@...driver.com> Cc: Xin Long <lucien.xin@...il.com>, network dev <netdev@...r.kernel.org>, netfilter-devel@...r.kernel.org, Marcelo Ricardo Leitner <marcelo.leitner@...il.com>, pablo@...filter.org, fw@...len.de Subject: Re: [PATCH net] netfilter: bridge: set skb transport_header before entering NF_INET_PRE_ROUTING Neil Horman <nhorman@...driver.com> wrote: > On Wed, Mar 13, 2019 at 04:33:29PM +0800, Xin Long wrote: > > Since Commit 21d1196a35f5 ("ipv4: set transport header earlier"), > > skb->transport_header has been always set before entering INET > > netfilter. This patch is to set skb->transport_header for bridge > > before entering INET netfilter by bridge-nf-call-iptables. > > > > It also fixes an issue that sctp_error() couldn't compute a right > > csum due to unset skb->transport_header. > > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code") > > Reported-by: Li Shuang <shuali@...hat.com> > > Suggested-by: Pablo Neira Ayuso <pablo@...filter.org> > > Signed-off-by: Xin Long <lucien.xin@...il.com> > > --- > > net/bridge/br_netfilter_hooks.c | 1 + > > net/bridge/br_netfilter_ipv6.c | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c > > index c93c35b..4d09a33 100644 > > --- a/net/bridge/br_netfilter_hooks.c > > +++ b/net/bridge/br_netfilter_hooks.c > > @@ -502,6 +502,7 @@ static unsigned int br_nf_pre_routing(void *priv, > > nf_bridge->ipv4_daddr = ip_hdr(skb)->daddr; > > > > skb->protocol = htons(ETH_P_IP); > > + skb->transport_header = skb->network_header + ip_hdr(skb)->ihl * 4; > > > > NF_HOOK(NFPROTO_IPV4, NF_INET_PRE_ROUTING, state->net, state->sk, skb, > > skb->dev, NULL, > > diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c > > index 564710f..e88d664 100644 > > --- a/net/bridge/br_netfilter_ipv6.c > > +++ b/net/bridge/br_netfilter_ipv6.c > > @@ -235,6 +235,8 @@ unsigned int br_nf_pre_routing_ipv6(void *priv, > > nf_bridge->ipv6_daddr = ipv6_hdr(skb)->daddr; > > > > skb->protocol = htons(ETH_P_IPV6); > > + skb->transport_header = skb->network_header + sizeof(struct ipv6hdr); > > + > > NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, state->net, state->sk, skb, > > skb->dev, NULL, > > br_nf_pre_routing_finish_ipv6); > > -- > > 2.1.0 > > > > > Acked-by: Neil Horman <nhorman@...driver.com> Acked-by: Florian Westphal <fw@...len.de> ... because this fixes discrepancy of normal stack vs. bridge path. However, I think we still need a fix for the ipv6 case as there is no guarantee ipv6 nexthdr will be the sctp one. We already pass the sctp offset as argument to sctp_compute_cksum(), why can't we just replace sctp_hdr(skb) with skb->data + dataoff, as Xin Long originally suggested? IOW, whats the problem with https://marc.info/?l=linux-netdev&m=155109395226858&w=2 ? (In theory conntrack can also inspect transport header inside icmp error messages like pkttoobig and so on, although this won't be the case here as we can't validate csum anyway if the packet isn't complete). Just pointing out that we can't rely on skb transport header being "correct" in all cases from netfilter point of view.
Powered by blists - more mailing lists