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.
This commit is contained in:
David Cooper 2018-08-31 12:47:19 -04:00
parent f2303637b9
commit 25fef82977

View File

@ -11827,6 +11827,7 @@ socksend_tls_clienthello() {
# then add a padding extension (see RFC 7685) # then add a padding extension (see RFC 7685)
len_all=$((0x$len_ciph_suites + 0x2b + 0x$len_extension_hex + 0x2)) len_all=$((0x$len_ciph_suites + 0x2b + 0x$len_extension_hex + 0x2))
"$offer_compression" && len_all+=2 "$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 -ge 256 ]] && [[ $len_all -le 511 ]] && [[ ! "$extra_extensions_list" =~ " 0015 " ]]; then
if [[ $len_all -gt 508 ]]; then if [[ $len_all -gt 508 ]]; then
len_padding_extension=1 # Final extension cannot be empty: see PR #792 len_padding_extension=1 # Final extension cannot be empty: see PR #792
@ -11841,12 +11842,18 @@ socksend_tls_clienthello() {
done done
len_extension=$len_extension+$len_padding_extension+0x4 len_extension=$len_extension+$len_padding_extension+0x4
len_extension_hex=$(printf "%02x\n" $len_extension) 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 fi
len2twobytes "$len_extension_hex" len2twobytes "$len_extension_hex"
all_extensions=" all_extensions="
,$LEN_STR # first the len of all extensions. ,$LEN_STR # first the len of all extensions.
,$all_extensions" ,$all_extensions"
fi fi
if [[ 0x$tls_low_byte -gt 0x03 ]]; then 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 alpn_proto alpn alpn_list_len_hex extn_len_hex
local selected_alpn_protocol grease_selected_alpn_protocol local selected_alpn_protocol grease_selected_alpn_protocol
local ciph list temp curve_found 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 local -i ret=0
# Note: The following values were taken from https://datatracker.ietf.org/doc/draft-ietf-tls-grease. # 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. # 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
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 # Check that server ignores unrecognized cipher suite values
# see https://datatracker.ietf.org/doc/draft-ietf-tls-grease # see https://datatracker.ietf.org/doc/draft-ietf-tls-grease
if "$normal_hello_ok"; then if "$normal_hello_ok"; then