From 9a86825ec23ccd6809e089bcb4b4f86b488e0e51 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 24 Mar 2017 11:37:06 -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 | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/testssl.sh b/testssl.sh index bd344b8..40078af 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3388,29 +3388,31 @@ 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 + 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 + 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