Git/testssl.sh
1
0
Fork 0
mirror of https://github.com/drwetter/testssl.sh.git synced 2025-01-06 00:39:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix TLSv1.3 second ClientHello

Browse Source 
This PR fixes two problems with modify_clienthello().

First, the function was incorrectly using the variable $key_share instead of $new_key_share. Since $key_share is defined when modify_clienthello() is called from resend_if_hello_retry_request(), but not when it is called from client_simulation_sockets(), this bug does not seem to result in incorrect behavior, but it should still be fixed.

Second, when this function is used to create a second ClientHello in response to a HelloRetryRequest, it removes the key_share extension from the original ClientHello and then appends the new key_share extension at the end. According to https://mailarchive.ietf.org/arch/msg/tls/8ZKCyamcYFaV90h6nf4MUnSPkEE, however, extensions must appear in the second ClientHello in the same order in which they appeared in the first ClientHello.

I am not aware of any servers that will actually complain if the extensions in the second ClientHello do not appear in the same order as in the first ClientHello, bug this fix helps to ensure that the ClientHello messages testssl.sh sends are in compliance with the standard.
This commit is contained in:
David Cooper 2019-02-08 15:40:28 -05:00 committed by GitHub
parent b5dbe73a7e
commit a26aae381c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 8 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
testssl.sh
View File

@ -4301,7 +4301,7 @@ modify_clienthello() {
offset+=+4
len_extension=2*$(hex2dec "${tls_handshake_ascii:$offset:4}")
if [[ "$extension_type" == 0000 ]] && [[ -z "$key_share" ]]; then
if [[ "$extension_type" == 0000 ]] && [[ -z "$new_key_share" ]]; then
# If this is an initial ClientHello, then either remove
# the SNI extension or replace it with the correct server name.
sni_extension_found=true
@ -4317,7 +4317,7 @@ modify_clienthello() {
tls_extensions+="000000${len_sni_ext}00${len_sni_listlen}0000${len_servername_hex}${servername_hexstr}"
offset+=$len_extension+4
fi
elif [[ "$extension_type" != 00$KEY_SHARE_EXTN_NR ]] || [[ -z "$key_share" ]]; then
elif [[ "$extension_type" != 00$KEY_SHARE_EXTN_NR ]] || [[ -z "$new_key_share" ]]; then
# If this is in response to a HelloRetryRequest, then do
# not copy over the old key_share extension, but
# all other extensions should be copied into the new ClientHello.
@ -4326,12 +4326,16 @@ modify_clienthello() {
tls_extensions+="${tls_handshake_ascii:$offset:$len}"
offset+=$len
else
# This is the key_share extension, and the modified ClientHello
# is being created in response to a HelloRetryRequest. Replace
# the existing key_share extension with the new one.
tls_extensions+="$new_key_share"
offset+=$len_extension+4
fi
done
tls_extensions+="$new_key_share$cookie"
tls_extensions+="$cookie"
if ! "$sni_extension_found" && [[ -z "$key_share" ]]; then
if ! "$sni_extension_found" && [[ -z "$new_key_share" ]]; then
tm_out "$tls_handshake_ascii"
return 0
fi