[PATCH nf-next v2 0/2] netfilter: Dissect flow after packet mangling

[PATCH nf-next v2 0/2] netfilter: Dissect flow after packet mangling

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Patch #1 fixes a day-one bug in the interaction between netfilter and
sport/dport/ipproto FIB rule keys.

Patch #2 adds a corresponding test case.

Targeting at nf-next since this use case never worked.

v2:
* Add a test case

Ido Schimmel (2):
  netfilter: Dissect flow after packet mangling
  selftests: fib_tests: Add test cases for interaction with mangling

 net/ipv4/netfilter.c                     |   2 +
 net/ipv6/netfilter.c                     |   2 +
 tools/testing/selftests/net/fib_tests.sh | 152 ++++++++++++++++++++++-
 3 files changed, 155 insertions(+), 1 deletion(-)

-- 
2.30.2

[PATCH nf-next v2 1/2] netfilter: Dissect flow after packet mangling

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Netfilter tries to reroute mangled packets as a different route might
need to be used following the mangling. When this happens, netfilter
does not populate the IP protocol, the source port and the destination
port in the flow key. Therefore, FIB rules that match on these fields
are ignored and packets can be misrouted.

Solve this by dissecting the outer flow and populating the flow key
before rerouting the packet. Note that flow dissection only happens when
FIB rules that match on these fields are installed, so in the common
case there should not be a penalty.

