From 9b9f4350597f3c9fff2bd921a4f09abbc9e62dba Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 21 Sep 2018 15:08:29 -0400 Subject: [PATCH] Fix #1118 This PR is an attempt to fix the problem identified in #1118. Currently, get_cipher() and get_protocol() attempt the extract the cipher and protocol from the SSL-Session information printed by OpenSSL s_client. This does not always work for TLSv1.3, however, since OpenSSL 1.1.1 will only print SSL-Session information for a TLSv1.3 connection if the server sends New Session Ticket. If the server doesn't, then get_cipher() and get_protocol() return empty strings. For TLSv1.3 connections in which the server does not send a New Session Ticket, there seems to be only one other source for this information. A line of the form: New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384 [Note that "New" would be "Reused" if the connection were created via session resumption.] The use of this line seems to be okay for extracting the negotiated cipher, but it cannot be used in general to extract the negotiated protocol. The reason is that this line is created as follows: c = SSL_get_current_cipher(s); BIO_printf(bio, "%s, Cipher is %s\n", SSL_CIPHER_get_version(c), SSL_CIPHER_get_name(c)); While the cipher that is printed seems to be the negotiated cipher, the protocol that is printed is "the SSL/TLS protocol version that first defined the cipher." Since TLS 1.3 ciphers may only be used with TLS 1.3, protocol version printed on this line may be accepted as the negotiated protocol if and only if it is "TLSv1.3." This PR addresses the problem by modifying get_cipher() and get_protocol() to check the "New, ..., Cipher is ..." line if lines from SSL-Session ("Cipher : ...", "Protocol : ...") cannot be found. In the case of get_protocol() the protocol on the "New, ..., Cipher is ..." will be accepted only if the protocol is "TLSv1.3" and the cipher is a TLSv1.3 cipher. This PR also adds a check for the "New, ..., Cipher is ..." to sclient_connect_successful(). If this line is present, and the protocol and cipher are not "(NONE)", then this is accepted as an indication that the connection was successful even if the "Master-Key" line does not appear. It is not clear whether this extra test is needed, however, as sclient_connect_successful() will not even look at the text in the output of OpenSSL s_client if function's return value is 0, and OpenSSL s_client should return 0 if the connection was successful. --- testssl.sh | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/testssl.sh b/testssl.sh index 5d4f31f..25cab29 100755 --- a/testssl.sh +++ b/testssl.sh @@ -1225,13 +1225,36 @@ strip_trailing_space() { # retrieve cipher from ServerHello (via openssl) get_cipher() { - awk '/Cipher *:/ { a=$3 } END { print a }' "$1" - #awk '/\/ && !/Cipher is/ && !/^New/ { print $3 }' "$1" + local cipher="" + local server_hello="$(cat "$1")" + + if [[ "$server_hello" =~ Cipher\ *:\ ([A-Z0-9]+-[A-Z0-9\-]+|TLS_[A-Z0-9_]+) ]]; then + cipher="${BASH_REMATCH##* }" + elif [[ "$server_hello" =~ (New|Reused)", "(SSLv[23]|TLSv1(\.[0-3])?(\/SSLv3)?)", Cipher is "([A-Z0-9]+-[A-Z0-9\-]+|TLS_[A-Z0-9_]+) ]]; then + cipher="${BASH_REMATCH##* }" + fi + tm_out "$cipher" } # retrieve protocol from ServerHello (via openssl) get_protocol() { - awk '/Protocol *:/ { a=$3 } END { print a }' "$1" + local protocol="" + local server_hello="$(cat "$1")" + + if [[ "$server_hello" =~ Protocol\ *:\ (SSLv[23]|TLSv1(\.[0-3])?) ]]; then + protocol="${BASH_REMATCH##* }" + elif [[ "$server_hello" =~ (New|Reused)", TLSv1.3, Cipher is "TLS_[A-Z0-9_]+ ]]; then + # Note: When OpenSSL prints "New, , Cipher is ", is the + # negotiated cipher, but is not the negotiated protocol. Instead, it is + # the SSL/TLS protocol that first defined . Since the ciphers that were + # first defined for TLSv1.3 may only be used with TLSv1.3, this line may be used + # to determine whether TLSv1.3 was negotiated, but if another protocol is specified + # on this line, then this line does not indicate the actual protocol negotiated. Also, + # only TLSv1.3 cipher suites have names that begin with TLS_, which provides additional + # assurance that the above match will only succeed if TLSv1.3 was negotiated. + protocol="TLSv1.3" + fi + tm_out "$protocol" } is_number() { @@ -6518,7 +6541,7 @@ sclient_connect_successful() { if [[ "$server_hello" =~ $re ]]; then [[ -n "${BASH_REMATCH[1]}" ]] && return 0 fi - # further check like ~ fgrep 'Cipher is (NONE)' "$2" &> /dev/null && return 1' not done. + [[ "$server_hello" =~ (New|Reused)", "(SSLv[23]|TLSv1(\.[0-3])?(\/SSLv3)?)", Cipher is "([A-Z0-9]+-[A-Z0-9\-]+|TLS_[A-Z0-9_]+) ]] && return 0 # what's left now is: master key empty and Session-ID not empty # ==> probably client-based auth with x509 certificate. We handle that at other places #