From dd35be2e4b86b06af3e34eff7b361ef8c682aea6 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Tue, 5 Apr 2022 11:09:34 -0400 Subject: [PATCH 1/3] Fix #2131 This commit fixes #2131 by having run_fs() attempt a TLS 1.2 ClientHello if the initial TLS 1.3 ClientHello fails. The TLS 1.2 ClientHello will offer many more curves than the TLS 1.3 ClientHello offers, and so it may succeed if the server supports ECDHE ciphers, but only with curves that were removed by RFC 8446. --- testssl.sh | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 173943c..acb6c05 100755 --- a/testssl.sh +++ b/testssl.sh @@ -10239,6 +10239,7 @@ run_fs() { local -a supported_curve local -i nr_supported_ciphers=0 nr_curves=0 nr_ossl_curves=0 i j low high local fs_ciphers curves_offered="" curves_to_test temp + local curves_option="" curves_list1="" curves_list2="" local len1 len2 curve_found local key_bitstring quality_str local -i len_dh_p quality @@ -10307,6 +10308,12 @@ run_fs() { tls_sockets "04" "${fs_hex_cipher_list:2}, 00,ff" sclient_success=$? [[ $sclient_success -eq 2 ]] && sclient_success=0 + # Sometimes a TLS 1.3 ClientHello will fail, but a TLS 1.2 ClientHello will succeed. See #2131. + if [[ $sclient_success -ne 0 ]]; then + tls_sockets "03" "${fs_hex_cipher_list:2}, 00,ff" + sclient_success=$? + [[ $sclient_success -eq 2 ]] && sclient_success=0 + fi else debugme echo $nr_supported_ciphers debugme echo $(actually_supported_osslciphers $fs_cipher_list "ALL") @@ -10320,6 +10327,38 @@ run_fs() { sclient_connect_successful $? $TMPFILE sclient_success=$? [[ $sclient_success -eq 0 ]] && [[ $(grep -ac "BEGIN CERTIFICATE" $TMPFILE) -eq 0 ]] && sclient_success=1 + # Sometimes a TLS 1.3 ClientHello will fail, but a TLS 1.2 ClientHello will succeed. See #2131. + if [[ $sclient_success -ne 0 ]] && "$HAS_TLS13"; then + # By default, OpenSSL 1.1.1 and above only include a few curves in the ClientHello, so in order + # to test all curves, the -curves option must be added. In addition, OpenSSL limits the number of + # curves that can be specified to 28. So, if more than 28 curves are supported, then the curves must + # be tested in batches. + if [[ "$(count_words "$OSSL_SUPPORTED_CURVES")" -le 28 ]]; then + curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" + curves_list1="${curves_list1// /:}" + else + # Place the first 28 supported curves in curves_list1 and the remainder in curves_list2. + curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" + curves_list1="${curves_list1// / }" + curves_list2="${curves_list1#* * * * * * * * * * * * * * * * * * * * * * * * * * * * }" + curves_list1="${curves_list1%$curves_list2}" + curves_list1="$(strip_trailing_space "$curves_list1")" + curves_list1="${curves_list1// /:}" + curves_list2="${curves_list2// /:}" + fi + curves_option="-curves $curves_list1" + $OPENSSL s_client $(s_client_options "-cipher $fs_cipher_list $curves_option $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE $TMPFILE 2>$ERRFILE $TMPFILE $TMPFILE Date: Fri, 8 Apr 2022 09:08:06 -0400 Subject: [PATCH 2/3] Improve compatibility with LibreSSL Older versions of LibreSSL that do not support TLS 1.3 only include a small list of curves in the supported_groups extension by default, so need to retry with curves explicitly defined even with versions of $OPENSSL that do not support TLS 1.3. --- testssl.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/testssl.sh b/testssl.sh index acb6c05..5985c6a 100755 --- a/testssl.sh +++ b/testssl.sh @@ -10328,18 +10328,17 @@ run_fs() { sclient_success=$? [[ $sclient_success -eq 0 ]] && [[ $(grep -ac "BEGIN CERTIFICATE" $TMPFILE) -eq 0 ]] && sclient_success=1 # Sometimes a TLS 1.3 ClientHello will fail, but a TLS 1.2 ClientHello will succeed. See #2131. - if [[ $sclient_success -ne 0 ]] && "$HAS_TLS13"; then + if [[ $sclient_success -ne 0 ]]; then # By default, OpenSSL 1.1.1 and above only include a few curves in the ClientHello, so in order # to test all curves, the -curves option must be added. In addition, OpenSSL limits the number of # curves that can be specified to 28. So, if more than 28 curves are supported, then the curves must # be tested in batches. + curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" + curves_list1="${curves_list1// / }" if [[ "$(count_words "$OSSL_SUPPORTED_CURVES")" -le 28 ]]; then - curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" curves_list1="${curves_list1// /:}" else # Place the first 28 supported curves in curves_list1 and the remainder in curves_list2. - curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" - curves_list1="${curves_list1// / }" curves_list2="${curves_list1#* * * * * * * * * * * * * * * * * * * * * * * * * * * * }" curves_list1="${curves_list1%$curves_list2}" curves_list1="$(strip_trailing_space "$curves_list1")" From 0be5ca530905273d52baace4be4b68ff73fb1b56 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 8 Apr 2022 10:09:30 -0400 Subject: [PATCH 3/3] Support DHE/ECDHE servers with uncommon curves When running in --ssl-native mode, run_fs() will not detect ECDHE ciphers if the server supports both DHE and ECDHE ciphers and the ECDHE ciphers are only supported with curves that are not offered by $OPENSSL by default. This commit fixes this by adding extra connection attempts with the -curves parameter explicitly provided. --- testssl.sh | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/testssl.sh b/testssl.sh index 5985c6a..1a22865 100755 --- a/testssl.sh +++ b/testssl.sh @@ -10323,28 +10323,26 @@ run_fs() { fileout "$jsonID" "WARN" "tests skipped as you only have $nr_supported_ciphers FS ciphers on the client site. ($CLIENT_MIN_FS are required)" return 1 fi + # By default, OpenSSL 1.1.1 and above only include a few curves in the ClientHello, so in order + # to test all curves, the -curves option must be added. In addition, OpenSSL limits the number of + # curves that can be specified to 28. So, if more than 28 curves are supported, then the curves must + # be tested in batches. + curves_list1="$(strip_trailing_space "$(strip_leading_space "$OSSL_SUPPORTED_CURVES")")" + curves_list1="${curves_list1// / }" + if [[ "$(count_words "$OSSL_SUPPORTED_CURVES")" -gt 28 ]]; then + # Place the first 28 supported curves in curves_list1 and the remainder in curves_list2. + curves_list2="${curves_list1#* * * * * * * * * * * * * * * * * * * * * * * * * * * * }" + curves_list1="${curves_list1%$curves_list2}" + curves_list1="$(strip_trailing_space "$curves_list1")" + curves_list2="${curves_list2// /:}" + fi + curves_list1="${curves_list1// /:}" $OPENSSL s_client $(s_client_options "-cipher $fs_cipher_list -ciphersuites ALL $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE $TMPFILE 2>$ERRFILE $TMPFILE $TMPFILE