Reported-by: Michal Soltys <msoltyspl@yandex.pl>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 net/ipv4/netfilter.c | 2 ++
 net/ipv6/netfilter.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index 7c841037c533..aff707988e23 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -25,6 +25,7 @@ int ip_route_me_harder(struct net *net, struct sock *sk, struct sk_buff *skb, un
 	__be32 saddr = iph->saddr;
 	__u8 flags;
 	struct net_device *dev = skb_dst(skb)->dev;
+	struct flow_keys flkeys;
 	unsigned int hh_len;
 
 	sk = sk_to_full_sk(sk);
@@ -48,6 +49,7 @@ int ip_route_me_harder(struct net *net, struct sock *sk, struct sk_buff *skb, un
 		fl4.flowi4_oif = l3mdev_master_ifindex(dev);
 	fl4.flowi4_mark = skb->mark;
 	fl4.flowi4_flags = flags;
+	fib4_rules_early_flow_dissect(net, skb, &fl4, &flkeys);
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
 		return PTR_ERR(rt);
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index ab9a279dd6d4..6ab710b5a1a8 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -24,6 +24,7 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct sock *sk = sk_to_full_sk(sk_partial);
+	struct flow_keys flkeys;
 	unsigned int hh_len;
 	struct dst_entry *dst;
 	int strict = (ipv6_addr_type(&iph->daddr) &
@@ -38,6 +39,7 @@ int ip6_route_me_harder(struct net *net, struct sock *sk_partial, struct sk_buff
 	};
 	int err;
 
+	fib6_rules_early_flow_dissect(net, skb, &fl6, &flkeys);
 	dst = ip6_route_output(net, sk, &fl6);
 	err = dst->error;
 	if (err) {
-- 
2.30.2

[PATCH nf-next v2 2/2] selftests: fib_tests: Add test cases for interaction with mangling

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Test that packets are correctly routed when netfilter mangling rules are
present.

Without previous patch:

 # ./fib_tests.sh -t ipv4_mangle

 IPv4 mangling tests
     TEST:     Connection with correct parameters                        [ OK ]
     TEST:     Connection with incorrect parameters                      [ OK ]
     TEST:     Connection with correct parameters - mangling             [FAIL]
     TEST:     Connection with correct parameters - no mangling          [ OK ]
     TEST:     Connection check - server side                            [FAIL]

 Tests passed:   3
 Tests failed:   2

 # ./fib_tests.sh -t ipv6_mangle

 IPv6 mangling tests
     TEST:     Connection with correct parameters                        [ OK ]
     TEST:     Connection with incorrect parameters                      [ OK ]
     TEST:     Connection with correct parameters - mangling             [FAIL]
     TEST:     Connection with correct parameters - no mangling          [ OK ]
     TEST:     Connection check - server side                            [FAIL]

 Tests passed:   3
 Tests failed:   2

With previous patch:

 # ./fib_tests.sh -t ipv4_mangle

 IPv4 mangling tests
     TEST:     Connection with correct parameters                        [ OK ]
     TEST:     Connection with incorrect parameters                      [ OK ]
     TEST:     Connection with correct parameters - mangling             [ OK ]
     TEST:     Connection with correct parameters - no mangling          [ OK ]
     TEST:     Connection check - server side                            [ OK ]

 Tests passed:   5
 Tests failed:   0

 # ./fib_tests.sh -t ipv6_mangle

 IPv6 mangling tests
     TEST:     Connection with correct parameters                        [ OK ]
     TEST:     Connection with incorrect parameters                      [ OK ]
     TEST:     Connection with correct parameters - mangling             [ OK ]
     TEST:     Connection with correct parameters - no mangling          [ OK ]
     TEST:     Connection check - server side                            [ OK ]

 Tests passed:   5
 Tests failed:   0

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 tools/testing/selftests/net/fib_tests.sh | 152 ++++++++++++++++++++++-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 2b5707738609..76d9487fb03c 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -9,7 +9,7 @@ ret=0
 ksft_skip=4
 
 # all tests in this script. Can be overridden with -t option
-TESTS="unregister down carrier nexthop suppress ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr"
+TESTS="unregister down carrier nexthop suppress ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr ipv4_mangle ipv6_mangle"
 
 VERBOSE=0
 PAUSE_ON_FAIL=no
@@ -1653,6 +1653,154 @@ ipv4_route_v6_gw_test()
 	route_cleanup
 }
 
+socat_check()
+{
+	if [ ! -x "$(command -v socat)" ]; then
+		echo "socat command not found. Skipping test"
+		return 1
+	fi
+
+	return 0
+}
+
+iptables_check()
+{
+	iptables -t mangle -L OUTPUT &> /dev/null
+	if [ $? -ne 0 ]; then
+		echo "iptables configuration not supported. Skipping test"
+		return 1
+	fi
+
+	return 0
+}
+
+ip6tables_check()
+{
+	ip6tables -t mangle -L OUTPUT &> /dev/null
+	if [ $? -ne 0 ]; then
+		echo "ip6tables configuration not supported. Skipping test"
+		return 1
+	fi
+
+	return 0
+}
+
+ipv4_mangle_test()
+{
+	local rc
+
+	echo
+	echo "IPv4 mangling tests"
+
+	socat_check || return 1
+	iptables_check || return 1
+
+	route_setup
+	sleep 2
+
+	local tmp_file=$(mktemp)
+	ip netns exec ns2 socat UDP4-LISTEN:54321,fork $tmp_file &
+
+	# Add a FIB rule and a route that will direct our connection to the
+	# listening server.
+	$IP rule add pref 100 ipproto udp sport 12345 dport 54321 table 123
+	$IP route add table 123 172.16.101.0/24 dev veth1
+
+	# Add an unreachable route to the main table that will block our
+	# connection in case the FIB rule is not hit.
+	$IP route add unreachable 172.16.101.2/32
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP4:172.16.101.2:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters"
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP4:172.16.101.2:54321,sourceport=11111"
+	log_test $? 1 "    Connection with incorrect parameters"
+
+	# Add a mangling rule and make sure connection is still successful.
+	$NS_EXEC iptables -t mangle -A OUTPUT -j MARK --set-mark 1
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP4:172.16.101.2:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters - mangling"
+
+	# Delete the mangling rule and make sure connection is still
+	# successful.
+	$NS_EXEC iptables -t mangle -D OUTPUT -j MARK --set-mark 1
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP4:172.16.101.2:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters - no mangling"
+
+	# Verify connections were indeed successful on server side.
+	[[ $(cat $tmp_file | wc -l) -eq 3 ]]
+	log_test $? 0 "    Connection check - server side"
+
+	$IP route del unreachable 172.16.101.2/32
+	$IP route del table 123 172.16.101.0/24 dev veth1
+	$IP rule del pref 100
+
+	{ kill %% && wait %%; } 2>/dev/null
+	rm $tmp_file
+
+	route_cleanup
+}
+
+ipv6_mangle_test()
+{
+	local rc
+
+	echo
+	echo "IPv6 mangling tests"
+
+	socat_check || return 1
+	ip6tables_check || return 1
+
+	route_setup
+	sleep 2
+
+	local tmp_file=$(mktemp)
+	ip netns exec ns2 socat UDP6-LISTEN:54321,fork $tmp_file &
+
+	# Add a FIB rule and a route that will direct our connection to the
+	# listening server.
+	$IP -6 rule add pref 100 ipproto udp sport 12345 dport 54321 table 123
+	$IP -6 route add table 123 2001:db8:101::/64 dev veth1
+
+	# Add an unreachable route to the main table that will block our
+	# connection in case the FIB rule is not hit.
+	$IP -6 route add unreachable 2001:db8:101::2/128
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP6:[2001:db8:101::2]:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters"
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP6:[2001:db8:101::2]:54321,sourceport=11111"
+	log_test $? 1 "    Connection with incorrect parameters"
+
+	# Add a mangling rule and make sure connection is still successful.
+	$NS_EXEC ip6tables -t mangle -A OUTPUT -j MARK --set-mark 1
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP6:[2001:db8:101::2]:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters - mangling"
+
+	# Delete the mangling rule and make sure connection is still
+	# successful.
+	$NS_EXEC ip6tables -t mangle -D OUTPUT -j MARK --set-mark 1
+
+	run_cmd "echo a | $NS_EXEC socat STDIN UDP6:[2001:db8:101::2]:54321,sourceport=12345"
+	log_test $? 0 "    Connection with correct parameters - no mangling"
+
+	# Verify connections were indeed successful on server side.
+	[[ $(cat $tmp_file | wc -l) -eq 3 ]]
+	log_test $? 0 "    Connection check - server side"
+
+	$IP -6 route del unreachable 2001:db8:101::2/128
+	$IP -6 route del table 123 2001:db8:101::/64 dev veth1
+	$IP -6 rule del pref 100
+
+	{ kill %% && wait %%; } 2>/dev/null
+	rm $tmp_file
+
+	route_cleanup
+}
+
 ################################################################################
 # usage
 
@@ -1725,6 +1873,8 @@ do
 	ipv6_route_metrics)		ipv6_route_metrics_test;;
 	ipv4_route_metrics)		ipv4_route_metrics_test;;
 	ipv4_route_v6_gw)		ipv4_route_v6_gw_test;;
