From e0ce496a4553195946dd7977052a10a7762ce57f Mon Sep 17 00:00:00 2001
From: David Cooper <david.cooper@nist.gov>
Date: Fri, 15 Feb 2019 14:28:56 -0500
Subject: [PATCH] Fix check_tls12_pref()

This is a minor bug when performing run_server_preference() if the server cannot handle ClientHello messages with more than 128 ciphers (i.e., $SERVER_SIZE_LIMIT_BUG is true) and the server supports at least one cipher in 'CAMELLIA:IDEA:KRB5:PSK:SRP:aNULL:eNULL'.

The problem is that `$OPENSSL s_client` is called with a cipher list such as
   ECDHE-RSA-AES256-GCM-SHA384:CAMELLIA256-SHA:AES256-SHA256
then
   ECDHE-RSA-AES256-GCM-SHA384:CAMELLIA256-SHA:AES256-SHA256:-CAMELLIA256-SHA
then
   ECDHE-RSA-AES256-GCM-SHA384:CAMELLIA256-SHA:AES256-SHA256:-CAMELLIA256-SHA:-AES256-SHA256
and finally
   ECDHE-RSA-AES256-GCM-SHA384:CAMELLIA256-SHA:AES256-SHA256:-CAMELLIA256-SHA:-AES256-SHA256:-ECDHE-RSA-AES256-GCM-SHA384

The last call to $OPENSSL s_client produces an error since the list of ciphers to send is empty, and this results in connectivity_problem() being called to print a "openssl s_client connect problem" warning.

This PR fixes the problem by constructing a list of ciphers to test for and by not calling $OPENSSL s_client if the list is empty.
---
 testssl.sh | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/testssl.sh b/testssl.sh
index adfb37c..0dfbaa5 100755
--- a/testssl.sh
+++ b/testssl.sh
@@ -6270,7 +6270,7 @@ run_server_preference() {
 check_tls12_pref() {
      local batchremoved="-CAMELLIA:-IDEA:-KRB5:-PSK:-SRP:-aNULL:-eNULL"
      local batchremoved_success=false
-     local tested_cipher=""
+     local tested_cipher="" cipher ciphers_to_test
      local order=""
      local -i nr_ciphers_found_r1=0 nr_ciphers_found_r2=0
 
@@ -6309,11 +6309,15 @@ check_tls12_pref() {
 
      if "$batchremoved_success"; then
           # now we combine the two cipher sets from both while loops
-          [[ "${order:0:1}" == " " ]] && order="${order:1}"
-          combined_ciphers="${order// /:}"
+          combined_ciphers="$order"
           order="" ; tested_cipher=""
           while true; do
-               $OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "$combined_ciphers$tested_cipher" -connect $NODEIP:$PORT $PROXY $SNI") </dev/null 2>>$ERRFILE >$TMPFILE
+               ciphers_to_test=""
+               for cipher in $combined_ciphers; do
+                    [[ ! "$tested_cipher:" =~ :-$cipher: ]] && ciphers_to_test+=":$cipher"
+               done
+               [[ -z "$ciphers_to_test" ]] && break
+               $OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "${ciphers_to_test:1}" -connect $NODEIP:$PORT $PROXY $SNI") </dev/null 2>>$ERRFILE >$TMPFILE
                if sclient_connect_successful $? $TMPFILE ; then
                     cipher=$(get_cipher $TMPFILE)
                     order+=" $cipher"
@@ -6321,7 +6325,7 @@ check_tls12_pref() {
                     nr_ciphers_found_r2+=1
                     "$FAST" && break
                else
-                    # nothing left, we're done
+                    # This shouldn't happen.
                     break
                fi
           done