Page MenuHomeFreeBSD
Differential D37210 Diff 112389 sbin/ping/tests/ping_test.sh

Changeset View

Standalone View

View Options

sbin/ping/tests/ping_test.sh

Show First 20 Lines • Show All 119 Lines • ▼ Show 20 Lines
ping6_46_head() { ping6_46_head() {
atf_set "descr" "-4 and -6 may not be used together" atf_set "descr" "-4 and -6 may not be used together"
} }
ping6_46_body() { ping6_46_body() {
atf_check -s exit:1 -e ignore ping6 -4 -6 atf_check -s exit:1 -e ignore ping6 -4 -6
} }
atf_test_case "inject_opts" "cleanup"
inject_opts_head() {
markjUnsubmitted
Not Done
Inline Actions

Opening braces should be on their own line.

markj: Opening braces should be on their own line.
atf_set "descr" "Inejct an ECHO REPLY with IP options"
atf_set "require.user" "root"
atf_set "require.progs" "scapy"
}
inject_opts_body() {
atf_check -s exit:0 -o ignore python3 $(atf_get_srcdir)/injection.py opts
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

I wonder if the output should also be checked:
-o match:"wrong total length" -o match:"NOP"?

jlduran_gmail.com: I wonder if the output should also be checked: `-o match:"wrong total length" -o match:"NOP"`?
asomersAuthorUnsubmitted
Done
Inline Actions

Yeah, it should. I'll add that.

asomers: Yeah, it should. I'll add that.
}
inject_opts_cleanup() {
ifconfig `cat tun.txt` destroy
}
atf_test_case "inject_pip" "cleanup"
inject_pip_head() {
atf_set "descr" "Inject an ICMP error with a quoted packet with IP options"
atf_set "require.user" "root"
atf_set "require.progs" "scapy"
}
inject_pip_body() {
atf_check -s exit:2 -o ignore python3 $(atf_get_srcdir)/injection.py pip
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

Same here:
-o not-match:"01010101"?

jlduran_gmail.com: Same here: `-o not-match:"01010101"`?
asomersAuthorUnsubmitted
Done
Inline Actions

What's the significance of "01010101"?

asomers: What's the significance of "01010101"?
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

Without this patch, the 40 01s from injection.py will show. It replies:

PING 192.0.2.15 (192.0.2.15): 56 data bytes
132 bytes from 192.0.2.15: Destination Host Unreachable
Vr HL TOS  Len   ID Flg  off TTL Pro  cks      Src      Dst
 4  f  00 007c 0001   0 0000  40  01 d84e 192.0.2.15  192.0.2.14 01010101010101010101010101010101010101010101010101010101010101010101010101010101


--- 192.0.2.15 ping statistics ---
1 packets transmitted, 0 packets received, 100.0% packet loss

After the patch,:

PING 192.0.2.15 (192.0.2.15): 56 data bytes
132 bytes from 192.0.2.15: Destination Host Unreachable
Vr HL TOS  Len   ID Flg  off TTL Pro  cks      Src      Dst
 4  f  00 007c 0001   0 0000  40  01 d84e 192.0.2.15  192.0.2.14 16110000ee8b2005171100000301fcfe000000004f00007c000100004001d84ec000020fc000020e


--- 192.0.2.15 ping statistics ---
1 packets transmitted, 0 packets received, 100.0% packet loss

Destination Host Unreachable would match either way... But I guess what matters most is the exit code.

jlduran_gmail.com: Without this patch, the 40 `01`s from `injection.py` will show. It replies: ``` PING 192.0.2.15…
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

This is where I think the test might get brittle. If one runs the test suite a number of times enough to find a collision with this pattern. But I do not understand fully the idea behind it. I would have guessed testing for inner packet too short or something more trivial.

jlduran_gmail.com: This is where I think the test might get brittle. If one runs the test suite a number of times…
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

I believe the test should check for the 40 01s in the original IP header. At the moment is checking for the absence of 4 NOPs, I would guess this is wrong.

jlduran_gmail.com: I believe the test should check for the 40 `01`s in the original IP header. At the moment is…
asomersAuthorUnsubmitted
Done
Inline Actions

I think you were looking at an older version of the test? The latest one in the review does check for those 01 bytes.

asomers: I think you were looking at an older version of the test? The latest one in the review does…
}
inject_pip_cleanup() {
ifconfig `cat tun.txt` destroy
}
# This is redundant with the ping_ tests, but it serves to ensure that scapy.py
# is working correctly.
atf_test_case "inject_reply" "cleanup"
inject_reply_head() {
atf_set "descr" "Basic ping test with packet injection"
atf_set "require.user" "root"
atf_set "require.progs" "scapy"
}
inject_reply_body() {
atf_check -s exit:0 -o ignore python3 $(atf_get_srcdir)/injection.py reply
jlduran_gmail.comUnsubmitted
Not Done
Inline Actions

Even here:
-o match:"1 packets transmitted, 1 packets received"?

jlduran_gmail.com: Even here: `-o match:"1 packets transmitted, 1 packets received"`?
asomersAuthorUnsubmitted
Done
Inline Actions

yes

asomers: yes
}
inject_reply_cleanup() {
ifconfig `cat tun.txt` destroy
}
atf_init_test_cases() { atf_init_test_cases() {
atf_add_test_case ping_c1_s56_t1 atf_add_test_case ping_c1_s56_t1
atf_add_test_case ping_c1_s56_t1_S127 atf_add_test_case ping_c1_s56_t1_S127
atf_add_test_case ping_6_c1_s8_t1 atf_add_test_case ping_6_c1_s8_t1
atf_add_test_case ping_c1_s8_t1_S1 atf_add_test_case ping_c1_s8_t1_S1
atf_add_test_case ping6_c1_s8_t1 atf_add_test_case ping6_c1_s8_t1
atf_add_test_case ping_c1t6 atf_add_test_case ping_c1t6
atf_add_test_case ping6_c1t4 atf_add_test_case ping6_c1t4
atf_add_test_case ping_46 atf_add_test_case ping_46
atf_add_test_case ping6_46 atf_add_test_case ping6_46
atf_add_test_case inject_opts
atf_add_test_case inject_pip
atf_add_test_case inject_reply
} }
check_ping_statistics() { check_ping_statistics() {
sed -e 's/0.[0-9]\{3\}//g' \ sed -e 's/0.[0-9]\{3\}//g' \
-e 's/[1-9][0-9]*.[0-9]\{3\}//g' \ -e 's/[1-9][0-9]*.[0-9]\{3\}//g' \
-e 's/localhost ([0-9]\{1,3\}\(\.[0-9]\{1,3\}\)\{1,3\})/localhost/' \ -e 's/localhost ([0-9]\{1,3\}\(\.[0-9]\{1,3\}\)\{1,3\})/localhost/' \
-e 's/from [0-9]\{1,3\}\(\.[0-9]\{1,3\}\)\{1,3\}/from/' \ -e 's/from [0-9]\{1,3\}\(\.[0-9]\{1,3\}\)\{1,3\}/from/' \
-e 's/ttl=[0-9][0-9]*/ttl=/' \ -e 's/ttl=[0-9][0-9]*/ttl=/' \
-e 's/hlim=[0-9][0-9]*/hlim=/' \ -e 's/hlim=[0-9][0-9]*/hlim=/' \
"$1" >"$1".filtered "$1" >"$1".filtered
atf_check -s exit:0 diff -u "$1".filtered "$2" atf_check -s exit:0 diff -u "$1".filtered "$2"
} }