+	ipv4_mangle)			ipv4_mangle_test;;
+	ipv6_mangle)			ipv6_mangle_test;;
 
 	help) echo "Test names: $TESTS"; exit 0;;
 	esac
-- 
2.30.2

Re: [PATCH nf-next v2 2/2] selftests: fib_tests: Add test cases for interaction with mangling

[David Ahern]

On 4/14/21 1:20 AM, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> Test that packets are correctly routed when netfilter mangling rules are
> present.
> 
> Without previous patch:
> 
>  # ./fib_tests.sh -t ipv4_mangle
> 
>  IPv4 mangling tests
>      TEST:     Connection with correct parameters                        [ OK ]
>      TEST:     Connection with incorrect parameters                      [ OK ]
>      TEST:     Connection with correct parameters - mangling             [FAIL]
>      TEST:     Connection with correct parameters - no mangling          [ OK ]
>      TEST:     Connection check - server side                            [FAIL]
> 
>  Tests passed:   3
>  Tests failed:   2
> 
>  # ./fib_tests.sh -t ipv6_mangle
> 
>  IPv6 mangling tests
>      TEST:     Connection with correct parameters                        [ OK ]
>      TEST:     Connection with incorrect parameters                      [ OK ]
>      TEST:     Connection with correct parameters - mangling             [FAIL]
>      TEST:     Connection with correct parameters - no mangling          [ OK ]
>      TEST:     Connection check - server side                            [FAIL]
> 
>  Tests passed:   3
>  Tests failed:   2
> 
> With previous patch:
> 
>  # ./fib_tests.sh -t ipv4_mangle
> 
>  IPv4 mangling tests
>      TEST:     Connection with correct parameters                        [ OK ]
>      TEST:     Connection with incorrect parameters                      [ OK ]
>      TEST:     Connection with correct parameters - mangling             [ OK ]
>      TEST:     Connection with correct parameters - no mangling          [ OK ]
>      TEST:     Connection check - server side                            [ OK ]
> 
>  Tests passed:   5
>  Tests failed:   0
> 
>  # ./fib_tests.sh -t ipv6_mangle
> 
>  IPv6 mangling tests
>      TEST:     Connection with correct parameters                        [ OK ]
>      TEST:     Connection with incorrect parameters                      [ OK ]
>      TEST:     Connection with correct parameters - mangling             [ OK ]
>      TEST:     Connection with correct parameters - no mangling          [ OK ]
>      TEST:     Connection check - server side                            [ OK ]
> 
>  Tests passed:   5
>  Tests failed:   0
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  tools/testing/selftests/net/fib_tests.sh | 152 ++++++++++++++++++++++-
>  1 file changed, 151 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Thanks, Ido.

Re: [PATCH nf-next v2 0/2] netfilter: Dissect flow after packet mangling

[Pablo Neira Ayuso]

On Wed, Apr 14, 2021 at 11:20:31AM +0300, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> Patch #1 fixes a day-one bug in the interaction between netfilter and
> sport/dport/ipproto FIB rule keys.
> 
> Patch #2 adds a corresponding test case.
> 
> Targeting at nf-next since this use case never worked.

Series applied, thanks Ido.