From nobody Tue Nov 01 17:47:52 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4N1yCr5lXpz4gtcl; Tue, 1 Nov 2022 17:47:52 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4N1yCr58RWz48vQ; Tue, 1 Nov 2022 17:47:52 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1667324872; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=N+ueQUiRF6zJBmEjM+FAB82qjP7M/Sxlf1Gy9UCoJOs=; b=ktOeaej1v2Lv2WUgYjsJvFwyBbUiymbUPCARdzKls1oVRWtRc6hhHDRbgUb4wAlVKq5OuR DWrv477NWF85mRpBMvJqMO0cuuoP2ICxyJ1iol2kI5Od0hhC4eZZwarRucEjS2bszZEmxS uMifLed4mj8byWXorCj/v34xHGWUeQ2Fm7b8ohmcGMzbwyXzD4SSaYDdAnxzgTFBKPfeEW OtDgc2JXgdwgYPE52JhB5Kj7Qo1fm6ZoLK8ioncSSI0QYa46E5TsUyVM6eNfZPVf6H7rOW /nqRf2+z2iUp3UQfkj9wZozKbcyy4chAtuI8JBb3oW1oec8ZKLtRoLFv8nnLqw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4N1yCr499xzPgd; Tue, 1 Nov 2022 17:47:52 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 2A1Hlqck086268; Tue, 1 Nov 2022 17:47:52 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 2A1HlqEW086267; Tue, 1 Nov 2022 17:47:52 GMT (envelope-from git) Date: Tue, 1 Nov 2022 17:47:52 GMT Message-Id: <202211011747.2A1HlqEW086267@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 45258e1bc71b - main - pf tests: make killstate tests more robust List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 45258e1bc71b1dc4ec28e5d37c3ca6c02de61857 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1667324872; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=N+ueQUiRF6zJBmEjM+FAB82qjP7M/Sxlf1Gy9UCoJOs=; b=Aqf9X1QnRhebr2n43jniA94wS/Q/ZMeu16OfpdJ1QMihKIocWODST7/Oyu99WVBdOn5Opm xIXA4F+frpfL9aj7izsRDVyI3DVEyESCzRoZf/k4+rWPxWF2wEqBFZ6qWR7Td4P+pKemUD gZyf4FZgYBuSohZYJ4WJZvADir+c1j4Xf1E9svd55A/E5nMbmZw+AyWFmhEQvLKEli3YKH TQqMErNfMYYn+UT1CCH9wXJjGr3wa8jrnIosgOt+r5yQFc5MQKi1bnfcMqnXxIgkYrLK1q YAfIft3galB7Xuillh3ms3/Ez+Ne+Kt6FcaZWIRugk8rZwEm0gZtQDQufkPs/g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1667324872; a=rsa-sha256; cv=none; b=J2BySSgdUd+t9iknOMeTIYLzrW7xr4lxBs1x/8aVjdyUfnGauttsu5phbY9NTgU7e2eZ+c qzXEFdf81NSo0CMapw1aE8djtO8LfRGT7k3SaSRFVoNuVX/i+PCoqVTuJYOsXTV86GRWR3 R/7aevgpwU0aNzyntwTMtc9zlLyd/8xZlJkvFpGq2K8/axJHiXGHj8MkN5IqB2Mm7QuF+f enJ3FiYU2ZF+KdfL36EJG9KFvXbEp4N8sH+CoyWphyD7wV2D5rHhHzqpoAkoHHLXkBYGsi y/tx5NnIdhzUcmKtSFFAoVYIov8ENw8AaF82E5NLm2vZMYCoNashlLI33HxX+Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=45258e1bc71b1dc4ec28e5d37c3ca6c02de61857 commit 45258e1bc71b1dc4ec28e5d37c3ca6c02de61857 Author: Kristof Provost AuthorDate: 2022-11-01 17:03:50 +0000 Commit: Kristof Provost CommitDate: 2022-11-01 17:47:05 +0000 pf tests: make killstate tests more robust Rather than using a Scapy-based Python script only check if the state still exists. Scapy tends to be slow to start, it appears because it lists all interfaces and gets their (IPv6) addresses a couple of times at startup. This can be sufficient for the ICMP state to time out and the test to fail. We now only check if the state exists or is removed as expected, which makes things faster, and should mean the test is more robust on slower machines (such as CI VMs). Sponsored by: Rubicon Communications, LLC ("Netgate") --- tests/sys/netpfil/pf/killstate.sh | 252 +++++++++++++++++--------------------- 1 file changed, 115 insertions(+), 137 deletions(-) diff --git a/tests/sys/netpfil/pf/killstate.sh b/tests/sys/netpfil/pf/killstate.sh index a2b20d6d7cbf..61edec8252eb 100644 --- a/tests/sys/netpfil/pf/killstate.sh +++ b/tests/sys/netpfil/pf/killstate.sh @@ -29,6 +29,17 @@ common_dir=$(atf_get_srcdir)/../common +find_state() +{ + jexec alcatraz pfctl -ss | grep icmp | grep 192.0.2.2 +} + +find_state_v6() +{ + jexec alcatraz pfctl -ss | grep icmp | grep 2001:db8::2 +} + + atf_test_case "v4" "cleanup" v4_head() { @@ -52,8 +63,6 @@ v4_body() "pass in proto icmp" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --sendif ${epair}a \ --to 192.0.2.2 \ @@ -61,39 +70,31 @@ v4_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Killing with the wrong IP doesn't affect our state jexec alcatraz pfctl -k 192.0.2.3 - - # So we can still ping - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing with the wrong IP removed our state." + fi # Killing with one correct address and one incorrect doesn't kill the state jexec alcatraz pfctl -k 192.0.2.1 -k 192.0.2.3 - - # So we can still ping - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing with one wrong IP removed our state." + fi # Killing with correct address does remove the state jexec alcatraz pfctl -k 192.0.2.1 - - # Now the ping fails - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing with the correct IP did not remove our state." + fi } v4_cleanup() @@ -128,8 +129,6 @@ v6_body() "pass in proto icmp6" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --ip6 \ --sendif ${epair}a \ @@ -138,38 +137,31 @@ v6_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --ip6 \ - --sendif ${epair}a \ - --to 2001:db8::2 \ - --replyif ${epair}a + if ! find_state_v6; + then + atf_fail "Setting new rules removed the state." + fi # Killing with the wrong IP doesn't affect our state jexec alcatraz pfctl -k 2001:db8::3 - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --ip6 \ - --sendif ${epair}a \ - --to 2001:db8::2 \ - --replyif ${epair}a + if ! find_state_v6; + then + atf_fail "Killing with the wrong IP removed our state." + fi # Killing with one correct address and one incorrect doesn't kill the state jexec alcatraz pfctl -k 2001:db8::1 -k 2001:db8::3 - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --ip6 \ - --sendif ${epair}a \ - --to 2001:db8::2 \ - --replyif ${epair}a + if ! find_state_v6; + then + atf_fail "Killing with one wrong IP removed our state." + fi # Killing with correct address does remove the state jexec alcatraz pfctl -k 2001:db8::1 - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --ip6 \ - --sendif ${epair}a \ - --to 2001:db8::2 \ - --replyif ${epair}a - + if find_state_v6; + then + atf_fail "Killing with the correct IP did not remove our state." + fi } v6_cleanup() @@ -201,8 +193,6 @@ label_body() "pass in proto icmp label foo" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --sendif ${epair}a \ --to 192.0.2.2 \ @@ -210,33 +200,31 @@ label_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Killing a label on a different rules keeps the state jexec alcatraz pfctl -k label -k bar - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing a different label removed the state." + fi # Killing a non-existing label keeps the state jexec alcatraz pfctl -k label -k baz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing a non-existing label removed the state." + fi # Killing the correct label kills the state jexec alcatraz pfctl -k label -k foo - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing the state did not remove it." + fi } label_cleanup() @@ -267,8 +255,6 @@ multilabel_body() "pass in proto icmp label foo label bar" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --sendif ${epair}a \ --to 192.0.2.2 \ @@ -276,26 +262,24 @@ multilabel_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Killing a label on a different rules keeps the state jexec alcatraz pfctl -k label -k baz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing a different label removed the state." + fi # Killing the state with the last label works jexec alcatraz pfctl -k label -k bar - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing with the last label did not remove the state." + fi pft_set_rules alcatraz "block all" \ "pass in proto icmp label foo label bar" @@ -308,13 +292,17 @@ multilabel_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Killing with the first label works too jexec alcatraz pfctl -k label -k foo - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing with the first label did not remove the state." + fi } multilabel_cleanup() @@ -354,26 +342,24 @@ gateway_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Killing with a different gateway does not affect our state jexec alcatraz pfctl -k gateway -k 192.0.2.2 - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing with a different gateway removed the state." + fi # Killing states with the relevant gateway does terminate our state jexec alcatraz pfctl -k gateway -k 192.0.2.1 - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing with the gateway did not remove the state." + fi } gateway_cleanup() @@ -487,8 +473,6 @@ interface_body() "pass in proto icmp" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --sendif ${epair}a \ --to 192.0.2.2 \ @@ -496,26 +480,24 @@ interface_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Flushing states on a different interface doesn't affect our state jexec alcatraz pfctl -i ${epair}a -Fs - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Flushing on a different interface removed the state." + fi # Flushing on the correct interface does (even with floating states) jexec alcatraz pfctl -i ${epair}b -Fs - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Flushing on a the interface did not remove the state." + fi } interface_cleanup() @@ -547,8 +529,6 @@ id_body() "pass in proto icmp" # Sanity check & establish state - # Note: use pft_ping so we always use the same ID, so pf considers all - # echo requests part of the same flow. atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ --sendif ${epair}a \ --to 192.0.2.2 \ @@ -556,12 +536,10 @@ id_body() # Change rules to now deny the ICMP traffic pft_set_rules noflush alcatraz "block all" - - # Established state means we can still ping alcatraz - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Setting new rules removed the state." + fi # Get the state ID id=$(jexec alcatraz pfctl -ss -vvv | grep -A 3 icmp | @@ -569,17 +547,17 @@ id_body() # Kill the wrong ID jexec alcatraz pfctl -k id -k 1 - atf_check -s exit:0 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if ! find_state; + then + atf_fail "Killing a different ID removed the state." + fi # Kill the correct ID jexec alcatraz pfctl -k id -k ${id} - atf_check -s exit:1 -o ignore ${common_dir}/pft_ping.py \ - --sendif ${epair}a \ - --to 192.0.2.2 \ - --replyif ${epair}a + if find_state; + then + atf_fail "Killing the state did not remove it." + fi } id_cleanup()