From 25fef829770c277f7fdbb744b3b77f77ac8a0d0a Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 31 Aug 2018 12:47:19 -0400 Subject: [PATCH 1/2] A ClientHello length intolerance bug Just as some servers will fail if the length of the ClientHello is between 256 and 511 bytes (see RFC 7685), it seems that some servers (or a middlebox sitting in front of the servers) will fail if the length of the ClientHello is 522, 778, 1034, ... bytes in length (i.e., if length mod 256 = 10). I have also encountered one server that will also fail if the length of the ClientHello is 526, 782, 1038, ... bytes in length (i.e., if length mod 256 = 14). In the case of that one server, the first ClientHello sent by run_pfs() was exactly 1038 bytes, and so run_pfs() was reporting that the server didn't support any PFS ciphers even though it did.. This PR addresses the problem in two ways. First, it modifies socksend_tls_clienthello() so that if the length of the ClientHello would be more than 511 bytes and length mod 256 would be 10 or 14, it adds a 5-byte padding extension in order to ensure that the final length of the ClientHello will not be a length that could trigger the bug. Second, this PR adds a test to run_grease() to send ClientHello messages of the exact lengths that do trigger the bug so that users can be made aware that their servers have the problem. --- testssl.sh | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index 7647f11..609f09f 100755 --- a/testssl.sh +++ b/testssl.sh @@ -11827,6 +11827,7 @@ socksend_tls_clienthello() { # then add a padding extension (see RFC 7685) len_all=$((0x$len_ciph_suites + 0x2b + 0x$len_extension_hex + 0x2)) "$offer_compression" && len_all+=2 + [[ 0x$tls_low_byte -gt 0x03 ]] && len_all+=32 # TLSv1.3 ClientHello includes a 32-byte session id if [[ $len_all -ge 256 ]] && [[ $len_all -le 511 ]] && [[ ! "$extra_extensions_list" =~ " 0015 " ]]; then if [[ $len_all -gt 508 ]]; then len_padding_extension=1 # Final extension cannot be empty: see PR #792 @@ -11841,12 +11842,18 @@ socksend_tls_clienthello() { done len_extension=$len_extension+$len_padding_extension+0x4 len_extension_hex=$(printf "%02x\n" $len_extension) + elif [[ ! "$extra_extensions_list" =~ " 0015 " ]] && ( [[ $((len_all%256)) -eq 10 ]] || [[ $((len_all%256)) -eq 14 ]] ); then + # Some servers fail if the length of the ClientHello is 522, 778, 1034, 1290, ... bytes. + # A few servers also fail if the length is 526, 782, 1038, 1294, ... bytes. + # So, if the ClientHello would be one of these length, add a 5-byte padding extension. + all_extensions="$all_extensions\\x00\\x15\\x00\\x01\\x00" + len_extension+=5 + len_extension_hex=$(printf "%02x\n" $len_extension) fi len2twobytes "$len_extension_hex" all_extensions=" ,$LEN_STR # first the len of all extensions. ,$all_extensions" - fi if [[ 0x$tls_low_byte -gt 0x03 ]]; then @@ -14477,7 +14484,7 @@ run_grease() { local alpn_proto alpn alpn_list_len_hex extn_len_hex local selected_alpn_protocol grease_selected_alpn_protocol local ciph list temp curve_found - local -i i j rnd alpn_list_len extn_len debug_level="" + local -i i j rnd alpn_list_len extn_len base_len clienthello_len debug_level="" local -i ret=0 # Note: The following values were taken from https://datatracker.ietf.org/doc/draft-ietf-tls-grease. # These arrays may need to be updated if the values change in the final version of this document. @@ -14681,6 +14688,34 @@ run_grease() { fi fi + # Some servers fail if the length of the ClientHello is 522, 778, 1034, 1290, ... bytes. + # A few servers also fail if the length is 526, 782, 1038, 1294, ... bytes. + # So, send a number of ClientHello messages of extactly these lengths to see whether + # the connection fails. + if "$normal_hello_ok" && [[ "$proto" != "00" ]]; then + if [[ -z "$SNI" ]]; then + base_len=426 + else + base_len=423+${#SNI} + fi + for clienthello_len in 522 526 778 782 1034 1290 1546 1802 2058; do + extn_len=$((clienthello_len-base_len)) + extn="" + for (( i=0; i < extn_len; i++ )); do + extn+=",00" + done + extn_len_hex=$(printf "%04x" $extn_len) + debugme echo -e "\nSending ClientHello with length $clienthello_len bytes" + tls_sockets "$proto" "$cipher_list" "" "00,15,${extn_len_hex:0:2},${extn_len_hex:2:2}${extn}" + success=$? + if [[ $success -ne 0 ]] && [[ $success -ne 2 ]]; then + prln_svrty_medium " Server fails if ClientHello is $clienthello_len bytes in length." + fileout "$jsonID" "CRITICAL" "Server fails if ClientHello is $clienthello_len bytes in length." + bug_found=true + fi + done + fi + # Check that server ignores unrecognized cipher suite values # see https://datatracker.ietf.org/doc/draft-ietf-tls-grease if "$normal_hello_ok"; then From 767ee94cb3f212a22c0a1cb7b352863666541312 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 12 Sep 2018 11:17:27 -0400 Subject: [PATCH 2/2] Some updates to size bug GREASE test This commit updates the size bug GREASE test in a few ways: * It removes the changes to socksend_tls_clienthello() - these will be submitted as a separate PR. * It adds a test for a ClientHello message length of 266 bytes, but only if the server can generally handle messages with lengths between 256 and 511 bytes. * It corrects the calculation of the length of the padding extension in cases in which a TLS 1 or TLS 1.1 ClientHello is being sent. --- testssl.sh | 56 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/testssl.sh b/testssl.sh index 609f09f..52f5329 100755 --- a/testssl.sh +++ b/testssl.sh @@ -11827,7 +11827,6 @@ socksend_tls_clienthello() { # then add a padding extension (see RFC 7685) len_all=$((0x$len_ciph_suites + 0x2b + 0x$len_extension_hex + 0x2)) "$offer_compression" && len_all+=2 - [[ 0x$tls_low_byte -gt 0x03 ]] && len_all+=32 # TLSv1.3 ClientHello includes a 32-byte session id if [[ $len_all -ge 256 ]] && [[ $len_all -le 511 ]] && [[ ! "$extra_extensions_list" =~ " 0015 " ]]; then if [[ $len_all -gt 508 ]]; then len_padding_extension=1 # Final extension cannot be empty: see PR #792 @@ -11842,18 +11841,12 @@ socksend_tls_clienthello() { done len_extension=$len_extension+$len_padding_extension+0x4 len_extension_hex=$(printf "%02x\n" $len_extension) - elif [[ ! "$extra_extensions_list" =~ " 0015 " ]] && ( [[ $((len_all%256)) -eq 10 ]] || [[ $((len_all%256)) -eq 14 ]] ); then - # Some servers fail if the length of the ClientHello is 522, 778, 1034, 1290, ... bytes. - # A few servers also fail if the length is 526, 782, 1038, 1294, ... bytes. - # So, if the ClientHello would be one of these length, add a 5-byte padding extension. - all_extensions="$all_extensions\\x00\\x15\\x00\\x01\\x00" - len_extension+=5 - len_extension_hex=$(printf "%02x\n" $len_extension) fi len2twobytes "$len_extension_hex" all_extensions=" ,$LEN_STR # first the len of all extensions. ,$all_extensions" + fi if [[ 0x$tls_low_byte -gt 0x03 ]]; then @@ -14479,7 +14472,7 @@ run_tls_truncation() { run_grease() { local -i success local bug_found=false - local normal_hello_ok=false + local normal_hello_ok=false clienthello_size_bug=false local cipher_list proto selected_cipher selected_cipher_hex="" extn rnd_bytes local alpn_proto alpn alpn_list_len_hex extn_len_hex local selected_alpn_protocol grease_selected_alpn_protocol @@ -14685,6 +14678,7 @@ run_grease() { prln_svrty_medium " Server fails if ClientHello is between 256 and 511 bytes in length." fileout "$jsonID" "CRITICAL" "Server fails if ClientHello is between 256 and 511 bytes in length." bug_found=true + clienthello_size_bug=true fi fi @@ -14692,21 +14686,51 @@ run_grease() { # A few servers also fail if the length is 526, 782, 1038, 1294, ... bytes. # So, send a number of ClientHello messages of extactly these lengths to see whether # the connection fails. - if "$normal_hello_ok" && [[ "$proto" != "00" ]]; then - if [[ -z "$SNI" ]]; then - base_len=426 + if "$normal_hello_ok" && [[ "$proto" != "00" ]] && [[ ${#SNI} -le 92 ]]; then + if [[ "$proto" == "03" ]]; then + if [[ -z "$SNI" ]]; then + base_len=426 + else + base_len=423+${#SNI} + fi else - base_len=423+${#SNI} + if [[ -z "$SNI" ]]; then + base_len=294 + else + base_len=291+${#SNI} + fi fi - for clienthello_len in 522 526 778 782 1034 1290 1546 1802 2058; do - extn_len=$((clienthello_len-base_len)) + for clienthello_len in 266 522 526 778 782 1034 1290 1546 1802 2058; do + if [[ $clienthello_len -eq 266 ]]; then + # If the server cannot handle ClientHello message lengths between + # 256 and 511, there is no need to test a ClientHello message of + # length 266. + "$clienthello_size_bug" && continue + # In order to create a message of length 266, need to send a message + # with fewer cipher suites, so the "true" base length of the messsage + # will be shorter. + extn_len=$((clienthello_len+250-base_len)) + else + extn_len=$((clienthello_len-base_len)) + fi extn="" for (( i=0; i < extn_len; i++ )); do extn+=",00" done extn_len_hex=$(printf "%04x" $extn_len) debugme echo -e "\nSending ClientHello with length $clienthello_len bytes" - tls_sockets "$proto" "$cipher_list" "" "00,15,${extn_len_hex:0:2},${extn_len_hex:2:2}${extn}" + if [[ $clienthello_len -eq 266 ]]; then + # Some extensions are only included if the ClientHello includes a TLS_ECDHE cipher suite. + # So, add one (either c0,01 or c0,02 in order to ensure that the message length will + # be correct. + if [[ "$selected_cipher_hex" == "C0,01" ]]; then + tls_sockets "$proto" "$selected_cipher_hex, c0,02, 00,ff" "" "00,15,${extn_len_hex:0:2},${extn_len_hex:2:2}${extn}" + else + tls_sockets "$proto" "$selected_cipher_hex, c0,01, 00,ff" "" "00,15,${extn_len_hex:0:2},${extn_len_hex:2:2}${extn}" + fi + else + tls_sockets "$proto" "$cipher_list" "" "00,15,${extn_len_hex:0:2},${extn_len_hex:2:2}${extn}" + fi success=$? if [[ $success -ne 0 ]] && [[ $success -ne 2 ]]; then prln_svrty_medium " Server fails if ClientHello is $clienthello_len bytes in length."