Skip to contentSkip to conversation

ipv4: Ensure ECN bits don't influence source address validation

PATCH Applied Guillaume Nault 2 participants 4 messages Jan 16, 2021 – Jan 19, 2021

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover ipv4: Ensure ECN bits don't influence source address validation

Functions that end up calling fib_table_lookup() should clear the ECN bits from the TOS, otherwise ECT(0) and ECT(1) packets can be treated differently.

Most functions already clear the ECN bits, but there are a few cases where this is not done. This series only fixes the ones related to source address validation.

Guillaume Nault (2):
udp: mask TOS bits in udp_v4_early_demux()
netfilter: rpfilter: mask ecn bits before fib lookup

 net/ipv4/netfilter/ipt_rpfilter.c | 2 +-
 net/ipv4/udp.c                    | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
Signature · 1 line
2.21.3
#1 lore raw

PATCH 1/2 udp: mask TOS bits in udp_v4_early_demux()

udp_v4_early_demux() is the only function that calls ip_mc_validate_source() with a TOS that hasn't been masked with IPTOS_RT_MASK.

This results in different behaviours for incoming multicast UDPv4 packets, depending on if ip_mc_validate_source() is called from the early-demux path (udp_v4_early_demux) or from the regular input path (ip_route_input_noref).

ECN would normally not be used with UDP multicast packets, so the practical consequences should be limited on that side. However, IPTOS_RT_MASK is used to also masks the TOS' high order bits, to align with the non-early-demux path behaviour.

Reproducer:

Setup two netns, connected with veth:

  $ ip netns add ns0
  $ ip netns add ns1
  $ ip -netns ns0 link set dev lo up
  $ ip -netns ns1 link set dev lo up
  $ ip link add name veth01 netns ns0 type veth peer name veth10 netns ns1
  $ ip -netns ns0 link set dev veth01 up
  $ ip -netns ns1 link set dev veth10 up
  $ ip -netns ns0 address add 192.0.2.10 peer 192.0.2.11/32 dev veth01
  $ ip -netns ns1 address add 192.0.2.11 peer 192.0.2.10/32 dev veth10

In ns0, add route to multicast address 224.0.2.0/24 using source address 198.51.100.10:

  $ ip -netns ns0 address add 198.51.100.10/32 dev lo
  $ ip -netns ns0 route add 224.0.2.0/24 dev veth01 src 198.51.100.10

In ns1, define route to 198.51.100.10, only for packets with TOS 4:

  $ ip -netns ns1 route add 198.51.100.10/32 tos 4 dev veth10

Also activate rp_filter in ns1, so that incoming packets not matching the above route get dropped:

  $ ip netns exec ns1 sysctl -wq net.ipv4.conf.veth10.rp_filter=1

Now try to receive packets on 224.0.2.11:

  $ ip netns exec ns1 socat UDP-RECVFROM:1111,ip-add-membership=224.0.2.11:veth10,ignoreeof -

In ns0, send packet to 224.0.2.11 with TOS 4 and ECT(0) (that is, tos 6 for socat):

  $ echo test0 | ip netns exec ns0 socat - UDP-DATAGRAM:224.0.2.11:1111,bind=:1111,tos=6

The "test0" message is properly received by socat in ns1, because early-demux has no cached dst to use, so source address validation is done by ip_route_input_mc(), which receives a TOS that has the ECN bits masked.

Now send another packet to 224.0.2.11, still with TOS 4 and ECT(0):

  $ echo test1 | ip netns exec ns0 socat - UDP-DATAGRAM:224.0.2.11:1111,bind=:1111,tos=6

The "test1" message isn't received by socat in ns1, because, now, early-demux has a cached dst to use and calls ip_mc_validate_source() immediately, without masking the ECN bits.

Fixes bc044e8db796 ("udp: perform source validation for mcast early demux")
---
 net/ipv4/udp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/udp.c +2 -1
index 7103b0a89756..69ea76578abb 100644
@@ -2555,7 +2555,8 @@ int udp_v4_early_demux(struct sk_buff *skb)
 		 */
 		if (!inet_sk(sk)->inet_daddr && in_dev)
 			return ip_mc_validate_source(skb, iph->daddr,
-						     iph->saddr, iph->tos,
+						     iph->saddr,
+						     iph->tos & IPTOS_RT_MASK,
 						     skb->dev, in_dev, &itag);
 	}
 	return 0;
