From bcd71555ea154dab2e543311e1797ea656a35f60 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 14 Jul 2017 15:48:59 -0400 Subject: [PATCH] Make sure last ClientHello extension is not empty According to a discussion thread on the IETF TLS WG mail list (see https://www.ietf.org/mail-archive/web/tls/current/msg19720.html), there is at least one TLS server that will fail if the last extension in the ClientHello has contains extension_data of length 0. Currently, `tls_sockets()` will create such a ClientHello if: * The padding extension is included, and the length of the ClientHello without the padding data would be between 508 and 511 bytes. * No padding extension is included, and the caller provided `$extra_extensions` in which the last extension in `$extra_extensions` is empty. * No padding extension is included, `$extra_extensions` is empty, no ECC cipher suites are offered, and the ClientHello is for TLSv1.1 or below (in this case the next protocol extension would be that last one). This PR avoids the server bug (in nearly all cases) by ensuring the the padding extension (when present) always contains at least one byte, and by ensuring that when the padding extension is not present that the (non-empty) heartbeat extension is the last extension. This PR does leave one possible scenario in which the last extension would be empty. If the caller provides an `$extra_extensions` in which the last extension in `$extra_extensions` is empty, `tls_sockets()` does not add a padding extension (or a padding extension is included in `$extra_extensions`), and `$extra_extensions` includes a heartbeat extension, then the last extension in the ClientHello would be empty. This, however, is a highly unlikely scenario, and certainly there are currently no such calls to `tls_sockets()` in testssl.sh. --- testssl.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/testssl.sh b/testssl.sh index 6b11544..8bb36dc 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8625,10 +8625,7 @@ socksend_tls_clienthello() { ,00, $len_servername_hex # server_name length. We assume len(hostname) < FF - 9 ,$servername_hexstr" # server_name target fi - if [[ ! "$extra_extensions_list" =~ " 000f " ]]; then - [[ -n "$all_extensions" ]] && all_extensions+="," - all_extensions+="$extension_heartbeat" - fi + if [[ ! "$extra_extensions_list" =~ " 0023 " ]]; then [[ -n "$all_extensions" ]] && all_extensions+="," all_extensions+="$extension_session_ticket" @@ -8661,6 +8658,13 @@ socksend_tls_clienthello() { all_extensions+="$extra_extensions" fi + # Make sure that a non-empty extension goes last (either heartbeat or padding). + # See PR #792 and https://www.ietf.org/mail-archive/web/tls/current/msg19720.html. + if [[ ! "$extra_extensions_list" =~ " 000f " ]]; then + [[ -n "$all_extensions" ]] && all_extensions+="," + all_extensions+="$extension_heartbeat" + fi + code2network "$all_extensions" # convert extensions all_extensions="$NW_STR" # we don't have the leading \x here so string length is two byte less, see next len_extension=${#all_extensions} @@ -8674,7 +8678,7 @@ socksend_tls_clienthello() { "$offer_compression" && len_all+=2 if [[ $len_all -ge 256 ]] && [[ $len_all -le 511 ]] && [[ ! "$extra_extensions_list" =~ " 0015 " ]]; then if [[ $len_all -gt 508 ]]; then - len_padding_extension=0 + len_padding_extension=1 # Final extension cannot be empty: see PR #792 else len_padding_extension=$((508 - 0x$len_ciph_suites - 0x2b - 0x$len_extension_hex - 0x2)) fi