Commit 39aa6928 authored by Florian Westphal's avatar Florian Westphal Committed by Steffen Klassert
Browse files

xfrm: policy: fix netlink/pf_key policy lookups 



Colin Ian King says:
 Static analysis with CoverityScan found a potential issue [..]
 It seems that pointer pol is set to NULL and then a check to see if it
 is non-null is used to set pol to tmp; howeverm this check is always
 going to be false because pol is always NULL.

Fix this and update test script to catch this.  Updated script only:
./xfrm_policy.sh ; echo $?
RTNETLINK answers: No such file or directory
FAIL: ip -net ns3 xfrm policy get src 10.0.1.0/24 dst 10.0.2.0/24 dir out
RTNETLINK answers: No such file or directory
[..]
PASS: policy before exception matches
PASS: ping to .254 bypassed ipsec tunnel
PASS: direct policy matches
PASS: policy matches
1

Fixes: 6be3b0db ("xfrm: policy: add inexact policy search tree infrastructure")
Reported-by: default avatarColin Ian King <colin.king@canonical.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
parent 7759d6a8

net/xfrm/xfrm_policy.c

+4 −1
Original line number Original line Diff line number Diff line
@@ -1663,7 +1663,10 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id, 
			tmp = __xfrm_policy_bysel_ctx(cand.res[i], mark,

			tmp = __xfrm_policy_bysel_ctx(cand.res[i], mark,

						      if_id, type, dir,

						      if_id, type, dir,

						      sel, ctx);

						      sel, ctx);

			if (tmp && pol && tmp->pos < pol->pos)

			if (!tmp)

				continue;



			if (!pol || tmp->pos < pol->pos)

				pol = tmp;

				pol = tmp;

		}

		}

	} else {

	} else {

tools/testing/selftests/net/xfrm_policy.sh

+32 −6
Original line number Original line Diff line number Diff line
@@ -21,6 +21,7 @@ 
# Kselftest framework requirement - SKIP code is 4.

# Kselftest framework requirement - SKIP code is 4.

ksft_skip=4

ksft_skip=4

ret=0

ret=0

policy_checks_ok=1





KEY_SHA=0xdeadbeef1234567890abcdefabcdefabcdefabcd

KEY_SHA=0xdeadbeef1234567890abcdefabcdefabcdefabcd

KEY_AES=0x0123456789abcdef0123456789012345

KEY_AES=0x0123456789abcdef0123456789012345
@@ -45,6 +46,26 @@ do_esp() { 
    ip -net $ns xfrm policy add src $rnet dst $lnet dir fwd tmpl src $remote dst $me proto esp mode tunnel priority 100 action allow

    ip -net $ns xfrm policy add src $rnet dst $lnet dir fwd tmpl src $remote dst $me proto esp mode tunnel priority 100 action allow

}

}





do_esp_policy_get_check() {

    local ns=$1

    local lnet=$2

    local rnet=$3



    ip -net $ns xfrm policy get src $lnet dst $rnet dir out > /dev/null

    if [ $? -ne 0 ] && [ $policy_checks_ok -eq 1 ] ;then

        policy_checks_ok=0

        echo "FAIL: ip -net $ns xfrm policy get src $lnet dst $rnet dir out"

        ret=1

    fi



    ip -net $ns xfrm policy get src $rnet dst $lnet dir fwd > /dev/null

    if [ $? -ne 0 ] && [ $policy_checks_ok -eq 1 ] ;then

        policy_checks_ok=0

        echo "FAIL: ip -net $ns xfrm policy get src $rnet dst $lnet dir fwd"

        ret=1

    fi

}



do_exception() {

do_exception() {

    local ns=$1

    local ns=$1

    local me=$2

    local me=$2
@@ -112,31 +133,31 @@ check_xfrm() { 
	# 1: iptables -m policy rule count != 0

	# 1: iptables -m policy rule count != 0

	rval=$1

	rval=$1

	ip=$2

	ip=$2

	ret=0

	lret=0





	ip netns exec ns1 ping -q -c 1 10.0.2.$ip > /dev/null

	ip netns exec ns1 ping -q -c 1 10.0.2.$ip > /dev/null





	check_ipt_policy_count ns3

	check_ipt_policy_count ns3

	if [ $? -ne $rval ] ; then

	if [ $? -ne $rval ] ; then

		ret=1

		lret=1

	fi

	fi

	check_ipt_policy_count ns4

	check_ipt_policy_count ns4

	if [ $? -ne $rval ] ; then

	if [ $? -ne $rval ] ; then

		ret=1

		lret=1

	fi

	fi





	ip netns exec ns2 ping -q -c 1 10.0.1.$ip > /dev/null

	ip netns exec ns2 ping -q -c 1 10.0.1.$ip > /dev/null





	check_ipt_policy_count ns3

	check_ipt_policy_count ns3

	if [ $? -ne $rval ] ; then

	if [ $? -ne $rval ] ; then

		ret=1

		lret=1

	fi

	fi

	check_ipt_policy_count ns4

	check_ipt_policy_count ns4

	if [ $? -ne $rval ] ; then

	if [ $? -ne $rval ] ; then

		ret=1

		lret=1

	fi

	fi





	return $ret

	return $lret

}

}





#check for needed privileges

#check for needed privileges
@@ -227,6 +248,11 @@ do_esp ns4 dead:3::10 dead:3::1 dead:2::/64 dead:1::/64 $SPI2 $SPI1 
do_dummies4 ns3

do_dummies4 ns3

do_dummies6 ns4

do_dummies6 ns4





do_esp_policy_get_check ns3 10.0.1.0/24 10.0.2.0/24

do_esp_policy_get_check ns4 10.0.2.0/24 10.0.1.0/24

do_esp_policy_get_check ns3 dead:1::/64 dead:2::/64

do_esp_policy_get_check ns4 dead:2::/64 dead:1::/64



# ping to .254 should use ipsec, exception is not installed.

# ping to .254 should use ipsec, exception is not installed.

check_xfrm 1 254

check_xfrm 1 254

if [ $? -ne 0 ]; then

if [ $? -ne 0 ]; then

CRA Git | Maintained and supported by SUSTech CRA and CCSE