From 3451083bbd92277278daa2c070752d792e8742e1 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 24 Mar 2017 11:48:01 -0400 Subject: [PATCH] Fix client simulation In `create_client_simulation_tls_clienthello()` the variable `sni_extension_found` should be set if the ClientHello includes an SNI extension. Instead it was being set if and only if the ClientHello included some extension other than SNI. This bug wasn't detected before for two reasons: It is rare to have a ClientHello that includes an SNI extension, but no other extensions. The code still works correctly if `sni_extension_found` is set even if there is no SNI in the ClientHello. So, the bug only creates a problem if the browser's ClientHello include an SNI extension and no other extensions (see "BingPreview Jun 2014" in the client_simulation branch). --- testssl.sh | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/testssl.sh b/testssl.sh index d2edbca..a2e3471 100755 --- a/testssl.sh +++ b/testssl.sh @@ -2118,29 +2118,32 @@ create_client_simulation_tls_clienthello() { len_extensions=2*$(hex2dec "${tls_handshake_ascii:$offset:4}") offset=$offset+4 for (( 1; offset < tls_handshake_ascii_len; 1 )); do - extension_type="${tls_handshake_ascii:$offset:4}" - offset=$offset+4 - len_extension=2*$(hex2dec "${tls_handshake_ascii:$offset:4}") + extension_type="${tls_handshake_ascii:$offset:4}" + offset=$offset+4 + len_extension=2*$(hex2dec "${tls_handshake_ascii:$offset:4}") - if [[ "$extension_type" != "0000" ]]; then - # The extension will just be copied into the revised ClientHello - sni_extension_found=true - offset=$offset-4 - len=$len_extension+8 - tls_extensions+="${tls_handshake_ascii:$offset:$len}" - offset=$offset+$len - elif [[ -n "$SNI" ]]; then - # Create a server name extension that corresponds to $SNI - len_servername=${#NODE} - hexdump_format_str="$len_servername/1 \"%02x\"" - servername_hexstr=$(printf $NODE | hexdump -v -e "${hexdump_format_str}") - # convert lengths we need to fill in from dec to hex: - len_servername_hex=$(printf "%02x\n" $len_servername) - len_sni_listlen=$(printf "%02x\n" $((len_servername+3))) - len_sni_ext=$(printf "%02x\n" $((len_servername+5))) - tls_extensions+="000000${len_sni_ext}00${len_sni_listlen}0000${len_servername_hex}${servername_hexstr}" - offset=$offset+$len_extension+4 - fi + if [[ "$extension_type" != "0000" ]]; then + # The extension will just be copied into the revised ClientHello + sni_extension_found=true + offset=$offset-4 + len=$len_extension+8 + tls_extensions+="${tls_handshake_ascii:$offset:$len}" + offset=$offset+$len + else + sni_extension_found=true + if [[ -n "$SNI" ]]; then + # Create a server name extension that corresponds to $SNI + len_servername=${#NODE} + hexdump_format_str="$len_servername/1 \"%02x\"" + servername_hexstr=$(printf $NODE | hexdump -v -e "${hexdump_format_str}") + # convert lengths we need to fill in from dec to hex: + len_servername_hex=$(printf "%02x\n" $len_servername) + len_sni_listlen=$(printf "%02x\n" $((len_servername+3))) + len_sni_ext=$(printf "%02x\n" $((len_servername+5))) + tls_extensions+="000000${len_sni_ext}00${len_sni_listlen}0000${len_servername_hex}${servername_hexstr}" + offset=$offset+$len_extension+4 + fi + fi done if ! $sni_extension_found; then