From 2bdbdec5d9254b2ddc28c0d7ff673ce7d836ea95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Fri, 1 Mar 2024 17:32:34 +0100 Subject: [PATCH 1/8] Do not wait on pid you are not a parent. The zombi fix did too much modifications breaking the global time-out function. As the wait $pid failed, we no longer create the watchdog file. Fix by reverting unnecessary changes. --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 03a616a..398eb7b 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17112,7 +17112,7 @@ run_renego() { for ((i=0; i < ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & pid=$! - ( sleep $(($ssl_reneg_attempts*3)) && pkill -HUP -P $pid && wait $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null & + ( sleep $(($ssl_reneg_attempts*3)) && kill $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null & watcher=$! # Trick to get the return value of the openssl command, output redirection and a timeout. Yes, some target hang/block after some tries. wait $pid && pkill -HUP -P $watcher From 8627ba518fd87d1e80849650ad92228a2dbc4f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Fri, 1 Mar 2024 21:17:56 +0100 Subject: [PATCH 2/8] Kill the heuristic an count the real number of renegociations The heuristic is too fragile and timing dependant. - As for the initial TLS negociation, wait for the result of the renegociation request before sending the next one. - Remove the result ratio calculation and message as we now reach the timeout in case of exponential backoff or connection hang. This commit depend on the fix of the timeout, broken by the zombi fix. --- testssl.sh | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/testssl.sh b/testssl.sh index 398eb7b..1c27f09 100755 --- a/testssl.sh +++ b/testssl.sh @@ -16981,6 +16981,8 @@ run_renego() { local jsonID="" local ssl_reneg_attempts=$SSL_RENEG_ATTEMPTS local ssl_reneg_wait=$SSL_RENEG_WAIT + local pid watcher + local tmp_result loop_reneg # In cases where there's no default host configured we need SNI here as openssl then would return otherwise an error and the test will fail "$HAS_TLS13" && [[ -z "$proto" ]] && proto="-no_tls1_3" @@ -17108,26 +17110,28 @@ run_renego() { # If we dont wait for the session to be established on slow server, we will try to re-negotiate # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). - (while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]]; do sleep 1; done; \ - for ((i=0; i < ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; done) | \ + (while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]]; do sleep $ssl_reneg_wait; done; \ + for ((i=0; i < ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; \ + while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ ! -f $TEMPDIR/was_killed ]]; \ + do sleep $ssl_reneg_wait; done; \ + done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & pid=$! ( sleep $(($ssl_reneg_attempts*3)) && kill $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null & watcher=$! # Trick to get the return value of the openssl command, output redirection and a timeout. Yes, some target hang/block after some tries. - wait $pid && pkill -HUP -P $watcher + wait $pid tmp_result=$? + pkill -HUP -P $watcher wait $watcher # If we are here, we have done two successful renegotiation (-2) and do the loop loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE )-2)) if [[ -f $TEMPDIR/was_killed ]]; then - tmp_result=3 + tmp_result=2 + sleep $ssl_reneg_wait # let the while loop see the watchdog file + sleep $ssl_reneg_wait # and + sleep $ssl_reneg_wait # avoid float arithmetic rm -f $TEMPDIR/was_killed - else - # If we got less than 2/3 successful attempts during the loop with 1s pause, we are in presence of exponential backoff. - if [[ $tmp_result -eq 0 ]] && [[ $loop_reneg -le $(($ssl_reneg_attempts*2/3)) ]]; then - tmp_result=2 - fi fi case $tmp_result in 0) pr_svrty_high "VULNERABLE (NOT ok)"; outln ", DoS threat ($ssl_reneg_attempts attempts)" @@ -17137,10 +17141,6 @@ run_renego() { fileout "$jsonID" "OK" "not vulnerable, mitigated" "$cve" "$cwe" ;; 2) pr_svrty_good "not vulnerable (OK)"; \ - outln " -- mitigated ($loop_reneg successful reneg within ${ssl_reneg_attempts} in ${ssl_reneg_attempts}x${ssl_reneg_wait}s)" - fileout "$jsonID" "OK" "not vulnerable, mitigated" "$cve" "$cwe" - ;; - 3) pr_svrty_good "not vulnerable (OK)"; \ outln " -- mitigated ($loop_reneg successful reneg within ${ssl_reneg_attempts} in $((${ssl_reneg_attempts}*3))s(timeout))" fileout "$jsonID" "OK" "not vulnerable, mitigated" "$cve" "$cwe" ;; From 81167dc90887f4dc1e17336e7a056ea5e175e5ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Mon, 4 Mar 2024 18:48:21 +0100 Subject: [PATCH 3/8] Fixes: - Add safety gards againts infinite sleep loop - correct the for loop test - reverse the watchdog file logic for sleep loop. No timing dependance. --- testssl.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/testssl.sh b/testssl.sh index 1c27f09..7bced0d 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17107,13 +17107,14 @@ run_renego() { else # Clear the log to not get the content of previous run before the execution of the new one. echo -n > $TMPFILE + touch $TEMPDIR/not_killed # If we dont wait for the session to be established on slow server, we will try to re-negotiate # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). - (while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]]; do sleep $ssl_reneg_wait; done; \ - for ((i=0; i < ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; \ - while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ ! -f $TEMPDIR/was_killed ]]; \ - do sleep $ssl_reneg_wait; done; \ + (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$(($j+1)); done; \ + for ((i=0; i < $ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; j=0; \ + while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/not_killed ]] && [[ $k -lt 180 ]]; \ + do sleep $ssl_reneg_wait; k=$(($k+1)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & pid=$! @@ -17128,10 +17129,7 @@ run_renego() { loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE )-2)) if [[ -f $TEMPDIR/was_killed ]]; then tmp_result=2 - sleep $ssl_reneg_wait # let the while loop see the watchdog file - sleep $ssl_reneg_wait # and - sleep $ssl_reneg_wait # avoid float arithmetic - rm -f $TEMPDIR/was_killed + rm -f $TEMPDIR/not_killed fi case $tmp_result in 0) pr_svrty_high "VULNERABLE (NOT ok)"; outln ", DoS threat ($ssl_reneg_attempts attempts)" From 35496e5c5f97b979ce59725c7411e5df772fa38c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Mon, 4 Mar 2024 19:16:48 +0100 Subject: [PATCH 4/8] Clean up watchdog file logic --- testssl.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index 7bced0d..ae7d5b3 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17107,13 +17107,14 @@ run_renego() { else # Clear the log to not get the content of previous run before the execution of the new one. echo -n > $TMPFILE - touch $TEMPDIR/not_killed + #RENEGOTIATING wait loop watchdog file + touch $TEMPDIR/allowed_to_loop # If we dont wait for the session to be established on slow server, we will try to re-negotiate # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$(($j+1)); done; \ for ((i=0; i < $ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; j=0; \ - while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/not_killed ]] && [[ $k -lt 180 ]]; \ + while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] && [[ $k -lt 180 ]]; \ do sleep $ssl_reneg_wait; k=$(($k+1)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & @@ -17125,11 +17126,12 @@ run_renego() { tmp_result=$? pkill -HUP -P $watcher wait $watcher + rm -f $TEMPDIR/allowed_to_loop # If we are here, we have done two successful renegotiation (-2) and do the loop loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE )-2)) if [[ -f $TEMPDIR/was_killed ]]; then tmp_result=2 - rm -f $TEMPDIR/not_killed + rm -f $TEMPDIR/was_killed fi case $tmp_result in 0) pr_svrty_high "VULNERABLE (NOT ok)"; outln ", DoS threat ($ssl_reneg_attempts attempts)" From 91367caa71fc961cd6d1ab4be16f239980aecc90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Wed, 6 Mar 2024 13:57:21 +0100 Subject: [PATCH 5/8] Fix and optimisation There is a race condition if openssl exit during a renego but after the RENEGOTIATING printing. In this case we could issue a R before the process exit and be blocked in the waiting loop. With the safety guards in place (loop count + timeout) this is harmless but not optimal. Fix this by: - reordering the sleep vs echo to let the process exit and catch the pipe error more frequently. - exit the while loop if RENEGOTIATING is not the last log line. We will catch the pipe error on the next for loop echo. - correct the k variable initialisation - correct the for (( ; ; )) variable $ convention usage - reduce the while loop count limit to 120 to align with the global timeout --- testssl.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index ae7d5b3..82e1bfe 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17113,8 +17113,9 @@ run_renego() { # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$(($j+1)); done; \ - for ((i=0; i < $ssl_reneg_attempts; i++ )); do echo R; sleep $ssl_reneg_wait; j=0; \ - while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] && [[ $k -lt 180 ]]; \ + for ((i=0; i < $ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \ + while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \ + && [[ $(tail -n1 $ERRFILE |grep -ac '^RENEGOTIATING') -eq 1 ]] && [[ $k -lt 120 ]]; \ do sleep $ssl_reneg_wait; k=$(($k+1)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & From 43e55617bb7104ab8ff5c1233edb88d48207904d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Wed, 6 Mar 2024 14:53:34 +0100 Subject: [PATCH 6/8] errorlog filtering fix Filter out verify and deph lines to not reintrodure timing race condition. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index 82e1bfe..9615dd8 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17113,9 +17113,9 @@ run_renego() { # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$(($j+1)); done; \ - for ((i=0; i < $ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \ + for ((i=0; i < ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \ while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \ - && [[ $(tail -n1 $ERRFILE |grep -ac '^RENEGOTIATING') -eq 1 ]] && [[ $k -lt 120 ]]; \ + && [[ $(tail -n1 $ERRFILE |grep -acE '^(RENEGOTIATING|depth|verify)') -eq 1 ]] && [[ $k -lt 120 ]]; \ do sleep $ssl_reneg_wait; k=$(($k+1)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & From 2824e347b498a2e0167204831899a32b2673eabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Wed, 6 Mar 2024 15:44:34 +0100 Subject: [PATCH 7/8] Cleanup bash $(( )) arithmetic usage --- testssl.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/testssl.sh b/testssl.sh index 9615dd8..54ba153 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17112,15 +17112,15 @@ run_renego() { # If we dont wait for the session to be established on slow server, we will try to re-negotiate # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). - (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$(($j+1)); done; \ + (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$((j++)); done; \ for ((i=0; i < ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \ - while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $(($i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \ + while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $((i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \ && [[ $(tail -n1 $ERRFILE |grep -acE '^(RENEGOTIATING|depth|verify)') -eq 1 ]] && [[ $k -lt 120 ]]; \ - do sleep $ssl_reneg_wait; k=$(($k+1)); done; \ + do sleep $ssl_reneg_wait; k=$((k++)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & pid=$! - ( sleep $(($ssl_reneg_attempts*3)) && kill $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null & + ( sleep $((ssl_reneg_attempts*3)) && kill $pid && touch $TEMPDIR/was_killed ) >&2 2>/dev/null & watcher=$! # Trick to get the return value of the openssl command, output redirection and a timeout. Yes, some target hang/block after some tries. wait $pid @@ -17129,7 +17129,7 @@ run_renego() { wait $watcher rm -f $TEMPDIR/allowed_to_loop # If we are here, we have done two successful renegotiation (-2) and do the loop - loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE )-2)) + loop_reneg=$(($(grep -ac '^RENEGOTIATING' $ERRFILE)-2)) if [[ -f $TEMPDIR/was_killed ]]; then tmp_result=2 rm -f $TEMPDIR/was_killed From 426bfa6cd54dd7024e5db2ba352b69b773e22d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Fust=C3=A9?= Date: Wed, 6 Mar 2024 16:02:19 +0100 Subject: [PATCH 8/8] Fix the cleanup ... --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index 54ba153..b598e78 100755 --- a/testssl.sh +++ b/testssl.sh @@ -17112,11 +17112,11 @@ run_renego() { # If we dont wait for the session to be established on slow server, we will try to re-negotiate # too early losing all the attempts before the session establishment as OpenSSL will not buffer them # (only the first will be till the establishement of the session). - (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; j=$((j++)); done; \ + (j=0; while [[ $(grep -ac '^SSL-Session:' $TMPFILE) -ne 1 ]] && [[ $j -lt 30 ]]; do sleep $ssl_reneg_wait; ((j++)); done; \ for ((i=0; i < ssl_reneg_attempts; i++ )); do sleep $ssl_reneg_wait; echo R; k=0; \ while [[ $(grep -ac '^RENEGOTIATING' $ERRFILE) -ne $((i+3)) ]] && [[ -f $TEMPDIR/allowed_to_loop ]] \ && [[ $(tail -n1 $ERRFILE |grep -acE '^(RENEGOTIATING|depth|verify)') -eq 1 ]] && [[ $k -lt 120 ]]; \ - do sleep $ssl_reneg_wait; k=$((k++)); done; \ + do sleep $ssl_reneg_wait; ((k++)); done; \ done) | \ $OPENSSL s_client $(s_client_options "$proto $legacycmd $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>>$ERRFILE & pid=$!