From 4f04820c76a8c0a14564bcb02d51a71f86e8fb09 Mon Sep 17 00:00:00 2001 From: Daniel Reichelt Date: Thu, 22 Sep 2016 16:53:43 +0200 Subject: [PATCH 1/4] Fix handling of empty argument to "-nextprotoneg" parameter s_client's manpage states for -nextprotoneg: "Empty list of protocols is treated specially and will cause the client to advertise support for the TLS extension but disconnect just after reciving ServerHello with a list of server supported protocols." Consequently, the previous workaround of just quoting an empty variable is insufficient and the "-nextprotoneg" parameter has to be removed entirely from the command-line in case of an empty argument. In other locations where "-nextprotoneg" is used - its argument cannot be empty ($NPN_PROTOs is initialized to a non- empty value and set read-only) or - its argument is intended to be empty (line 3724) or - the command will not be invoked at all (for-loop parameter, line 3725) This fixes #467 - again. Additionally this patch prefers usage of -alpn over -nextprotoneg if the openssl binary used supports it. --- testssl.sh | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index d9a1679..2e12c5a 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3933,11 +3933,15 @@ sclient_connect_successful() { determine_tls_extensions() { local proto addcmd local success - local alpn="" + local alpnOrNpnParam="" local savedir local nrsaved - "$HAS_ALPN" && alpn="h2-14,h2-15,h2" + if "$HAS_ALPN"; then + alpnOrNpnParam="-alpn \"http/1.1,spdy/1,spdy/2,spdy/3,stun.turn,stun.nat-discovery,h2,h2c,webrtc,c-webrtc,ftp\"" + elif "$HAS_SPDY"; then + alpnOrNpnParam="-nextprotoneg \"h2-14,h2-15,h2\"" + fi if [[ -n "$2" ]]; then protocols_to_try="$2" @@ -3983,7 +3987,7 @@ determine_tls_extensions() { # alpn: echo | openssl s_client -connect google.com:443 -tlsextdebug -alpn h2-14 -servername google.com <-- suport needs to be checked b4 -- see also: ssl/t1_trce.c addcmd="" [[ ! "$proto" =~ ssl ]] && addcmd="$SNI" - $OPENSSL s_client $STARTTLS $BUGS $1 -showcerts -connect $NODEIP:$PORT $PROXY $addcmd -$proto -tlsextdebug -nextprotoneg "$alpn" -status $ERRFILE >$TMPFILE + $OPENSSL s_client $STARTTLS $BUGS $1 -showcerts -connect $NODEIP:$PORT $PROXY $addcmd -$proto -tlsextdebug $alpnOrNpnParam -status $ERRFILE >$TMPFILE sclient_connect_successful $? $TMPFILE && success=0 && break done # this loop is needed for IIS6 and others which have a handshake size limitations if [[ $success -eq 7 ]]; then From 566623c4a9a2a5ca6cde6d19d4a791975bcb6829 Mon Sep 17 00:00:00 2001 From: Weida Hong Date: Sat, 24 Sep 2016 15:10:10 +0800 Subject: [PATCH 2/4] Remove duplicated do_rc4 in debug_globals() --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 7df6836..0728ca4 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8351,7 +8351,7 @@ debug_globals() { local gbl for gbl in do_allciphers do_vulnerabilities do_beast do_breach do_ccs_injection do_cipher_per_proto do_crime \ - do_freak do_logjam do_drown do_header do_heartbleed do_rc4 do_mx_all_ips do_pfs do_protocols do_rc4 do_renego \ + do_freak do_logjam do_drown do_header do_heartbleed do_mx_all_ips do_pfs do_protocols do_rc4 do_renego \ do_std_cipherlists do_server_defaults do_server_preference do_spdy do_http2 do_ssl_poodle do_tls_fallback_scsv \ do_client_simulation do_test_just_one do_tls_sockets do_mass_testing do_display_only; do printf "%-22s = %s\n" $gbl "${!gbl}" From 0cadeefb0583a8ece31ccdb92787b0070614f012 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sat, 24 Sep 2016 16:07:23 +0200 Subject: [PATCH 3/4] cleanup #473 --- testssl.sh | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/testssl.sh b/testssl.sh index 6d38841..127a1cb 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3955,15 +3955,11 @@ sclient_connect_successful() { determine_tls_extensions() { local proto addcmd local success - local alpnOrNpnParam="" + local npn_params="" local savedir local nrsaved - if "$HAS_ALPN"; then - alpnOrNpnParam="-alpn \"http/1.1,spdy/1,spdy/2,spdy/3,stun.turn,stun.nat-discovery,h2,h2c,webrtc,c-webrtc,ftp\"" - elif "$HAS_SPDY"; then - alpnOrNpnParam="-nextprotoneg \"h2-14,h2-15,h2\"" - fi + $HAS_SPDY && npn_params="-nextprotoneg \"$NPN_PROTO\"" if [[ -n "$2" ]]; then protocols_to_try="$2" @@ -4009,7 +4005,7 @@ determine_tls_extensions() { # alpn: echo | openssl s_client -connect google.com:443 -tlsextdebug -alpn h2-14 -servername google.com <-- suport needs to be checked b4 -- see also: ssl/t1_trce.c addcmd="" [[ ! "$proto" =~ ssl ]] && addcmd="$SNI" - $OPENSSL s_client $STARTTLS $BUGS $1 -showcerts -connect $NODEIP:$PORT $PROXY $addcmd -$proto -tlsextdebug $alpnOrNpnParam -status $ERRFILE >$TMPFILE + $OPENSSL s_client $STARTTLS $BUGS $1 -showcerts -connect $NODEIP:$PORT $PROXY $addcmd -$proto -tlsextdebug $npn_params -status $ERRFILE >$TMPFILE sclient_connect_successful $? $TMPFILE && success=0 && break done # this loop is needed for IIS6 and others which have a handshake size limitations if [[ $success -eq 7 ]]; then @@ -8856,4 +8852,4 @@ fi exit $? -# $Id: testssl.sh,v 1.546 2016/09/21 19:59:48 dirkw Exp $ +# $Id: testssl.sh,v 1.547 2016/09/24 14:07:22 dirkw Exp $ From fcdc15b24bf7074539291f44cda848f9851ba98e Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sat, 24 Sep 2016 16:59:28 +0200 Subject: [PATCH 4/4] no STARTTLS for NPN, preparing #477 --- testssl.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/testssl.sh b/testssl.sh index 127a1cb..a3c6705 100755 --- a/testssl.sh +++ b/testssl.sh @@ -187,8 +187,9 @@ IKNOW_FNAME=false # further global vars just declared here readonly NPN_PROTOs="spdy/4a2,spdy/3,spdy/3.1,spdy/2,spdy/1,http/1.1" -# alpn_protos needs to be space-separated, not comma-seperated -readonly ALPN_PROTOs="h2 h2-17 h2-16 h2-15 h2-14 spdy/3.1 http/1.1" +# alpn_protos needs to be space-separated, not comma-seperated, including odd ones observerd @ facebook and others, old ones like h2-17 omitted as they could not be found +readonly ALPN_PROTOs="h2 spdy/3.1 http/1.1 h2-fb spdy/1 spdy/2 spdy/3 stun.turn stun.nat-discovery webrtc c-webrtc ftp" + TEMPDIR="" TMPFILE="" ERRFILE="" @@ -3565,6 +3566,7 @@ run_server_preference() { [[ -n "$PROXY" ]] && arg=" SPDY/NPN is" [[ -n "$STARTTLS" ]] && arg=" " if spdy_pre " $arg" ; then # is NPN/SPDY supported and is this no STARTTLS? / no PROXY + # ALPN needs also some lines here $OPENSSL s_client -connect $NODEIP:$PORT $BUGS -nextprotoneg "$NPN_PROTOs" $SNI >$ERRFILE >$TMPFILE if sclient_connect_successful $? $TMPFILE; then proto[i]=$(grep -aw "Next protocol" $TMPFILE | sed -e 's/^Next protocol://' -e 's/(.)//' -e 's/ //g') @@ -3955,11 +3957,12 @@ sclient_connect_successful() { determine_tls_extensions() { local proto addcmd local success - local npn_params="" + local npn_params="" alpn_params="" local savedir local nrsaved - $HAS_SPDY && npn_params="-nextprotoneg \"$NPN_PROTO\"" + $HAS_SPDY && [[ -z $STARTTLS ]] && npn_params="-nextprotoneg \"$NPN_PROTOs\"" + $HAS_ALPN && [[ -z $STARTTLS ]] && alpn_params="-alpn \"${ALPN_PROTOs// /,}\"" # we need to replace " " by "," if [[ -n "$2" ]]; then protocols_to_try="$2" @@ -8852,4 +8855,4 @@ fi exit $? -# $Id: testssl.sh,v 1.547 2016/09/24 14:07:22 dirkw Exp $ +# $Id: testssl.sh,v 1.548 2016/09/24 14:59:26 dirkw Exp $