From 76fb81112b874a5829841b3e3820d4d2fad3be0b Mon Sep 17 00:00:00 2001
From: David Cooper <david.cooper@nist.gov>
Date: Fri, 20 Sep 2019 17:37:11 -0400
Subject: [PATCH] Fix run_protocols()

This PR fixes a problem in run_protocols() that was introduced by https://github.com/drwetter/testssl.sh/commit/7ec3c6ab997b012b1518903137d84043cf9825a8.

https://github.com/drwetter/testssl.sh/commit/7ec3c6ab997b012b1518903137d84043cf9825a8 changes run_protocols() to perform the initial testing for TLSv1.3 support before testing for TLSv1.2 support. The problem with this is that the code for testing TLSv1.3 makes use of the results of the TLSv1.2 testing.

In the current code, Line 5183 looks at the value of $subret to determine whether the TLSv1.2 ClientHello resulted in a successful connection. However, $subet has not yet been set (it has just been initialized to 0 at the beginning of the function). Since $subret will always be 0, the code will try to extract a cipher from $TEMPDIR/$NODEIP.parse_tls_serverhello.txt. This may work, since $TEMPDIR/$NODEIP.parse_tls_serverhello.txt may have been populated by a prior function call, but this is not how the code was intended to work.

This PR fixes the problem by doing the TLSv1.2 testing before the TLSv1.3 testing is done. It still waits until both have been tested, however, before outputting the results, so that the output for TLSv1.2 can be modified depending on whether TLSv1.3 is supported.
---
 testssl.sh | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/testssl.sh b/testssl.sh
index d0cc3ad..144b90e 100755
--- a/testssl.sh
+++ b/testssl.sh
@@ -4866,7 +4866,7 @@ run_protocols() {
      local lines nr_ciphers_detected
      local tls13_ciphers_to_test=""
      local i drafts_offered=""  drafts_offered_str="" supported_versions debug_recomm=""
-     local -i ret=0 subret=0 ret_val_tls13=0
+     local -i ret=0 ret_val_tls12=0 ret_val_tls13=0
      local offers_tls13=false
      local jsonID="SSLv2"
 
@@ -5171,16 +5171,23 @@ run_protocols() {
                ;;
      esac
 
-     # Now, we are doing a basic/pre test for TLS 1.3 in order not to penalize servers (medium)
+     # Now, we are doing a basic/pre test for TLS 1.2 and 1.3 in order not to penalize servers (medium)
      # running TLS 1.3 only when TLS 1.2 is not offered.  0 and 5 are the return codes for
      # TLS 1.3 support (kind of, including deprecated pre-versions of TLS 1.3)
      if "$using_sockets"; then
+          tls_sockets "03" "$TLS12_CIPHER"
+          ret_val_tls12=$?
+          if [[ $ret_val_tls12 -ne 0 ]]; then
+               tls_sockets "03" "$TLS12_CIPHER_2ND_TRY"
+               [[ $? -eq 0 ]] && ret_val_tls12=0
+               # see #807 and #806
+          fi
           # Need to ensure that at most 128 ciphers are included in ClientHello.
           # If the TLSv1.2 test was successful, then use the 5 TLSv1.3 ciphers
           # plus the cipher selected in the TLSv1.2 test. If the TLSv1.2 test was
           # not successful, then just use the 5 TLSv1.3 ciphers plus the list of
           # ciphers used in all of the previous tests ($TLS_CIPHER).
-          if [[ $subret -eq 0 ]] || [[ $subret -eq 2 ]]; then
+          if [[ $ret_val_tls12 -eq 0 ]] || [[ $ret_val_tls12 -eq 2 ]]; then
                tls13_ciphers_to_test="$(get_cipher "$TEMPDIR/$NODEIP.parse_tls_serverhello.txt")"
                if [[ "$tls13_ciphers_to_test" == TLS_* ]] || [[ "$tls13_ciphers_to_test" == SSL_* ]]; then
                     tls13_ciphers_to_test="$(rfc2hexcode "$tls13_ciphers_to_test")"
@@ -5195,30 +5202,19 @@ run_protocols() {
           fi
           tls_sockets "04" "$tls13_ciphers_to_test"
      else
+          run_prototest_openssl "-tls1_2"
+          ret_val_tls12=$?
           run_prototest_openssl "-tls1_3"
      fi
      ret_val_tls13=$?
      if [[ $ret_val_tls13 -eq 0 ]] || [[ $ret_val_tls13 -eq 5 ]]; then
           offers_tls13=true             # This variable comes in handy for further if statements below
      fi
-     # Done with pretesting TLS 1.3. Normally we should/could reverse the order for the protocols -- or
-     # keep the order and mute the output, until we can make a final verdict
+     # Done with pretesting TLS 1.2 and 1.3.
 
      pr_bold " TLS 1.2    ";
      jsonID="TLS1_2"
-     if "$using_sockets"; then
-          tls_sockets "03" "$TLS12_CIPHER"
-          subret=$?
-          if [[ $subret -ne 0 ]]; then
-               tls_sockets "03" "$TLS12_CIPHER_2ND_TRY"
-               [[ $? -eq 0 ]] && subret=0
-               # see #807 and #806
-          fi
-     else
-          run_prototest_openssl "-tls1_2"
-          subret=$?
-     fi
-     case $subret in
+     case $ret_val_tls12 in
           0)   prln_svrty_best "offered (OK)"
                fileout "$jsonID" "OK" "offered"
                latest_supported="0303"