Signature · 1 line
2.21.3
#2 lore raw

PATCH 2/2 netfilter: rpfilter: mask ecn bits before fib lookup

In reply to Guillaume Nault

RT_TOS() only masks one of the two ECN bits. Therefore rpfilter_mt() treats Not-ECT or ECT(1) packets in a different way than those with ECT(0) or CE.

Reproducer:

Create two netns, connected with a veth:

  $ ip netns add ns0
  $ ip netns add ns1
  $ ip link add name veth01 netns ns0 type veth peer name veth10 netns ns1
  $ ip -netns ns0 link set dev veth01 up
  $ ip -netns ns1 link set dev veth10 up
  $ ip -netns ns0 address add 192.0.2.10/32 dev veth01
  $ ip -netns ns1 address add 192.0.2.11/32 dev veth10

Add a route to ns1 in ns0:

  $ ip -netns ns0 route add 192.0.2.11/32 dev veth01

In ns1, only packets with TOS 4 can be routed to ns0:

  $ ip -netns ns1 route add 192.0.2.10/32 tos 4 dev veth10

Ping from ns0 to ns1 works regardless of the ECN bits, as long as TOS is 4:

  $ ip netns exec ns0 ping -Q 4 192.0.2.11   # TOS 4, Not-ECT
    ... 0% packet loss ...
  $ ip netns exec ns0 ping -Q 5 192.0.2.11   # TOS 4, ECT(1)
    ... 0% packet loss ...
  $ ip netns exec ns0 ping -Q 6 192.0.2.11   # TOS 4, ECT(0)
    ... 0% packet loss ...
  $ ip netns exec ns0 ping -Q 7 192.0.2.11   # TOS 4, CE
    ... 0% packet loss ...

Now use iptable's rpfilter module in ns1:

  $ ip netns exec ns1 iptables-legacy -t raw -A PREROUTING -m rpfilter --invert -j DROP

Not-ECT and ECT(1) packets still pass:

  $ ip netns exec ns0 ping -Q 4 192.0.2.11   # TOS 4, Not-ECT
    ... 0% packet loss ...
  $ ip netns exec ns0 ping -Q 5 192.0.2.11   # TOS 4, ECT(1)
    ... 0% packet loss ...

But ECT(0) and ECN packets are dropped:

  $ ip netns exec ns0 ping -Q 6 192.0.2.11   # TOS 4, ECT(0)
    ... 100% packet loss ...
  $ ip netns exec ns0 ping -Q 7 192.0.2.11   # TOS 4, CE
    ... 100% packet loss ...

After this patch, rpfilter doesn't drop ECT(0) and CE packets anymore.

Fixes 8f97339d3feb ("netfilter: add ipv4 reverse path filter match")
---
 net/ipv4/netfilter/ipt_rpfilter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv4/netfilter/ipt_rpfilter.c +1 -1
index cc23f1ce239c..8cd3224d913e 100644
@@ -76,7 +76,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	flow.daddr = iph->saddr;
 	flow.saddr = rpfilter_get_saddr(iph->daddr);
 	flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
-	flow.flowi4_tos = RT_TOS(iph->tos);
+	flow.flowi4_tos = iph->tos & IPTOS_RT_MASK;
 	flow.flowi4_scope = RT_SCOPE_UNIVERSE;
 	flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
 
Signature · 1 line
2.21.3
#3 lore raw
In reply to Guillaume Nault
This series was applied to netdev/net.git (refs/heads/master):

Hello:

This series was applied to netdev/net.git (refs/heads/master):

Quoted 9 lines · On Sat, 16 Jan 2021 11:44:18 +0100 you wrote:

Functions that end up calling fib_table_lookup() should clear the ECN bits from the TOS, otherwise ECT(0) and ECT(1) packets can be treated differently.

Most functions already clear the ECN bits, but there are a few cases where this is not done. This series only fixes the ones related to source address validation.

[...]

Here is the summary with links:
- [net,1/2] udp: mask TOS bits in udp_v4_early_demux()

    https://git.kernel.org/netdev/net/c/8d2b51b008c2
  - [net,2/2] netfilter: rpfilter: mask ecn bits before fib lookup
    https://git.kernel.org/netdev/net/c/2e5a6266fbb1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html