From 00f613f62d286dbbdb816b486314c0dff864ac6b Mon Sep 17 00:00:00 2001 From: David Cooper Date: Thu, 14 Nov 2019 16:41:19 -0500 Subject: [PATCH 1/5] WIP: Don't call s_client for unsupported protocol versions This PR fixes a couple of places where "$OPENSSL s_client" is called with "-ssl3" even if SSLv3 is not supported. The fix in ciphers_by_strength() is easy, as the issue only occurs if "$using_sockets" is true. If SSLv3 (or TLSv1.3) is not supported, then testing using "$OPENSSL s_client" is skipped and all of the supported ciphers are found using tls_sockets(). The fix for run_tls_fallback_scsv() is more complicated. While it is easy to avoid calling "$OPENSSL s_client" with "-ssl3" if SSLv3 is not supported, it is not easy to determine the correct message to present to the user if support for SSLv3 (and possibly also TLSv1.3) is unknown. For the case in which $high_proto cannot be set, I believe that I have covered all of the possibilities, but an not sure if the correct message/rating is used in every case. For the case in which it is not possible to determine whether SSLv3 is the $low_proto, more could be done. If $high_proto is TLS 1.1 or TLS 1, then this PR is okay, as it is possible that SSLv3 would be the fallback protocol, but there is no way to tell. However, it seems unlikely that a server would support TLS 1.2 and SSLv3, but not TLS 1.1 or TLS 1. So, perhaps if $high_proto is TLS 1.2 and the server does not support TLS 1.1 or TLS 1, it should just be assumed that SSLv3 is not supported, even if it cannot be tested. --- testssl.sh | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/testssl.sh b/testssl.sh index e682815..49bc19e 100755 --- a/testssl.sh +++ b/testssl.sh @@ -4083,14 +4083,16 @@ ciphers_by_strength() { fi else # no SSLv2 nr_ossl_ciphers=0 - for (( i=0; i < nr_ciphers; i++ )); do - if "${ossl_supported[i]}"; then - ciphers_found2[nr_ossl_ciphers]=false - ciph2[nr_ossl_ciphers]="${ciph[i]}" - index[nr_ossl_ciphers]=$i - nr_ossl_ciphers+=1 - fi - done + if ( "$HAS_SSL3" || [[ $proto != -ssl3 ]] ) && ( "$HAS_TLS13" || [[ $proto != -tls1_3 ]] ); then + for (( i=0; i < nr_ciphers; i++ )); do + if "${ossl_supported[i]}"; then + ciphers_found2[nr_ossl_ciphers]=false + ciph2[nr_ossl_ciphers]="${ciph[i]}" + index[nr_ossl_ciphers]=$i + nr_ossl_ciphers+=1 + fi + done + fi if [[ $nr_ossl_ciphers -eq 0 ]]; then num_bundles=0 else @@ -14785,6 +14787,7 @@ run_tls_fallback_scsv() { high_proto="$p" break fi + [[ "$p" == ssl3 ]] && ! "$HAS_SSL3" && continue $OPENSSL s_client $(s_client_options "-$p $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE Date: Fri, 15 Nov 2019 10:03:04 -0500 Subject: [PATCH 2/5] Improve check for $low_proto in run_tls_fallback_scsv() If $high_proto is set to something other than SSLv3, support for SSLv3 will not have been determined by determine_optimal_sockets_params(), but it may have been determined later (e.g., by run_protocols()). So, this commit changes the loop to always check for SSLv3 support (without calling "$OPENSSL s_client" if $HAS_SSL3 is false). The check for whether the fallback test can be performed is moved until after the loop --- testssl.sh | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/testssl.sh b/testssl.sh index 49bc19e..d0b9537 100755 --- a/testssl.sh +++ b/testssl.sh @@ -14840,16 +14840,12 @@ run_tls_fallback_scsv() { # Next find a second protocol that the server supports. for p in $protos_to_try; do - if [[ "$p" == ssl3 ]] && ! "$HAS_SSL3"; then - prln_local_problem "Can't test: $OPENSSL does not support SSLv3" - fileout "$jsonID" "WARN" "Can't test: $OPENSSL does not support SSLv3" - return 1 - fi [[ $(has_server_protocol "$p") -eq 1 ]] && continue if [[ $(has_server_protocol "$p") -eq 0 ]]; then low_proto="$p" break fi + [[ "$p" == ssl3 ]] && ! "$HAS_SSL3" && continue $OPENSSL s_client $(s_client_options "-$p $STARTTLS $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE Date: Wed, 18 Dec 2019 09:56:37 -0500 Subject: [PATCH 3/5] Minor tweaks to run_tls_fallback_scsv() --- testssl.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index d0b9537..c284a10 100755 --- a/testssl.sh +++ b/testssl.sh @@ -14821,11 +14821,12 @@ run_tls_fallback_scsv() { # This may be an SSLv2-only server, if $OPENSSL does not support SSLv2. prln_warning "test failed (couldn't connect)" fileout "$jsonID" "WARN" "Check failed. (couldn't connect)" + return 1 elif [[ $(has_server_protocol tls1_3) -eq 1 ]]; then # If the server does not support TLS 1.3, TLS 1.2, TLS 1.1, or TLS 1, and # support for SSLv3 cannot be tested, then treat it as HIGH severity, since # it is very likely that SSLv3 is the only supported protocol. - prln_svrty_high "No fallback possible, TLS 1.2, TLS 1.1, and TLS 1 not supported (OK)" + prln_svrty_high "No fallback possible, TLS 1.2, TLS 1.1, and TLS 1 not supported" fileout "$jsonID" "HIGH" "TLS 1.2, TLS 1.1, and TLS 1 not supported" else # TLS 1.2, TLS 1.1, and TLS 1 are not supported, but can't tell whether TLS 1.3 is supported. @@ -14855,7 +14856,7 @@ run_tls_fallback_scsv() { if ! "$HAS_SSL3" && \ ( [[ "$low_proto" == ssl3 ]] || \ - ( [[ "$high_proto" == tls1 ]] && [[ $(has_server_protocol "$p") -eq 2 ]] ) ); then + ( [[ "$high_proto" == tls1 ]] && [[ $(has_server_protocol "ssl3") -eq 2 ]] ) ); then # If the protocol that the server would fall back to is SSLv3, but $OPENSSL does # not support SSLv3, then the test cannot be performed. So, if $OPENSSL does not # support SSLv3 and it is known that SSLv3 is the fallback protocol ($low_proto), then From a0b2fb5d569c371452afe50bad1208a3383038cd Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 18 Dec 2019 10:17:55 -0500 Subject: [PATCH 4/5] Minor tweak to run_tls_fallback_scsv() Don't report "OK" if the server may be TLS 1.3-only or SSLv3-only, as one is very good and one is very bad. --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index c284a10..fdcd948 100755 --- a/testssl.sh +++ b/testssl.sh @@ -14833,7 +14833,7 @@ run_tls_fallback_scsv() { # This could be a TLS 1.3 only server, an SSLv3 only server (if SSLv3 support cannot be tested), # or a server that does not support SSLv3 or any TLS protocol. So, don't report a severity, # since this could either be good or bad. - outln "No fallback possible, TLS 1.2, TLS 1.1, and TLS 1 not supported (OK)" + outln "No fallback possible, TLS 1.2, TLS 1.1, and TLS 1 not supported" fileout "$jsonID" "INFO" "TLS 1.2, TLS 1.1, and TLS 1 not supported" fi return 0 From 7c1b8139b2153aab45a7f4f3e996d527698d1f19 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 18 Dec 2019 10:22:14 -0500 Subject: [PATCH 5/5] Minor tweak to run_tls_fallback_scsv() If the server is known not to support TLS 1.3 (as well as TLS 1.2, TLS 1.1, and TLS 1), then mention TLS 1.3 in the list of not supported protocols. While lack of TLS 1.3 support is not part of the reason that no fallback is possible, it is part of the reason that the result is reported as prln_svrty_high. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index fdcd948..7f1fe77 100755 --- a/testssl.sh +++ b/testssl.sh @@ -14826,8 +14826,8 @@ run_tls_fallback_scsv() { # If the server does not support TLS 1.3, TLS 1.2, TLS 1.1, or TLS 1, and # support for SSLv3 cannot be tested, then treat it as HIGH severity, since # it is very likely that SSLv3 is the only supported protocol. - prln_svrty_high "No fallback possible, TLS 1.2, TLS 1.1, and TLS 1 not supported" - fileout "$jsonID" "HIGH" "TLS 1.2, TLS 1.1, and TLS 1 not supported" + prln_svrty_high "No fallback possible, TLS 1.3, TLS 1.2, TLS 1.1, and TLS 1 not supported" + fileout "$jsonID" "HIGH" "TLS 1.3, TLS 1.2, TLS 1.1, and TLS 1 not supported" else # TLS 1.2, TLS 1.1, and TLS 1 are not supported, but can't tell whether TLS 1.3 is supported. # This could be a TLS 1.3 only server, an SSLv3 only server (if SSLv3 support cannot be tested),