From 30b8132ca93d66573f53e28bc4ebbfbe7608a854 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Oct 2021 11:20:27 +0200 Subject: [PATCH 1/7] Remove redundant "should we skip?" block There's a second one just below. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f3e0f05c19..4f010ca340 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -634,14 +634,6 @@ run_test() { esac fi - # should we skip? - if [ "X$SKIP_NEXT" = "XYES" ]; then - SKIP_NEXT="NO" - echo "SKIP" - SKIPS=$(( $SKIPS + 1 )) - return - fi - # does this test use a proxy? if [ "X$1" = "X-p" ]; then PXY_CMD="$2" From 87b036f57271256d3bd98b447a3f544f1aa8a9d0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 21 Oct 2021 11:11:51 +0200 Subject: [PATCH 2/7] Add trivial record_outcome function to facilitate backports Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4f010ca340..a2410b7fab 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -439,9 +439,14 @@ print_name() { } +# Trivial function for compatibility with later Mbed TLS versions +record_outcome() { + echo "$1" +} + # fail fail() { - echo "FAIL" + record_outcome "FAIL" "$1" echo " ! $1" mv $SRV_OUT o-srv-${TESTS}.log @@ -657,7 +662,7 @@ run_test() { # should we skip? if [ "X$SKIP_NEXT" = "XYES" ]; then SKIP_NEXT="NO" - echo "SKIP" + record_outcome "SKIP" SKIPS=$(( $SKIPS + 1 )) return fi @@ -856,7 +861,7 @@ run_test() { fi # if we're here, everything is ok - echo "PASS" + record_outcome "PASS" if [ "$PRESERVE_LOGS" -gt 0 ]; then mv $SRV_OUT o-srv-${TESTS}.log mv $CLI_OUT o-cli-${TESTS}.log From 342147a8a8880557fc028b2790a54a926e10fd94 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Oct 2021 16:25:10 +0200 Subject: [PATCH 3/7] Move some code of run_test into auxiliary functions No behavior change. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 225 +++++++++++++++++++++++++++-------------------- 1 file changed, 128 insertions(+), 97 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index a2410b7fab..184666cf1b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -611,62 +611,12 @@ detect_dtls() { esac } -# Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] -# Options: -s pattern pattern that must be present in server output -# -c pattern pattern that must be present in client output -# -u pattern lines after pattern must be unique in client output -# -f call shell function on client output -# -S pattern pattern that must be absent in server output -# -C pattern pattern that must be absent in client output -# -U pattern lines after pattern must be unique in server output -# -F call shell function on server output -run_test() { - NAME="$1" - shift 1 - - if is_excluded "$NAME"; then - SKIP_NEXT="NO" - return - fi - - print_name "$NAME" - - # Do we only run numbered tests? - if [ -n "$RUN_TEST_NUMBER" ]; then - case ",$RUN_TEST_NUMBER," in - *",$TESTS,"*) :;; - *) SKIP_NEXT="YES";; - esac - fi - - # does this test use a proxy? - if [ "X$1" = "X-p" ]; then - PXY_CMD="$2" - shift 2 - else - PXY_CMD="" - fi - - # get commands and client output - SRV_CMD="$1" - CLI_CMD="$2" - CLI_EXPECT="$3" - shift 3 - - # Check if test uses files - case "$SRV_CMD $CLI_CMD" in - *data_files/*) - requires_config_enabled MBEDTLS_FS_IO;; - esac - - # should we skip? - if [ "X$SKIP_NEXT" = "XYES" ]; then - SKIP_NEXT="NO" - record_outcome "SKIP" - SKIPS=$(( $SKIPS + 1 )) - return - fi - +# Analyze the commands that will be used in a test. +# +# Analyze and possibly instrument $PXY_CMD, $CLI_CMD, $SRV_CMD to pass +# extra arguments or go through wrappers. +# Set $DTLS (0=TLS, 1=DTLS). +analyze_test_commands() { # update DTLS variable detect_dtls "$SRV_CMD" @@ -696,48 +646,21 @@ run_test() { CLI_CMD="valgrind --leak-check=full $CLI_CMD" fi fi +} - TIMES_LEFT=2 - while [ $TIMES_LEFT -gt 0 ]; do - TIMES_LEFT=$(( $TIMES_LEFT - 1 )) - - # run the commands - if [ -n "$PXY_CMD" ]; then - printf "# %s\n%s\n" "$NAME" "$PXY_CMD" > $PXY_OUT - $PXY_CMD >> $PXY_OUT 2>&1 & - PXY_PID=$! - wait_proxy_start "$PXY_PORT" "$PXY_PID" - fi - - check_osrv_dtls - printf '# %s\n%s\n' "$NAME" "$SRV_CMD" > $SRV_OUT - provide_input | $SRV_CMD >> $SRV_OUT 2>&1 & - SRV_PID=$! - wait_server_start "$SRV_PORT" "$SRV_PID" - - printf '# %s\n%s\n' "$NAME" "$CLI_CMD" > $CLI_OUT - eval "$CLI_CMD" >> $CLI_OUT 2>&1 & - wait_client_done - - sleep 0.05 - - # terminate the server (and the proxy) - kill $SRV_PID - wait $SRV_PID - SRV_RET=$? - - if [ -n "$PXY_CMD" ]; then - kill $PXY_PID >/dev/null 2>&1 - wait $PXY_PID - fi - - # retry only on timeouts - if grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null; then - printf "RETRY " - else - TIMES_LEFT=0 - fi - done +# Check for failure conditions after a test case. +# +# Inputs from run_test: +# * positional parameters: test options (see run_test documentation) +# * $CLI_EXIT: client return code +# * $CLI_EXPECT: expected client return code +# * $SRV_RET: server return code +# * $CLI_OUT, $SRV_OUT, $PXY_OUT: files containing client/server/proxy logs +# +# Outputs: +# * $pass: set to 1 if no failures are detected, 0 otherwise +check_test_failure() { + pass=0 # check if the client and server went at least to the handshake stage # (useful to avoid tests with only negative assertions and non-zero @@ -861,6 +784,114 @@ run_test() { fi # if we're here, everything is ok + pass=1 +} + +# Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] +# Options: -s pattern pattern that must be present in server output +# -c pattern pattern that must be present in client output +# -u pattern lines after pattern must be unique in client output +# -f call shell function on client output +# -S pattern pattern that must be absent in server output +# -C pattern pattern that must be absent in client output +# -U pattern lines after pattern must be unique in server output +# -F call shell function on server output +run_test() { + NAME="$1" + shift 1 + + if is_excluded "$NAME"; then + SKIP_NEXT="NO" + return + fi + + print_name "$NAME" + + # Do we only run numbered tests? + if [ -n "$RUN_TEST_NUMBER" ]; then + case ",$RUN_TEST_NUMBER," in + *",$TESTS,"*) :;; + *) SKIP_NEXT="YES";; + esac + fi + + # does this test use a proxy? + if [ "X$1" = "X-p" ]; then + PXY_CMD="$2" + shift 2 + else + PXY_CMD="" + fi + + # get commands and client output + SRV_CMD="$1" + CLI_CMD="$2" + CLI_EXPECT="$3" + shift 3 + + # Check if test uses files + case "$SRV_CMD $CLI_CMD" in + *data_files/*) + requires_config_enabled MBEDTLS_FS_IO;; + esac + + # should we skip? + if [ "X$SKIP_NEXT" = "XYES" ]; then + SKIP_NEXT="NO" + record_outcome "SKIP" + SKIPS=$(( $SKIPS + 1 )) + return + fi + + analyze_test_commands "$@" + + TIMES_LEFT=2 + while [ $TIMES_LEFT -gt 0 ]; do + TIMES_LEFT=$(( $TIMES_LEFT - 1 )) + + # run the commands + if [ -n "$PXY_CMD" ]; then + printf "# %s\n%s\n" "$NAME" "$PXY_CMD" > $PXY_OUT + $PXY_CMD >> $PXY_OUT 2>&1 & + PXY_PID=$! + wait_proxy_start "$PXY_PORT" "$PXY_PID" + fi + + check_osrv_dtls + printf '# %s\n%s\n' "$NAME" "$SRV_CMD" > $SRV_OUT + provide_input | $SRV_CMD >> $SRV_OUT 2>&1 & + SRV_PID=$! + wait_server_start "$SRV_PORT" "$SRV_PID" + + printf '# %s\n%s\n' "$NAME" "$CLI_CMD" > $CLI_OUT + eval "$CLI_CMD" >> $CLI_OUT 2>&1 & + wait_client_done + + sleep 0.05 + + # terminate the server (and the proxy) + kill $SRV_PID + wait $SRV_PID + SRV_RET=$? + + if [ -n "$PXY_CMD" ]; then + kill $PXY_PID >/dev/null 2>&1 + wait $PXY_PID + fi + + # retry only on timeouts + if grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null; then + printf "RETRY " + else + TIMES_LEFT=0 + fi + done + + check_test_failure "$@" + if [ "$pass" -eq 0 ]; then + return + fi + record_outcome "PASS" if [ "$PRESERVE_LOGS" -gt 0 ]; then mv $SRV_OUT o-srv-${TESTS}.log From ad58e92eac0337468173f70f465e378a2a94915e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Oct 2021 16:35:35 +0200 Subject: [PATCH 4/7] Move the core loop of run_test into an auxiliary function No behavior change. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 73 +++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 184666cf1b..b86e194d1a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -787,6 +787,49 @@ check_test_failure() { pass=1 } +# Run the current test case: start the server and if applicable the proxy, run +# the client, wait for all processes to finish or time out. +# +# Inputs: +# * $NAME: test case name +# * $CLI_CMD, $SRV_CMD, $PXY_CMD: commands to run +# * $CLI_OUT, $SRV_OUT, $PXY_OUT: files to contain client/server/proxy logs +# +# Outputs: +# * $CLI_EXIT: client return code +# * $SRV_RET: server return code +do_run_test_once() { + # run the commands + if [ -n "$PXY_CMD" ]; then + printf "# %s\n%s\n" "$NAME" "$PXY_CMD" > $PXY_OUT + $PXY_CMD >> $PXY_OUT 2>&1 & + PXY_PID=$! + wait_proxy_start "$PXY_PORT" "$PXY_PID" + fi + + check_osrv_dtls + printf '# %s\n%s\n' "$NAME" "$SRV_CMD" > $SRV_OUT + provide_input | $SRV_CMD >> $SRV_OUT 2>&1 & + SRV_PID=$! + wait_server_start "$SRV_PORT" "$SRV_PID" + + printf '# %s\n%s\n' "$NAME" "$CLI_CMD" > $CLI_OUT + eval "$CLI_CMD" >> $CLI_OUT 2>&1 & + wait_client_done + + sleep 0.05 + + # terminate the server (and the proxy) + kill $SRV_PID + wait $SRV_PID + SRV_RET=$? + + if [ -n "$PXY_CMD" ]; then + kill $PXY_PID >/dev/null 2>&1 + wait $PXY_PID + fi +} + # Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] # Options: -s pattern pattern that must be present in server output # -c pattern pattern that must be present in client output @@ -849,35 +892,7 @@ run_test() { while [ $TIMES_LEFT -gt 0 ]; do TIMES_LEFT=$(( $TIMES_LEFT - 1 )) - # run the commands - if [ -n "$PXY_CMD" ]; then - printf "# %s\n%s\n" "$NAME" "$PXY_CMD" > $PXY_OUT - $PXY_CMD >> $PXY_OUT 2>&1 & - PXY_PID=$! - wait_proxy_start "$PXY_PORT" "$PXY_PID" - fi - - check_osrv_dtls - printf '# %s\n%s\n' "$NAME" "$SRV_CMD" > $SRV_OUT - provide_input | $SRV_CMD >> $SRV_OUT 2>&1 & - SRV_PID=$! - wait_server_start "$SRV_PORT" "$SRV_PID" - - printf '# %s\n%s\n' "$NAME" "$CLI_CMD" > $CLI_OUT - eval "$CLI_CMD" >> $CLI_OUT 2>&1 & - wait_client_done - - sleep 0.05 - - # terminate the server (and the proxy) - kill $SRV_PID - wait $SRV_PID - SRV_RET=$? - - if [ -n "$PXY_CMD" ]; then - kill $PXY_PID >/dev/null 2>&1 - wait $PXY_PID - fi + do_run_test_once # retry only on timeouts if grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null; then From e31a9ea6011a38c4d85114d7fb381100829917c0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Oct 2021 17:23:25 +0200 Subject: [PATCH 5/7] Move retry logic into check_test_failure This will allow having other retry conditions, in particular based on run_test options. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b86e194d1a..2cfe74ea5b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -656,11 +656,19 @@ analyze_test_commands() { # * $CLI_EXPECT: expected client return code # * $SRV_RET: server return code # * $CLI_OUT, $SRV_OUT, $PXY_OUT: files containing client/server/proxy logs +# * $TIMES_LEFT: if nonzero, a RETRY outcome is allowed # # Outputs: -# * $pass: set to 1 if no failures are detected, 0 otherwise +# * $outcome: one of PASS/RETRY/FAIL check_test_failure() { - pass=0 + outcome=FAIL + + if [ $TIMES_LEFT -gt 0 ] && + grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null + then + outcome=RETRY + return + fi # check if the client and server went at least to the handshake stage # (useful to avoid tests with only negative assertions and non-zero @@ -784,7 +792,7 @@ check_test_failure() { fi # if we're here, everything is ok - pass=1 + outcome=PASS } # Run the current test case: start the server and if applicable the proxy, run @@ -894,19 +902,15 @@ run_test() { do_run_test_once - # retry only on timeouts - if grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null; then - printf "RETRY " - else - TIMES_LEFT=0 - fi + check_test_failure "$@" + case $outcome in + PASS) break;; + RETRY) printf "RETRY ";; + FAIL) return;; + esac done - check_test_failure "$@" - if [ "$pass" -eq 0 ]; then - return - fi - + # If we get this far, the test case passed. record_outcome "PASS" if [ "$PRESERVE_LOGS" -gt 0 ]; then mv $SRV_OUT o-srv-${TESTS}.log From 838902547cd142d631802a4bec6d61b83b4a8a99 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Oct 2021 18:00:10 +0200 Subject: [PATCH 6/7] Retry if a test case fails because of an unexpected resend Palliative for https://github.com/ARMmbed/mbedtls/issues/3377. If a test case fails due to an unexpected resend, allow retrying, like in the case of a client timeout. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 2cfe74ea5b..1d698279ce 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -659,14 +659,14 @@ analyze_test_commands() { # * $TIMES_LEFT: if nonzero, a RETRY outcome is allowed # # Outputs: -# * $outcome: one of PASS/RETRY/FAIL +# * $outcome: one of PASS/RETRY*/FAIL check_test_failure() { outcome=FAIL if [ $TIMES_LEFT -gt 0 ] && grep '===CLIENT_TIMEOUT===' $CLI_OUT >/dev/null then - outcome=RETRY + outcome="RETRY(client-timeout)" return fi @@ -727,14 +727,22 @@ check_test_failure() { "-S") if grep -v '^==' $SRV_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then - fail "pattern '$2' MUST NOT be present in the Server output" + if [ "$2" = "resend" ] && [ $TIMES_LEFT -gt 0 ]; then + outcome="RETRY(resend)" + else + fail "pattern '$2' MUST NOT be present in the Server output" + fi return fi ;; "-C") if grep -v '^==' $CLI_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then - fail "pattern '$2' MUST NOT be present in the Client output" + if [ "$2" = "resend" ] && [ $TIMES_LEFT -gt 0 ]; then + outcome="RETRY(resend)" + else + fail "pattern '$2' MUST NOT be present in the Client output" + fi return fi ;; @@ -905,7 +913,7 @@ run_test() { check_test_failure "$@" case $outcome in PASS) break;; - RETRY) printf "RETRY ";; + RETRY*) printf "$outcome ";; FAIL) return;; esac done From 37125014654c70af8a546e83fb71011b335caf5f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 20 Oct 2021 14:17:02 +0200 Subject: [PATCH 7/7] Move is-it-resend logic into a function Improve the code structure in case we want to add other similar conditions later. Document better what we're doing, and document why we're doing it. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1d698279ce..fe79a75993 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -444,6 +444,32 @@ record_outcome() { echo "$1" } +# True if the presence of the given pattern in a log definitely indicates +# that the test has failed. False if the presence is inconclusive. +# +# Inputs: +# * $1: pattern found in the logs +# * $TIMES_LEFT: >0 if retrying is an option +# +# Outputs: +# * $outcome: set to a retry reason if the pattern is inconclusive, +# unchanged otherwise. +# * Return value: 1 if the pattern is inconclusive, +# 0 if the failure is definitive. +log_pattern_presence_is_conclusive() { + # If we've run out of attempts, then don't retry no matter what. + if [ $TIMES_LEFT -eq 0 ]; then + return 0 + fi + case $1 in + "resend") + # An undesired resend may have been caused by the OS dropping or + # delaying a packet at an inopportune time. + outcome="RETRY(resend)" + return 1;; + esac +} + # fail fail() { record_outcome "FAIL" "$1" @@ -727,9 +753,7 @@ check_test_failure() { "-S") if grep -v '^==' $SRV_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then - if [ "$2" = "resend" ] && [ $TIMES_LEFT -gt 0 ]; then - outcome="RETRY(resend)" - else + if log_pattern_presence_is_conclusive "$2"; then fail "pattern '$2' MUST NOT be present in the Server output" fi return @@ -738,9 +762,7 @@ check_test_failure() { "-C") if grep -v '^==' $CLI_OUT | grep -v 'Serious error when reading debug info' | grep "$2" >/dev/null; then - if [ "$2" = "resend" ] && [ $TIMES_LEFT -gt 0 ]; then - outcome="RETRY(resend)" - else + if log_pattern_presence_is_conclusive "$2"; then fail "pattern '$2' MUST NOT be present in the Client output" fi return