From 4536e933f901b4be7aa1c4b9b310e0af92836a0a Mon Sep 17 00:00:00 2001 From: David Cooper Date: Mon, 25 Oct 2021 14:28:51 -0400 Subject: [PATCH] Improve cipher_pref_check() Some servers are configured to prioritize ChaCha ciphers if those ciphers are preferred by the client, even if the server is generally configured to use the server's cipher preferences rather than the client's. As a result of this, if a ChaCha cipher appears in the ClientHello before a non-ChaCha cipher, the server may select the ChaCha cipher even if the server is configured to prefer the non-ChaCha cipher. In a few cases, e.g., cloudflare.com for TLS 1.2, this affects the ordering of the ciphers presented by cipher_pref_check(). This commit fixes the problem by having cipher_pref_check() (and check_tls12_pref()) always place any ChaCha ciphers at the end of the cipher list in the ClientHello. This ensures that cipher_pref_check() presents the ciphers in the server's preference order. --- testssl.sh | 88 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/testssl.sh b/testssl.sh index 94eb444..4dbc23e 100755 --- a/testssl.sh +++ b/testssl.sh @@ -6841,14 +6841,24 @@ run_server_preference() { # arg1: true if the list that is returned does not need to be ordered by preference. check_tls12_pref() { local unordered_list_ok="$1" + local chacha20_ciphers="" non_chacha20_ciphers="" local batchremoved="-CAMELLIA:-IDEA:-KRB5:-PSK:-SRP:-aNULL:-eNULL" local batchremoved_success=false local tested_cipher="" cipher ciphers_to_test local order="" local -i nr_ciphers_found_r1=0 nr_ciphers_found_r2=0 + # Place ChaCha20 ciphers at the end of the list to avoid accidentally + # triggering the server's PrioritizeChaCha setting. + ciphers_to_test="$(actually_supported_osslciphers "ALL:$batchremoved" "" "")" + for cipher in $(colon_to_spaces "$ciphers_to_test"); do + [[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher:" || non_chacha20_ciphers+="$cipher:" + done + ciphers_to_test="$non_chacha20_ciphers$chacha20_ciphers" + ciphers_to_test="${ciphers_to_test%:}" + while true; do - $OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "ALL$tested_cipher:$batchremoved" -connect $NODEIP:$PORT $PROXY $SNI") >$ERRFILE >$TMPFILE + $OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "$ciphers_to_test:$tested_cipher" -connect $NODEIP:$PORT $PROXY $SNI") >$ERRFILE >$TMPFILE if sclient_connect_successful $? $TMPFILE ; then cipher=$(get_cipher $TMPFILE) order+=" $cipher" @@ -6882,7 +6892,14 @@ check_tls12_pref() { if "$batchremoved_success" && ! "$unordered_list_ok"; then # now we combine the two cipher sets from both while loops - combined_ciphers="$order" + + # Place ChaCha20 ciphers at the end of the list to avoid accidentally + # triggering the server's PrioritizeChaCha setting. + chacha20_ciphers=""; non_chacha20_ciphers="" + for cipher in $order; do + [[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher " || non_chacha20_ciphers+="$cipher " + done + combined_ciphers="$non_chacha20_ciphers$chacha20_ciphers" order="" ; tested_cipher="" while true; do ciphers_to_test="" @@ -6921,16 +6938,15 @@ cipher_pref_check() { local proto="$1" proto_hex="$2" proto_text="$3" local using_sockets="$4" local wide="$5" # at the moment always = true - local tested_cipher cipher order rfc_cipher rfc_order + local tested_cipher cipher order="" rfc_cipher rfc_order local -i i nr_ciphers nr_nonossl_ciphers num_bundles bundle_size bundle end_of_bundle success local -i nr_ciphers_found - local hexc ciphers_to_test + local hexc ciphers_to_test cipher_list chacha20_ciphers non_chacha20_ciphers local -a normalized_hexcode ciph kx enc export2 sigalg local -a rfc_ciph hexcode ciphers_found="" ciphers_found2 local -a -i index - local ciphers_found_with_sockets + local ciphers_found_with_sockets=false - order=""; ciphers_found_with_sockets=false if [[ $proto == ssl3 ]] && ! "$HAS_SSL3" && ! "$using_sockets"; then prln_local_problem "$OPENSSL doesn't support \"s_client -ssl3\""; return 0 @@ -6947,25 +6963,31 @@ cipher_pref_check() { ciphers_found="$order" fi if "$wide" || [[ -z "$order" ]]; then + # Place ChaCha20 ciphers at the end of the list to avoid accidentally + # triggering the server's PrioritizeChaCha setting. + cipher_list=""; chacha20_ciphers=""; non_chacha20_ciphers="" + if [[ $proto == tls1_3 ]]; then + cipher_list="$(colon_to_spaces "$TLS13_OSSL_CIPHERS")" + elif [[ -n "$ciphers_found" ]]; then + cipher_list="$ciphers_found" + else + cipher_list="$(colon_to_spaces "$(actually_supported_osslciphers "ALL:COMPLEMENTOFALL" "" "")")" + fi + for cipher in $cipher_list; do + [[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher " || non_chacha20_ciphers+="$cipher " + done + cipher_list="$non_chacha20_ciphers $chacha20_ciphers" + tested_cipher=""; order=""; nr_ciphers_found=0 while true; do + ciphers_to_test="" + for cipher in $cipher_list; do + [[ ! "$tested_cipher:" =~ :-$cipher: ]] && ciphers_to_test+=":$cipher" + done + [[ -z "$ciphers_to_test" ]] && break if [[ $proto != tls1_3 ]]; then - if [[ -n "$ciphers_found" ]]; then - ciphers_to_test="" - for cipher in $ciphers_found; do - [[ ! "$tested_cipher:" =~ :-$cipher: ]] && ciphers_to_test+=":$cipher" - done - [[ -z "$ciphers_to_test" ]] && break - ciphers_to_test="-cipher ${ciphers_to_test:1}" - else - ciphers_to_test="-cipher ALL:COMPLEMENTOFALL${tested_cipher}" - fi + ciphers_to_test="-cipher ${ciphers_to_test:1}" else - ciphers_to_test="" - for cipher in $(colon_to_spaces "$TLS13_OSSL_CIPHERS"); do - [[ ! "$tested_cipher:" =~ :-$cipher: ]] && ciphers_to_test+=":$cipher" - done - [[ -z "$ciphers_to_test" ]] && break ciphers_to_test="-ciphersuites ${ciphers_to_test:1}" fi $OPENSSL s_client $(s_client_options "$STARTTLS -"$proto" $BUGS $ciphers_to_test -connect $NODEIP:$PORT $PROXY $SNI") >$ERRFILE >$TMPFILE @@ -7074,8 +7096,32 @@ cipher_pref_check() { # If there is a SERVER_SIZE_LIMIT_BUG, then use sockets to find the cipher # order, but starting with the list of ciphers supported by the server. if "$ciphers_found_with_sockets"; then + # Create an array of the ciphers to test with any ChaCha20 + # listed last in order to avoid accidentally triggering the + # server's PriorizeChaCha setting. order=""; nr_ciphers=0; nr_ciphers_found=0 for (( i=0; i < TLS_NR_CIPHERS; i++ )); do + [[ "${TLS_CIPHER_RFC_NAME[i]}" =~ CHACHA20 ]] && continue + [[ "${TLS_CIPHER_OSSL_NAME[i]}" =~ CHACHA20 ]] && continue + hexc="${TLS_CIPHER_HEXCODE[i]}" + if "${ciphers_found[i]}" && [[ ${#hexc} -eq 9 ]]; then + ciphers_found2[nr_ciphers]=false + hexcode[nr_ciphers]="${hexc:2:2},${hexc:7:2}" + rfc_ciph[nr_ciphers]="${TLS_CIPHER_RFC_NAME[i]}" + if [[ $proto == tls1_3 ]]; then + [[ "${hexc:2:2}" == "13" ]] && nr_ciphers+=1 + elif [[ $proto == tls1_2 ]]; then + [[ "${hexc:2:2}" != "13" ]] && nr_ciphers+=1 + elif [[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA256 ]] && \ + [[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA384 ]] && \ + [[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM" ]] && \ + [[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM_8" ]]; then + nr_ciphers+=1 + fi + fi + done + for (( i=0; i < TLS_NR_CIPHERS; i++ )); do + [[ "${TLS_CIPHER_RFC_NAME[i]}" =~ CHACHA20 ]] || [[ "${TLS_CIPHER_OSSL_NAME[i]}" =~ CHACHA20 ]] || continue hexc="${TLS_CIPHER_HEXCODE[i]}" if "${ciphers_found[i]}" && [[ ${#hexc} -eq 9 ]]; then ciphers_found2[nr_ciphers]=false