From 644c1aa83aebbd7663d21eb67a3dcecb69261022 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:15:44 -0700 Subject: [PATCH 01/23] Replace numeric ALLCIPHERS comparison with simpler -n test. --- cipherscan | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cipherscan b/cipherscan index 45da476..c2c5ae5 100755 --- a/cipherscan +++ b/cipherscan @@ -176,7 +176,7 @@ FALLBACKCIPHERSUITE=( DEBUG=0 VERBOSE=0 DELAY=0 -ALLCIPHERS=0 +ALLCIPHERS="" OUTPUTFORMAT="terminal" TIMEOUT=30 # place where to put the found intermediate CA certificates and where @@ -1505,7 +1505,7 @@ else fi # If asked, test every single cipher individually -if [[ $ALLCIPHERS -gt 0 ]]; then +if [[ -n $ALLCIPHERS ]]; then echo; echo "All accepted ciphersuites" for c in $($OPENSSLBIN ciphers -v ALL:COMPLEMENTOFALL 2>/dev/null |awk '{print $1}'|sort|uniq); do r="fail" From fc71ed72048d328f85b883e2e0096fe019728a12 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:16:10 -0700 Subject: [PATCH 02/23] Replace |sort|uniq with more efficient |sort -u. --- cipherscan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index c2c5ae5..75af88c 100755 --- a/cipherscan +++ b/cipherscan @@ -1507,7 +1507,7 @@ fi # If asked, test every single cipher individually if [[ -n $ALLCIPHERS ]]; then echo; echo "All accepted ciphersuites" - for c in $($OPENSSLBIN ciphers -v ALL:COMPLEMENTOFALL 2>/dev/null |awk '{print $1}'|sort|uniq); do + for c in $($OPENSSLBIN ciphers -v ALL:COMPLEMENTOFALL 2>/dev/null |awk '{print $1}'|sort -u); do r="fail" osslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client $SCLIENTARGS -connect $TARGET -cipher $c" test_cipher_on_target "$osslcommand" From 9e3154389e3dc1d56a56eda2a69d04019dd546d6 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:17:49 -0700 Subject: [PATCH 03/23] Replace unnecessary test of command; if $? with if command. --- cipherscan | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cipherscan b/cipherscan index 75af88c..4f467d6 100755 --- a/cipherscan +++ b/cipherscan @@ -651,10 +651,8 @@ get_cipher_pref() { sslcommand+=" -status $SCLIENTARGS -connect $TARGET -cipher $ciphersuite" verbose "Connecting to '$TARGET' with ciphersuite '$ciphersuite'" - test_cipher_on_target "$sslcommand" - local success=$? # If the connection succeeded with the current cipher, benchmark and store - if [[ $success -eq 0 ]]; then + if test_cipher_on_target "$sslcommand"; then cipherspref=("${cipherspref[@]}" "$result") ciphercertificates=("${ciphercertificates[@]}" "$certificates") pciph=($result) @@ -1510,8 +1508,7 @@ if [[ -n $ALLCIPHERS ]]; then for c in $($OPENSSLBIN ciphers -v ALL:COMPLEMENTOFALL 2>/dev/null |awk '{print $1}'|sort -u); do r="fail" osslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client $SCLIENTARGS -connect $TARGET -cipher $c" - test_cipher_on_target "$osslcommand" - if [[ $? -eq 0 ]]; then + if test_cipher_on_target "$osslcommand"; then r="pass" fi echo "$c $r"|awk '{printf "%-35s %s\n",$1,$2}' From a342ff7579857837cf319f8cbbbdbb60d4354af9 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:19:18 -0700 Subject: [PATCH 04/23] Assign r=pass/fail once only, rather than twice for fail->pass. --- cipherscan | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index 4f467d6..019cca8 100755 --- a/cipherscan +++ b/cipherscan @@ -1506,10 +1506,11 @@ fi if [[ -n $ALLCIPHERS ]]; then echo; echo "All accepted ciphersuites" for c in $($OPENSSLBIN ciphers -v ALL:COMPLEMENTOFALL 2>/dev/null |awk '{print $1}'|sort -u); do - r="fail" osslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client $SCLIENTARGS -connect $TARGET -cipher $c" if test_cipher_on_target "$osslcommand"; then r="pass" + else + r="fail" fi echo "$c $r"|awk '{printf "%-35s %s\n",$1,$2}' done From 487f7cb6a47fbf7620e561a1151177d1c0edcd17 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:20:39 -0700 Subject: [PATCH 05/23] Replace an echo | awk printf with builtin printf. --- cipherscan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index 019cca8..c5250e6 100755 --- a/cipherscan +++ b/cipherscan @@ -1512,6 +1512,6 @@ if [[ -n $ALLCIPHERS ]]; then else r="fail" fi - echo "$c $r"|awk '{printf "%-35s %s\n",$1,$2}' + printf "%-35s %s\n" "$c" "$r" done fi From 2764a16693e887c5f0a4a39abdb945249464c0bc Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:46:04 -0700 Subject: [PATCH 06/23] Replace OLDIFS/IFS joins with join_array_by_char(), avoiding $(...) subshell slowdown. --- cipherscan | 56 ++++++++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/cipherscan b/cipherscan index c5250e6..2f07570 100755 --- a/cipherscan +++ b/cipherscan @@ -173,6 +173,19 @@ FALLBACKCIPHERSUITE=( 'EXP-RC2-CBC-MD5' 'EXP-RC4-MD5' ) + +join_array_by_char() { + # Two or less parameters (join + 0 or 1 value), then no need to set IFS because no join occurs. + if (( $# >= 3 )); then + # Three or more parameters (join + 2 values), then we need to set IFS for the join. + local IFS=$1 + fi + # Discard the join string (usually ':', could be others). + shift + # Store the joined string in the result. + joined_array="$*" +} + DEBUG=0 VERBOSE=0 DELAY=0 @@ -436,10 +449,8 @@ parse_openssl_output() { local match=($data) unset match[0] unset match[1] - local old_IFS="$IFS" - IFS="_" - current_sigalg="${match[*]}" - IFS="$old_IFS" + join_array_by_char '_' "${match[@]}" + current_sigalg="$joined_array" fi done <<<"$ossl_out" fi @@ -896,10 +907,8 @@ test_curves() { local curves=(${CURVES[*]}) - OLDIFS="$IFS" - IFS=':' - verbose "Will test following curves: ${curves[*]}" - IFS="$OLDIFS" + join_array_by_char ':' "${curves[@]}" + verbose "Will test following curves: $joined_array" # prepare the ssl command we'll be using local sslcommand="" @@ -921,10 +930,8 @@ test_curves() { # tries to negotiate a curve we didn't advertise # while [[ ${#curves[@]} -gt 0 ]]; do - OLDIFS="$IFS" - IFS=':' - local test_curves="${curves[*]}" - IFS="$OLDIFS" + join_array_by_char ':' "${curves[@]}" + local test_curves="$joined_array" verbose "Testing $test_curves with command $sslcommand" ratelimit @@ -1043,10 +1050,8 @@ test_curves_fallback() { # local curves=(${CURVES[*]}) while [[ ${#curves[@]} -gt 0 ]]; do - OLDIFS="$IFS" - IFS=':' - local test_curves="${curves[*]}" - IFS="$OLDIFS" + join_array_by_char ':' "${curves[@]}" + local test_curves="$joined_array" verbose "Testing $sslcommand -curves $test_curves" ratelimit @@ -1141,10 +1146,8 @@ test_tls_tolerance() { # # try a smaller, but still v2 compatible Client Hello # - OLDIFS="$IFS" - IFS=":" - local ciphers="${SHORTCIPHERSUITE[*]}" - IFS="$OLDIFS" + join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" + local ciphers="$joined_array" local sslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client" if [[ -n "$CAPATH" ]]; then @@ -1215,10 +1218,8 @@ test_tls_tolerance() { # # use v3 format TLSv1.2 hello, small cipher list # - OLDIFS="$IFS" - IFS=":" - local ciphers="${SHORTCIPHERSUITE[*]}" - IFS="$OLDIFS" + join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" + local ciphers="$joined_array" local sslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client" if [[ -n "$CAPATH" ]]; then @@ -1480,11 +1481,8 @@ if [[ ${#cipherspref[@]} -eq 0 ]] || [[ ${pref[1]} == "SSLv2" ]]; then cipherspref=() ciphercertificates=() results=() - OLDIFS="$IFS" - IFS=":" - CIPHERS="${FALLBACKCIPHERSUITE[*]}" - IFS="$OLDIFS" - get_cipher_pref "$CIPHERS" + join_array_by_char ':' "${FALLBACKCIPHERSUITE[@]}" + get_cipher_pref "$joined_array" fi test_tls_tolerance From 871ad92ae2c68af33d8df538b9eb62441b963084 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 03:13:46 -0700 Subject: [PATCH 07/23] Simplify signature algorithm extraction to use a capturing regex and string substitution. --- cipherscan | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cipherscan b/cipherscan index 2f07570..e195d21 100755 --- a/cipherscan +++ b/cipherscan @@ -444,13 +444,10 @@ parse_openssl_output() { # the signature algorithm on it (it's the server's certificate) if [[ $certs_found -gt 0 ]]; then local ossl_out=$(${OPENSSLBIN} x509 -noout -text 2>/dev/null <<<"${current_raw_certificates[0]}") + local regex='Signature Algorithm[^ ]+ +(.+$)' while read data; do - if [[ $data =~ Signature\ Algorithm ]]; then - local match=($data) - unset match[0] - unset match[1] - join_array_by_char '_' "${match[@]}" - current_sigalg="$joined_array" + if [[ $data =~ $regex ]]; then + current_sigalg="${BASH_REMATCH[1]// /_}" fi done <<<"$ossl_out" fi From 90ac19cfe807697656bb1cae076e3754c8ad7c09 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 03:27:00 -0700 Subject: [PATCH 08/23] Replace an instance of string-ish [[ $? -gt 0 ]] with arithmetic (( $? != 0 )). This more accurately reflects that "non-zero exit status indicates failure"; while > 0 will no doubt work as well, != 0 avoids the question of whether $? is signed or unsigned in bash and more accurately represents the documentation ("non-zero", != 0). --- cipherscan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index e195d21..1b1126a 100755 --- a/cipherscan +++ b/cipherscan @@ -633,7 +633,7 @@ bench_cipher() { for i in $(seq 1 $BENCHMARKITER); do debug Connection $i (echo "Q" | $sslcommand 2>/dev/null 1>/dev/null) - if [[ $? -gt 0 ]]; then + if (( $? != 0 )); then break fi done From 9c63841e4614f25d82d4a49592563adfcd011aae Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 02:23:22 -0700 Subject: [PATCH 09/23] Replace instances of string-ish [[ -eq ]] with arithmetic (( == )). --- cipherscan | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cipherscan b/cipherscan index 1b1126a..e85daf8 100755 --- a/cipherscan +++ b/cipherscan @@ -850,7 +850,7 @@ test_serverside_ordering() { serverside="True" return 0 # server supports just two ciphers, so rotate them, that should be enough - elif [[ ${#cipherspref[@]} -eq 2 ]]; then + elif (( ${#cipherspref[@]} == 2 )); then local cipher=(${cipherspref[1]}) prefered="$cipher" @@ -1303,7 +1303,7 @@ test_tls_tolerance() { } # If no options are given, give usage information and exit (with error code) -if [[ $# -eq 0 ]]; then +if (( $# == 0 )); then usage exit 1 fi @@ -1474,7 +1474,7 @@ get_cipher_pref $CIPHERSUITE # do that either when the normal scan returns no ciphers or just SSLv2 # ciphers (where it's likely that the limiting by OpenSSL worked) pref=(${cipherspref[0]}) -if [[ ${#cipherspref[@]} -eq 0 ]] || [[ ${pref[1]} == "SSLv2" ]]; then +if (( ${#cipherspref[@]} == 0 )) || [[ ${pref[1]} == "SSLv2" ]]; then cipherspref=() ciphercertificates=() results=() From 3d3789828b0c1caf1d50ca5b2cf32cabe7bfb5b4 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 03:03:02 -0700 Subject: [PATCH 10/23] Replace instances of string-ish [[ -gt ]] with arithmetic (( > )). --- cipherscan | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cipherscan b/cipherscan index e85daf8..0711ceb 100755 --- a/cipherscan +++ b/cipherscan @@ -442,7 +442,7 @@ parse_openssl_output() { # if we found any certs in output, process the first one and extract # the signature algorithm on it (it's the server's certificate) - if [[ $certs_found -gt 0 ]]; then + if (( certs_found > 0 )); then local ossl_out=$(${OPENSSLBIN} x509 -noout -text 2>/dev/null <<<"${current_raw_certificates[0]}") local regex='Signature Algorithm[^ ]+ +(.+$)' while read data; do @@ -795,7 +795,7 @@ display_results_in_json() { echo -n "{\"target\":\"$TARGET\",\"utctimestamp\":\"$(date -u '+%FT%T.0Z')\",\"serverside\":\"${serverside}\",\"ciphersuite\": [" for cipher in "${cipherspref[@]}"; do local cipher_arr=($cipher) - [[ $ctr -gt 0 ]] && echo -n ',' + (( ctr > 0 )) && echo -n ',' echo -n "{\"cipher\":\"${cipher_arr[0]}\"," echo -n "\"protocols\":[\"${cipher_arr[1]//,/\",\"}\"]," echo -n "\"pubkey\":[\"${cipher_arr[2]//,/\",\"}\"]," @@ -828,7 +828,7 @@ display_results_in_json() { ctr=0 for test_name in "${!tls_tolerance[@]}"; do local result=(${tls_tolerance[$test_name]}) - [[ $ctr -gt 0 ]] && echo -n "," + (( ctr > 0 )) && echo -n "," echo -n "\"$test_name\":{" if [[ ${result[0]} == "False" ]]; then echo -n "\"tolerant\":\"False\"" @@ -926,7 +926,7 @@ test_curves() { # either get a fallback to a non ECC cipher, we run of curves or server # tries to negotiate a curve we didn't advertise # - while [[ ${#curves[@]} -gt 0 ]]; do + while (( ${#curves[@]} > 0 )); do join_array_by_char ':' "${curves[@]}" local test_curves="$joined_array" verbose "Testing $test_curves with command $sslcommand" @@ -1046,7 +1046,7 @@ test_curves_fallback() { # tries to negotiate a curve we didn't advertise # local curves=(${CURVES[*]}) - while [[ ${#curves[@]} -gt 0 ]]; do + while (( ${#curves[@]} > 0 )); do join_array_by_char ':' "${curves[@]}" local test_curves="$joined_array" verbose "Testing $sslcommand -curves $test_curves" From 34ae0ccab9dd831a043d23767870ce484c17b29b Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 03:26:34 -0700 Subject: [PATCH 11/23] Replace instances of string-ish [[ -ne ]] with arithmetic (( != )). --- cipherscan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index 0711ceb..c8f5224 100755 --- a/cipherscan +++ b/cipherscan @@ -883,7 +883,7 @@ test_serverside_ordering() { sslcommand+=" -status $SCLIENTARGS -connect $TARGET -cipher $ciphersuite" test_cipher_on_target "$sslcommand" - if [[ $? -ne 0 ]]; then + if (( $? != 0 )); then serverside="True" else local selected=($result) From b91b153bbdf11e9aa06cfc23862e32a9be6dded7 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 03:36:40 -0700 Subject: [PATCH 12/23] Replace instances of string-ish [[ -lt ]] with arithmetic (( < )). --- cipherscan | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cipherscan b/cipherscan index c8f5224..ec472e1 100755 --- a/cipherscan +++ b/cipherscan @@ -846,7 +846,7 @@ test_serverside_ordering() { local ciphersuite="" local prefered="" # server supports only one cipher or no ciphers, so it effectively uses server side ordering... - if [[ ${#cipherspref[@]} -lt 2 ]]; then + if (( ${#cipherspref[@]} < 2 )); then serverside="True" return 0 # server supports just two ciphers, so rotate them, that should be enough @@ -974,7 +974,7 @@ test_curves() { # server supports just one or none, so it effectively uses server side # ordering (as it dictates what curves client must support) - if [[ ${#tmp_curves[@]} -lt 2 ]]; then + if (( ${#tmp_curves[@]} < 2 )); then curves_ordering="server" else # server supports at least 2 curves, rotate their order, see if From d2e1784eb8ec0370e8cd5dcb01d870cc5a0e6d1a Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:01:36 -0700 Subject: [PATCH 13/23] Simplify test_serverside_ordering() to use half as many assignments. --- cipherscan | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/cipherscan b/cipherscan index ec472e1..70d1ed0 100755 --- a/cipherscan +++ b/cipherscan @@ -843,36 +843,29 @@ display_results_in_json() { } test_serverside_ordering() { + local -a ciphersuites=() local ciphersuite="" local prefered="" # server supports only one cipher or no ciphers, so it effectively uses server side ordering... if (( ${#cipherspref[@]} < 2 )); then serverside="True" return 0 - # server supports just two ciphers, so rotate them, that should be enough - elif (( ${#cipherspref[@]} == 2 )); then - - local cipher=(${cipherspref[1]}) - prefered="$cipher" - ciphersuite=$cipher - - cipher=(${cipherspref[0]}) - ciphersuite+=":$cipher" - - # server supports 3 or more ciphers, rotate all three. This is necessary because google does - # select first client provided cipher, if it is either CDHE-RSA-AES128-GCM-SHA256 or - # ECDHE-RSA-CHACHA20-POLY1305 - else - local cipher=(${cipherspref[2]}) - prefered="$cipher" - ciphersuite="$cipher" - - cipher=(${cipherspref[1]}) - ciphersuite+=":$cipher" - - cipher=(${cipherspref[0]}) - ciphersuite+=":$cipher" fi + local cipher="" + if (( ${#cipherspref[@]} > 2 )); then + # server supports 3 or more ciphers, rotate all three. This is necessary because google does + # select first client provided cipher, if it is either CDHE-RSA-AES128-GCM-SHA256 or + # ECDHE-RSA-CHACHA20-POLY1305 + ciphersuites+=("${cipherspref[2]%% *}") + fi + # else, server supports just two ciphers, so rotate them, that should be enough + ciphersuites+=("${cipherspref[1]%% *}") + ciphersuites+=("${cipherspref[0]%% *}") + + prefered="${ciphersuites[0]%% *}" + + join_array_by_char ':' "${ciphersuites[@]}" + ciphersuite="$joined_array" local sslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client" if [[ -n "$CAPATH" ]]; then From 9ea1749f6cd0a46939994724fc3b62f003e95ef8 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:04:46 -0700 Subject: [PATCH 14/23] Pre-cache the cipher array-to-string result to do one less join. --- cipherscan | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/cipherscan b/cipherscan index 70d1ed0..629c385 100755 --- a/cipherscan +++ b/cipherscan @@ -104,6 +104,18 @@ if [[ -e $DIRNAMEPATH/openssl.cnf ]]; then export OPENSSL_CONF="$DIRNAMEPATH/openssl.cnf" fi +join_array_by_char() { + # Two or less parameters (join + 0 or 1 value), then no need to set IFS because no join occurs. + if (( $# >= 3 )); then + # Three or more parameters (join + 2 values), then we need to set IFS for the join. + local IFS=$1 + fi + # Discard the join string (usually ':', could be others). + shift + # Store the joined string in the result. + joined_array="$*" +} + # RSA ciphers are put at the end to force Google servers to accept ECDSA ciphers # (probably a result of a workaround for the bug in Apple implementation of ECDSA) CIPHERSUITE="ALL:COMPLEMENTOFALL:+aRSA" @@ -135,6 +147,9 @@ SHORTCIPHERSUITE=( 'RC4-SHA' 'RC4-MD5' ) +join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" +SHORTCIPHERSUITESTRING="$joined_array" + # as some servers are intolerant to large client hello's (or ones that have # RC4 ciphers below position 64), use the following for cipher testing in case # of problems @@ -173,18 +188,8 @@ FALLBACKCIPHERSUITE=( 'EXP-RC2-CBC-MD5' 'EXP-RC4-MD5' ) - -join_array_by_char() { - # Two or less parameters (join + 0 or 1 value), then no need to set IFS because no join occurs. - if (( $# >= 3 )); then - # Three or more parameters (join + 2 values), then we need to set IFS for the join. - local IFS=$1 - fi - # Discard the join string (usually ':', could be others). - shift - # Store the joined string in the result. - joined_array="$*" -} +join_array_by_char ':' "${FALLBACKCIPHERSUITE[@]}" +FALLBACKCIPHERSUITESTRING="$joined_array" DEBUG=0 VERBOSE=0 @@ -353,6 +358,9 @@ check_option_support() { [[ $OPENSSLBINHELP =~ "$1" ]] } +# We stop processing certificates on each connection once any of them produces a set of valid certificates. +current_sigalg="None" + parse_openssl_output() { # clear variables in case matching doesn't hit them current_ocspstaple="False" @@ -362,7 +370,6 @@ parse_openssl_output() { current_tickethint="None" current_pubkey=0 current_trusted="False" - current_sigalg="None" certs_found=0 current_raw_certificates=() @@ -427,7 +434,7 @@ parse_openssl_output() { fi # extract certificates - if [[ $line =~ -----BEGIN\ CERTIFICATE----- ]]; then + if [[ $current_sigalg == 'None' && $line =~ -----BEGIN\ CERTIFICATE----- ]]; then current_raw_certificates[$certs_found]="$line"$'\n' while read data; do current_raw_certificates[$certs_found]+="$data"$'\n' @@ -1136,8 +1143,7 @@ test_tls_tolerance() { # # try a smaller, but still v2 compatible Client Hello # - join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" - local ciphers="$joined_array" + local ciphers="$SHORTCIPHERSUITESTRING" local sslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client" if [[ -n "$CAPATH" ]]; then @@ -1208,8 +1214,7 @@ test_tls_tolerance() { # # use v3 format TLSv1.2 hello, small cipher list # - join_array_by_char ':' "${SHORTCIPHERSUITE[@]}" - local ciphers="$joined_array" + local ciphers="$SHORTCIPHERSUITESTRING" local sslcommand="$TIMEOUTBIN $TIMEOUT $OPENSSLBIN s_client" if [[ -n "$CAPATH" ]]; then @@ -1471,8 +1476,7 @@ if (( ${#cipherspref[@]} == 0 )) || [[ ${pref[1]} == "SSLv2" ]]; then cipherspref=() ciphercertificates=() results=() - join_array_by_char ':' "${FALLBACKCIPHERSUITE[@]}" - get_cipher_pref "$joined_array" + get_cipher_pref "$FALLBACKCIPHERSUITESTRING" fi test_tls_tolerance From 5c09af67fd0012e70ffbff9e6e48e466f5d1faff Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:10:58 -0700 Subject: [PATCH 15/23] Remove one unnecessary string-to-array-to-string from get_curve_name(). --- cipherscan | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cipherscan b/cipherscan index 629c385..6262eaf 100755 --- a/cipherscan +++ b/cipherscan @@ -324,8 +324,7 @@ get_curve_name() { for c in "${CURVES_MAP[@]}"; do if [[ "$c" =~ $identifier ]]; then verbose "$c matches identifier $identifier" - local map=(${c// / }) - echo ${map[0]} + echo "${c%% *}" return fi done From c103805a384e83dbc611bd1233e7dda8897e941a Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:17:47 -0700 Subject: [PATCH 16/23] Replace instances of [[ $ != "" ]] with [[ -n "" ]]. --- cipherscan | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cipherscan b/cipherscan index 6262eaf..0fd83cc 100755 --- a/cipherscan +++ b/cipherscan @@ -615,7 +615,7 @@ test_cipher_on_target() { has_curves="True" if [[ $TEST_CURVES == "True" ]]; then test_curves - if [[ "$ecc_ciphers" != "" ]]; then + if [[ -n $ecc_ciphers ]]; then ecc_ciphers+=":" fi ecc_ciphers+="$cipher" @@ -702,7 +702,7 @@ display_results_in_terminal() { trusted="${cipher_data[4]}" tickethint="${cipher_data[5]}" ocspstaple="${cipher_data[6]}" - if [[ $TEST_CURVES == "True" && "${cipher_data[9]}" != "" ]]; then + if [[ $TEST_CURVES == "True" && -n ${cipher_data[9]} ]]; then curvesordering="${cipher_data[9]}" fi else @@ -721,10 +721,10 @@ display_results_in_terminal() { if [[ "$ocspstaple" != "${cipher_data[6]}" ]]; then different=True fi - if [[ "$curvesordering" == "" && "${cipher_data[9]}" != "" ]]; then + if [[ "$curvesordering" == "" && -n "${cipher_data[9]}" ]]; then curvesordering="${cipher_data[9]}" fi - if [[ "$curvesordering" != "" && "$curvesordering" != "${cipher_data[9]}" ]]; then + if [[ -n $curvesordering && "$curvesordering" != "${cipher_data[9]}" ]]; then different=True fi fi @@ -941,7 +941,7 @@ test_curves() { local ephem_data=(${current_pfs//,/ }) local cname="" if [[ ${ephem_data[0]} =~ ECDH ]]; then - if [[ "$current_curves" != "" ]]; then + if [[ -n $current_curves ]]; then current_curves+="," fi cname="$(get_curve_name ${ephem_data[1]})" From bc79c510654b7fb3c22d0a31686dc6be23acc149 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:25:55 -0700 Subject: [PATCH 17/23] Fixes instances of SC2086, SC2046 errors regarding unquoted variables. In cipherscan line 294: echo $identifier ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 587: current_curves="$(get_curve_name $(echo $pfs|cut -d ',' -f2))" ^-- SC2046: Quote this to prevent word splitting. In cipherscan line 603: debug Connection $i ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 715: echo $header ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 719: echo $result|grep -v '(NONE)' ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 897: local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 910: cname="$(get_curve_name ${ephem_data[1]})" ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 953: local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 967: local cname="$(get_curve_name ${ephem_data[1]})" ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 1017: local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) ^-- SC2086: Double quote to prevent globbing and word splitting. In cipherscan line 1030: local cname="$(get_curve_name ${ephem_data[1]})" ^-- SC2086: Double quote to prevent globbing and word splitting. --- cipherscan | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cipherscan b/cipherscan index 0fd83cc..1c89996 100755 --- a/cipherscan +++ b/cipherscan @@ -328,7 +328,7 @@ get_curve_name() { return fi done - echo $identifier + echo "$identifier" return } @@ -621,7 +621,7 @@ test_cipher_on_target() { ecc_ciphers+="$cipher" else # resolve the openssl curve to the proper IANA name - current_curves="$(get_curve_name $(echo $pfs|cut -d ',' -f2))" + current_curves="$(get_curve_name "$(echo $pfs|cut -d ',' -f2)")" fi fi result="$cipher $protocols $pubkey $sigalg $trusted $tickethint $ocspstaple $pfs $current_curves $curves_ordering" @@ -637,7 +637,7 @@ bench_cipher() { local t="$(date +%s%N)" verbose "Benchmarking handshake on '$TARGET' with ciphersuite '$ciphersuite'" for i in $(seq 1 $BENCHMARKITER); do - debug Connection $i + debug "Connection $i" (echo "Q" | $sslcommand 2>/dev/null 1>/dev/null) if (( $? != 0 )); then break @@ -749,11 +749,11 @@ display_results_in_terminal() { ctr=0 for result in "${results[@]}"; do if [[ $ctr -eq 0 ]]; then - echo $header + echo "$header" ctr=$((ctr+1)) fi if [[ $different == "True" ]]; then - echo $result|grep -v '(NONE)' + echo "$result"|grep -v '(NONE)' else # prints priority, ciphersuite, protocols and pfs awk '!/(NONE)/{print $1 " " $2 " " $3 " " $9 " " $10}' <<<"$result" @@ -931,7 +931,7 @@ test_curves() { verbose "Testing $test_curves with command $sslcommand" ratelimit - local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) + local tmp=$(echo Q | $sslcommand -curves "$test_curves" 2>/dev/null) parse_openssl_output <<<"$tmp" if [[ -z $current_protocol || $current_cipher == "(NONE)" || $current_cipher == '0000' ]]; then @@ -944,7 +944,7 @@ test_curves() { if [[ -n $current_curves ]]; then current_curves+="," fi - cname="$(get_curve_name ${ephem_data[1]})" + cname="$(get_curve_name "${ephem_data[1]}")" verbose "Server selected ${ephem_data[1]}, a.k.a $cname" current_curves+="$cname" fi @@ -987,7 +987,7 @@ test_curves() { verbose "Testing ordering with $sslcommand -curves $test_curves" ratelimit - local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) + local tmp=$(echo Q | $sslcommand -curves "$test_curves" 2>/dev/null) parse_openssl_output <<<"$tmp" if [[ -z $current_protocol || $current_cipher == "(NONE)" || $current_cipher == '0000' ]]; then @@ -1001,7 +1001,7 @@ test_curves() { if [[ ${ephem_data[0]} =~ ECDH ]]; then verbose "Server did select ${ephem_data[1]} curve" curves_ordering="inconclusive-${ephem_data[1]}" - local cname="$(get_curve_name ${ephem_data[1]})" + local cname="$(get_curve_name "${ephem_data[1]}")" if [[ "$cname" == "$most_wanted" ]]; then curves_ordering="client" else @@ -1051,7 +1051,7 @@ test_curves_fallback() { verbose "Testing $sslcommand -curves $test_curves" ratelimit - local tmp=$(echo Q | $sslcommand -curves $test_curves 2>/dev/null) + local tmp=$(echo Q | $sslcommand -curves "$test_curves" 2>/dev/null) parse_openssl_output <<<"$tmp" if [[ -z $current_protocol || $current_cipher == "(NONE)" || $current_cipher == '0000' ]]; then @@ -1064,7 +1064,7 @@ test_curves_fallback() { if [[ ${ephem_data[0]} =~ ECDH ]]; then # we got an ecc connection, remove the curve from the list of testable curves - local cname="$(get_curve_name ${ephem_data[1]})" + local cname="$(get_curve_name "${ephem_data[1]}")" verbose "Server selected curve $cname" for id in "${!curves[@]}"; do if [[ "${curves[id]}" == "$cname" ]]; then From 24268e063e7642f3e5a2bb2cfa08b72aa7083ff5 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:33:06 -0700 Subject: [PATCH 18/23] Fixes one instance of "SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate." In cipherscan line 427: local sslcommand=$@ ^-- SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate. --- cipherscan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cipherscan b/cipherscan index 1c89996..62c0281 100755 --- a/cipherscan +++ b/cipherscan @@ -461,7 +461,7 @@ parse_openssl_output() { # Connect to a target host with the selected ciphersuite test_cipher_on_target() { - local sslcommand=$@ + local sslcommand="$*" cipher="" local cmnd="" protocols="" From b2521c8e42a5d8c2242d0ca11fdd865a3a09991e Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:35:58 -0700 Subject: [PATCH 19/23] Fixes instances of "SC2053: Quote the rhs of == in [[ ]] to prevent glob matching." In cipherscan line 469: if [[ ${known_certs[$cksum]} == $cert ]]; then ^-- SC2053: Quote the rhs of == in [[ ]] to prevent glob matching. In cipherscan line 852: if [[ $selected == $prefered ]]; then ^-- SC2053: Quote the rhs of == in [[ ]] to prevent glob matching. In cipherscan line 915: if [[ "$cname" == ${curves[$id]} ]]; then ^-- SC2053: Quote the rhs of == in [[ ]] to prevent glob matching. --- cipherscan | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cipherscan b/cipherscan index 62c0281..71bab44 100755 --- a/cipherscan +++ b/cipherscan @@ -503,7 +503,7 @@ test_cipher_on_target() { local cksum=($(cksum <<<"$cert")) # compare the values not just checksums so that eventual collision # doesn't mess up results - if [[ ${known_certs[$cksum]} == $cert ]]; then + if [[ ${known_certs[$cksum]} == "$cert" ]]; then if [[ -n "${current_certificates}" ]]; then current_certificates+="," fi @@ -886,7 +886,7 @@ test_serverside_ordering() { serverside="True" else local selected=($result) - if [[ $selected == $prefered ]]; then + if [[ $selected == "$prefered" ]]; then serverside="False" else serverside="True" @@ -949,7 +949,7 @@ test_curves() { current_curves+="$cname" fi for id in "${!curves[@]}"; do - if [[ "$cname" == ${curves[$id]} ]]; then + if [[ $cname == "${curves[$id]}" ]]; then # we know it's supported, remove it from set of offered ones unset curves[$id] break From 236b0b8cfe4a9f3b8d3ca394b0d7bef29639a3ca Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Sat, 5 Sep 2015 04:40:07 -0700 Subject: [PATCH 20/23] Fixes instances of "SC2128: Expanding an array without an index only gives the first element.". In cipherscan line 851: local selected=($result) ^-- SC2128: Expanding an array without an index only gives the first element. In cipherscan line 852: if [[ $selected == "$prefered" ]]; then ^-- SC2128: Expanding an array without an index only gives the first element. --- cipherscan | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cipherscan b/cipherscan index 71bab44..fc1c986 100755 --- a/cipherscan +++ b/cipherscan @@ -885,8 +885,7 @@ test_serverside_ordering() { if (( $? != 0 )); then serverside="True" else - local selected=($result) - if [[ $selected == "$prefered" ]]; then + if [[ ${result%% *} == "$prefered" ]]; then serverside="False" else serverside="True" From ce2f97f05c7b75ba057579ef17db870e0581bc09 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Fri, 18 Sep 2015 11:39:29 -0700 Subject: [PATCH 21/23] Replace instances of [[ $ == "" ]] with [[ -z "" ]]. --- cipherscan | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cipherscan b/cipherscan index fc1c986..642b6f8 100755 --- a/cipherscan +++ b/cipherscan @@ -49,9 +49,9 @@ else # test that readlink or greadlink (darwin) are present READLINKBIN="$(which readlink)" - if [[ "$READLINKBIN" == "" ]]; then + if [[ -z $READLINKBIN ]]; then READLINKBIN="$(which greadlink)" - if [[ "$READLINKBIN" == "" ]]; then + if [[ -z $READLINKBIN ]]; then echo "neither readlink nor greadlink are present. install coreutils with {apt-get,yum,brew} install coreutils" 1>&2 exit 1 fi @@ -60,9 +60,9 @@ else # test that timeout or gtimeout (darwin) are present TIMEOUTBIN="$(which timeout)" - if [[ "$TIMEOUTBIN" == "" ]]; then + if [[ -z $TIMEOUTBIN ]]; then TIMEOUTBIN="$(which gtimeout)" - if [[ "$TIMEOUTBIN" == "" ]]; then + if [[ -z $TIMEOUTBIN ]]; then echo "neither timeout nor gtimeout are present. install coreutils with {apt-get,yum,brew} install coreutils" 1>&2 exit 1 fi @@ -721,7 +721,7 @@ display_results_in_terminal() { if [[ "$ocspstaple" != "${cipher_data[6]}" ]]; then different=True fi - if [[ "$curvesordering" == "" && -n "${cipher_data[9]}" ]]; then + if [[ -z $curvesordering && -n "${cipher_data[9]}" ]]; then curvesordering="${cipher_data[9]}" fi if [[ -n $curvesordering && "$curvesordering" != "${cipher_data[9]}" ]]; then @@ -813,7 +813,7 @@ display_results_in_json() { echo -n "\"ticket_hint\":\"${cipher_arr[5]}\"," echo -n "\"ocsp_stapling\":\"${cipher_arr[6]}\"," pfs="${cipher_arr[7]}" - [[ "$pfs" == "" ]] && pfs="None" + [[ -z $pfs ]] && pfs="None" echo -n "\"pfs\":\"$pfs\"" if [[ "${cipher_arr[0]}" =~ ECDH ]]; then echo -n "," @@ -1019,7 +1019,7 @@ test_curves_fallback() { # client doesn't advertise support for curves the server needs fallback_supported="unknown" - if [[ "$ecc_ciphers" == "" ]]; then + if [[ -z $ecc_ciphers ]]; then verbose "No ECC cipher found, can't test curve fallback" return fi From 8f3341a165d18c07793805e28792ac490ef1760a Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Fri, 18 Sep 2015 11:53:18 -0700 Subject: [PATCH 22/23] openssl fallback and version warnings should go to STDERR --- cipherscan | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cipherscan b/cipherscan index 642b6f8..3e4ee19 100755 --- a/cipherscan +++ b/cipherscan @@ -1415,13 +1415,13 @@ debug "target: $TARGET" if [[ ! -x $OPENSSLBIN ]]; then OPENSSLBIN=$(which openssl) if [[ "$OUTPUTFORMAT" == "terminal" ]]; then - echo "custom openssl not executable, falling back to system one from $OPENSSLBIN" + echo "custom openssl not executable, falling back to system one from $OPENSSLBIN" 1>&2 fi fi if [[ $TEST_CURVES == "True" ]]; then if [[ ! -z "$($OPENSSLBIN s_client -curves 2>&1|head -1|grep 'unknown option')" ]]; then - echo "curves testing not available with your version of openssl, disabling it" + echo "curves testing not available with your version of openssl, disabling it" 1>&2 TEST_CURVES="False" fi fi From 179cbe8db1c6cfa9c40b6d597200c14c83f86a68 Mon Sep 17 00:00:00 2001 From: Richard Soderberg Date: Fri, 18 Sep 2015 11:56:28 -0700 Subject: [PATCH 23/23] refuse to permit --allciphers and --json together --- cipherscan | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cipherscan b/cipherscan index 3e4ee19..4a784d2 100755 --- a/cipherscan +++ b/cipherscan @@ -1387,6 +1387,11 @@ if [[ -n $CAPATH && -n $CACERTS ]]; then exit 1 fi +if [[ -n $ALLCIPHERS && $OUTPUTFORMAT == "json" ]]; then + echo "--allciphers cannot produce JSON output, aborting." 1>&2 + exit 1 +fi + # echo parameters left: $@ if (( $# < 1 )); then