Skip to content

Commit 1e4dd4d

Browse files
author
David Cooper
authored
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 PR 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.
1 parent aaf7248 commit 1e4dd4d

1 file changed

Lines changed: 73 additions & 22 deletions

File tree

‎testssl.sh‎

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6618,14 +6618,24 @@ run_server_preference() {
66186618
}
66196619

66206620
check_tls12_pref() {
6621+
local chacha20_ciphers="" non_chacha20_ciphers=""
66216622
local batchremoved="-CAMELLIA:-IDEA:-KRB5:-PSK:-SRP:-aNULL:-eNULL"
66226623
local batchremoved_success=false
66236624
local tested_cipher="" cipher ciphers_to_test
66246625
local order=""
66256626
local -i nr_ciphers_found_r1=0 nr_ciphers_found_r2=0
66266627

6628+
# Place ChaCha20 ciphers at the end of the list to avoid accidentally
6629+
# triggering the server's PrioritizeChaCha setting.
6630+
ciphers_to_test="$(actually_supported_osslciphers "ALL:$batchremoved" "" "")"
6631+
for cipher in $(colon_to_spaces "$ciphers_to_test"); do
6632+
[[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher:" || non_chacha20_ciphers+="$cipher:"
6633+
done
6634+
ciphers_to_test="$non_chacha20_ciphers$chacha20_ciphers"
6635+
ciphers_to_test="${ciphers_to_test%:}"
6636+
66276637
while true; do
6628-
$OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "ALL$tested_cipher:$batchremoved" -connect $NODEIP:$PORT $PROXY $SNI") </dev/null 2>>$ERRFILE >$TMPFILE
6638+
$OPENSSL s_client $(s_client_options "$STARTTLS -tls1_2 $BUGS -cipher "$ciphers_to_test$tested_cipher" -connect $NODEIP:$PORT $PROXY $SNI") </dev/null 2>>$ERRFILE >$TMPFILE
66296639
if sclient_connect_successful $? $TMPFILE ; then
66306640
cipher=$(get_cipher $TMPFILE)
66316641
order+=" $cipher"
@@ -6659,7 +6669,14 @@ check_tls12_pref() {
66596669

66606670
if "$batchremoved_success"; then
66616671
# now we combine the two cipher sets from both while loops
6662-
combined_ciphers="$order"
6672+
6673+
# Place ChaCha20 ciphers at the end of the list to avoid accidentally
6674+
# triggering the server's PrioritizeChaCha setting.
6675+
chacha20_ciphers=""; non_chacha20_ciphers=""
6676+
for cipher in $order; do
6677+
[[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher " || non_chacha20_ciphers+="$cipher "
6678+
done
6679+
combined_ciphers="$non_chacha20_ciphers$chacha20_ciphers"
66636680
order="" ; tested_cipher=""
66646681
while true; do
66656682
ciphers_to_test=""
@@ -6697,15 +6714,13 @@ check_tls12_pref() {
66976714
cipher_pref_check() {
66986715
local p="$1" proto_hex="$2" proto="$3"
66996716
local using_sockets="$4"
6700-
local tested_cipher cipher order rfc_cipher rfc_order
6701-
local overflow_probe_cipherlist="ALL:-ECDHE-RSA-AES256-GCM-SHA384:-AES128-SHA:-DES-CBC3-SHA"
6717+
local tested_cipher cipher order="" rfc_cipher rfc_order
67026718
local -i i nr_ciphers nr_nonossl_ciphers num_bundles mod_check bundle_size bundle end_of_bundle success
6703-
local hexc ciphers_to_test
6719+
local hexc ciphers_to_test cipher_list chacha20_ciphers non_chacha20_ciphers
67046720
local -a rfc_ciph hexcode ciphers_found ciphers_found2
67056721
local -a -i index
6706-
local ciphers_found_with_sockets
6722+
local ciphers_found_with_sockets=false
67076723

6708-
order=""; ciphers_found_with_sockets=false
67096724
if [[ $p == ssl3 ]] && ! "$HAS_SSL3" && ! "$using_sockets"; then
67106725
out "\n SSLv3: "; pr_local_problem "$OPENSSL doesn't support \"s_client -ssl3\"";
67116726
return 0
@@ -6721,16 +6736,28 @@ cipher_pref_check() {
67216736
if [[ $p == tls1_2 ]] && "$SERVER_SIZE_LIMIT_BUG"; then
67226737
order="$(check_tls12_pref)"
67236738
else
6739+
# Place ChaCha20 ciphers at the end of the list to avoid accidentally
6740+
# triggering the server's PrioritizeChaCha setting.
6741+
cipher_list=""; chacha20_ciphers=""; non_chacha20_ciphers=""
6742+
if [[ $p == tls1_3 ]]; then
6743+
cipher_list="$(colon_to_spaces "$TLS13_OSSL_CIPHERS")"
6744+
else
6745+
cipher_list="$(colon_to_spaces "$(actually_supported_osslciphers "ALL:COMPLEMENTOFALL" "" "")")"
6746+
fi
6747+
for cipher in $cipher_list; do
6748+
[[ "$cipher" =~ CHACHA20 ]] && chacha20_ciphers+="$cipher " || non_chacha20_ciphers+="$cipher "
6749+
done
6750+
cipher_list="$non_chacha20_ciphers $chacha20_ciphers"
67246751
tested_cipher=""
67256752
while true; do
6753+
ciphers_to_test=""
6754+
for cipher in $cipher_list; do
6755+
[[ ! "$tested_cipher:" =~ :-$cipher: ]] && ciphers_to_test+=":$cipher"
6756+
done
6757+
[[ -z "$ciphers_to_test" ]] && break
67266758
if [[ $p != tls1_3 ]]; then
6727-
ciphers_to_test="-cipher ALL:COMPLEMENTOFALL${tested_cipher}"
6759+
ciphers_to_test="-cipher ${ciphers_to_test:1}"
67286760
else
6729-
ciphers_to_test=""
6730-
for cipher in $(colon_to_spaces "$TLS13_OSSL_CIPHERS"); do
6731-
[[ ! "$tested_cipher" =~ ":-"$cipher ]] && ciphers_to_test+=":$cipher"
6732-
done
6733-
[[ -z "$ciphers_to_test" ]] && break
67346761
ciphers_to_test="-ciphersuites ${ciphers_to_test:1}"
67356762
fi
67366763
$OPENSSL s_client $(s_client_options "$STARTTLS -"$p" $BUGS $ciphers_to_test -connect $NODEIP:$PORT $PROXY $SNI") </dev/null 2>>$ERRFILE >$TMPFILE
@@ -6750,7 +6777,7 @@ cipher_pref_check() {
67506777
ciphers_found[i]=false
67516778
hexc="${TLS_CIPHER_HEXCODE[i]}"
67526779
if [[ ${#hexc} -eq 9 ]]; then
6753-
if [[ " $order " =~ " ${TLS_CIPHER_OSSL_NAME[i]} " ]]; then
6780+
if [[ " $order " =~ \ ${TLS_CIPHER_OSSL_NAME[i]}\ ]]; then
67546781
ciphers_found[i]=true
67556782
else
67566783
ciphers_found2[nr_nonossl_ciphers]=false
@@ -6764,8 +6791,8 @@ cipher_pref_check() {
67646791
[[ "${hexc:2:2}" != 13 ]] && nr_nonossl_ciphers+=1
67656792
elif [[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA256 ]] && \
67666793
[[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA384 ]] && \
6767-
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM" ]] && \
6768-
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM_8" ]]; then
6794+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM ]] && \
6795+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM_8 ]]; then
67696796
nr_nonossl_ciphers+=1
67706797
fi
67716798
fi
@@ -6822,22 +6849,46 @@ cipher_pref_check() {
68226849
# If there is a SERVER_SIZE_LIMIT_BUG, then use sockets to find the cipher
68236850
# order, but starting with the list of ciphers supported by the server.
68246851
if "$ciphers_found_with_sockets"; then
6852+
# Create an array of the ciphers to test with any ChaCha20
6853+
# listed last in order to avoid accidentally triggering the
6854+
# server's PriorizeChaCha setting.
68256855
order=""
68266856
nr_ciphers=0
68276857
for (( i=0; i < TLS_NR_CIPHERS; i++ )); do
6858+
[[ "${TLS_CIPHER_RFC_NAME[i]}" =~ CHACHA20 ]] && continue
6859+
[[ "${TLS_CIPHER_OSSL_NAME[i]}" =~ CHACHA20 ]] && continue
6860+
hexc="${TLS_CIPHER_HEXCODE[i]}"
6861+
if "${ciphers_found[i]}" && [[ ${#hexc} -eq 9 ]]; then
6862+
ciphers_found2[nr_ciphers]=false
6863+
hexcode[nr_ciphers]="${hexc:2:2},${hexc:7:2}"
6864+
rfc_ciph[nr_ciphers]="${TLS_CIPHER_RFC_NAME[i]}"
6865+
if [[ "$p" == tls1_3 ]]; then
6866+
[[ "${hexc:2:2}" == 13 ]] && nr_ciphers+=1
6867+
elif [[ "$p" == tls1_2 ]]; then
6868+
[[ "${hexc:2:2}" != 13 ]] && nr_ciphers+=1
6869+
elif [[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA256 ]] && \
6870+
[[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA384 ]] && \
6871+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM ]] && \
6872+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM_8 ]]; then
6873+
nr_ciphers+=1
6874+
fi
6875+
fi
6876+
done
6877+
for (( i=0; i < TLS_NR_CIPHERS; i++ )); do
6878+
[[ "${TLS_CIPHER_RFC_NAME[i]}" =~ CHACHA20 ]] || [[ "${TLS_CIPHER_OSSL_NAME[i]}" =~ CHACHA20 ]] || continue
68286879
hexc="${TLS_CIPHER_HEXCODE[i]}"
68296880
if "${ciphers_found[i]}" && [[ ${#hexc} -eq 9 ]]; then
68306881
ciphers_found2[nr_ciphers]=false
68316882
hexcode[nr_ciphers]="${hexc:2:2},${hexc:7:2}"
68326883
rfc_ciph[nr_ciphers]="${TLS_CIPHER_RFC_NAME[i]}"
6833-
if [[ "$p" == "tls1_3" ]]; then
6834-
[[ "${hexc:2:2}" == "13" ]] && nr_ciphers+=1
6835-
elif [[ "$p" == "tls1_2" ]]; then
6836-
[[ "${hexc:2:2}" != "13" ]] && nr_ciphers+=1
6884+
if [[ "$p" == tls1_3 ]]; then
6885+
[[ "${hexc:2:2}" == 13 ]] && nr_ciphers+=1
6886+
elif [[ "$p" == tls1_2 ]]; then
6887+
[[ "${hexc:2:2}" != 13 ]] && nr_ciphers+=1
68376888
elif [[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA256 ]] && \
68386889
[[ ! "${TLS_CIPHER_RFC_NAME[i]}" =~ SHA384 ]] && \
6839-
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM" ]] && \
6840-
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *"_CCM_8" ]]; then
6890+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM ]] && \
6891+
[[ "${TLS_CIPHER_RFC_NAME[i]}" != *_CCM_8 ]]; then
68416892
nr_ciphers+=1
68426893
fi
68436894
fi

0 commit comments

Comments
 (0)