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.
This commit is contained in:
David Cooper 2017-07-14 15:48:59 -04:00 committed by GitHub
parent 507e59dc97
commit bcd71555ea

View File

@ -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