From f0132dcb7ff120fdb2b095155e701bda3dbbf3ff Mon Sep 17 00:00:00 2001 From: Dirk Date: Wed, 7 Sep 2016 21:34:27 +0200 Subject: [PATCH 1/8] stringer usabiliy warning for SHA1 + HTTP --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index fb0145d..01f4213 100755 --- a/testssl.sh +++ b/testssl.sh @@ -4208,7 +4208,7 @@ certificate_info() { sha1WithRSAEncryption) pr_svrty_medium "SHA1 with RSA" if [[ "$SERVICE" == HTTP ]]; then - out " -- besides: users will receive a strong browser warning" + out " -- besides: users will receive a "; pr_svrty_high "strong browser WARNING" fi outln fileout "${json_prefix}algorithm" "MEDIUM" "Signature Algorithm: SHA1 with RSA (warning)" @@ -8790,4 +8790,4 @@ fi exit $? -# $Id: testssl.sh,v 1.540 2016/09/06 06:32:04 dirkw Exp $ +# $Id: testssl.sh,v 1.541 2016/09/07 19:34:26 dirkw Exp $ From 7dbbe42ea06243365259e8a79450c2fe88a87346 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 12 Sep 2016 16:09:00 +0200 Subject: [PATCH 2/8] compare_server_name_to_cert(): Fix unassigned vars. Two instances of referenced but not assigned variables ('req' instead of 'ret'). In testssl.sh line 4130: if [[ $req -eq 0 ]]; then ^-- SC2154: req is referenced but not assigned. Found by ShellCheck. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index 01f4213..61c1163 100755 --- a/testssl.sh +++ b/testssl.sh @@ -4127,7 +4127,7 @@ compare_server_name_to_cert() [[ $(toupper "$san") == "$servername" ]] && ret=1 && break done - if [[ $req -eq 0 ]]; then + if [[ $ret -eq 0 ]]; then # Check whether any of the IP addresses in the certificate match the servername ip_sans=$($OPENSSL x509 -in "$cert" -noout -text 2>>$ERRFILE | grep -A2 "Subject Alternative Name" | \ tr ',' '\n' | grep "IP Address:" | sed -e 's/IP Address://g' -e 's/ //g') @@ -4138,7 +4138,7 @@ compare_server_name_to_cert() # Check whether any of the DNS names in the certificate are wildcard names # that match the servername - if [[ $req -eq 0 ]]; then + if [[ $ret -eq 0 ]]; then for san in $dns_sans; do wildcard_match "$servername" "$san" [[ $? -eq 0 ]] && ret=2 && break From b9d9a909b1fc646cd3367e16ae4e3f4c7b8e9b23 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Mon, 12 Sep 2016 16:20:05 +0200 Subject: [PATCH 3/8] certificate_info(): Fix unassigned variable. Fix referenced but not assigned variable 'sign_algo'. In testssl.sh line 4309: fileout "${json_prefix}algorithm" "DEBUG" "Signature Algorithm: $sign_algo" ^-- SC2154: sign_algo is referenced but not assigned. Found by ShellCheck. --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 01f4213..383291f 100755 --- a/testssl.sh +++ b/testssl.sh @@ -4306,7 +4306,7 @@ certificate_info() { out "$cert_sig_algo (" pr_warning "FIXME: can't tell whether this is good or not" outln ")" - fileout "${json_prefix}algorithm" "DEBUG" "Signature Algorithm: $sign_algo" + fileout "${json_prefix}algorithm" "DEBUG" "Signature Algorithm: $cert_sig_algo" ;; esac # old, but interesting: https://blog.hboeck.de/archives/754-Playing-with-the-EFF-SSL-Observatory.html From cca1b4989091ee6ab646e58af80a0fc239617bdf Mon Sep 17 00:00:00 2001 From: Dirk Date: Mon, 12 Sep 2016 21:54:51 +0200 Subject: [PATCH 4/8] - fixing wrong cipher order for URL=ipaddress --- testssl.sh | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/testssl.sh b/testssl.sh index df9275d..de04f4e 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3366,8 +3366,16 @@ run_server_preference() { outln pr_bold " Has server cipher order? " - [[ "$OPTIMAL_PROTO" == "-ssl2" ]] && addcmd="$OPTIMAL_PROTO" - [[ ! "$OPTIMAL_PROTO" =~ ssl ]] && addcmd="$SNI" && sni="$SNI" + [[ "$OPTIMAL_PROTO" == "-ssl2" ]] && addcmd="$OPTIMAL_PROTO" + if [[ ! "$OPTIMAL_PROTO" =~ ssl ]]; then + addcmd="$SNI" + sni="$SNI" + if "$HAS_NO_SSL2" && [[ -z "$SNI" ]]; then + # the supplied openssl sends otherwise an sslv2 hello -- e.g. if IP address supplied as target + # for STARTTLS this doesn't seem to be needed + addcmd="-no_ssl2" + fi + fi $OPENSSL s_client $STARTTLS -cipher $list_fwd $BUGS -connect $NODEIP:$PORT $PROXY $addcmd $ERRFILE >$TMPFILE if ! sclient_connect_successful $? $TMPFILE && [[ -z "$STARTTLS_PROTOCOL" ]]; then pr_warning "no matching cipher in this list found (pls report this): " @@ -3380,7 +3388,6 @@ run_server_preference() { # workaround is to connect with a protocol debugme out "(workaround #188) " determine_optimal_proto $STARTTLS_PROTOCOL - [[ ! "$STARTTLS_OPTIMAL_PROTO" =~ ssl ]] && addcmd2="$SNI" $OPENSSL s_client $STARTTLS $STARTTLS_OPTIMAL_PROTO -cipher $list_fwd $BUGS -connect $NODEIP:$PORT $PROXY $addcmd2 $ERRFILE >$TMPFILE if ! sclient_connect_successful $? $TMPFILE; then pr_warning "no matching cipher in this list found (pls report this): " @@ -3398,7 +3405,11 @@ run_server_preference() { addcmd2="$STARTTLS_OPTIMAL_PROTO" [[ ! "$STARTTLS_OPTIMAL_PROTO" =~ ssl ]] && addcmd2="$addcmd2 $SNI" else - [[ "$OPTIMAL_PROTO" == "-ssl2" ]] && addcmd2="$OPTIMAL_PROTO" + if [[ "$OPTIMAL_PROTO" == "-ssl2" ]]; then + addcmd2="$OPTIMAL_PROTO" + elif "$HAS_NO_SSL2"; then + addcmd2="$addcmd2 -no_ssl2" + fi [[ ! "$OPTIMAL_PROTO" =~ ssl ]] && addcmd2="$addcmd2 $SNI" fi $OPENSSL s_client $STARTTLS -cipher $list_reverse $BUGS -connect $NODEIP:$PORT $PROXY $addcmd2 >$ERRFILE >$TMPFILE From 2a926609ca7be7befff96bafbe6088205c93e730 Mon Sep 17 00:00:00 2001 From: Daniel Reichelt Date: Tue, 13 Sep 2016 21:15:43 +0200 Subject: [PATCH 5/8] quote argument for s_client's -nextprotoneg parameter The argument to -nextprotoneg is provided in sometimes empty an unquoted variables. Because of the missing quotes, the next word on the line "-status" gets parsed as "-nextprotoneg"'s argument instead of enabling the OCSP status check. This fixes #467. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index de04f4e..5c9619e 100755 --- a/testssl.sh +++ b/testssl.sh @@ -3983,7 +3983,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 -nextprotoneg "$alpn" -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 @@ -5132,7 +5132,7 @@ run_spdy() { outln return 0 fi - $OPENSSL s_client -connect $NODEIP:$PORT $BUGS $SNI -nextprotoneg $NPN_PROTOs $ERRFILE >$TMPFILE + $OPENSSL s_client -connect $NODEIP:$PORT $BUGS $SNI -nextprotoneg "$NPN_PROTOs" $ERRFILE >$TMPFILE tmpstr=$(grep -a '^Protocols' $TMPFILE | sed 's/Protocols.*: //') if [[ -z "$tmpstr" ]] || [[ "$tmpstr" == " " ]]; then outln "not offered" From beae0ce19562dbf9a611c6607445d970a9d5d3ac Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Wed, 14 Sep 2016 12:11:51 +0200 Subject: [PATCH 6/8] run_{rp,application}_banner(): Fix unassigned variables. This commit fixes the following two instances of referenced but not assigned variables: ``` In testssl.sh line 1159: rp_banners="$rp_bannersline" ^-- SC2154: rp_bannersline is referenced but not assigned. In testssl.sh line 1193: app_banners="$app_bannersline" ^-- SC2154: app_bannersline is referenced but not assigned. ``` Found by ShellCheck. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index 5c9619e..fdf2f21 100755 --- a/testssl.sh +++ b/testssl.sh @@ -1156,7 +1156,7 @@ run_rp_banner() { first=false fi emphasize_stuff_in_headers "$line" - rp_banners="$rp_bannersline" + rp_banners="${rp_banners}${line}" done < $TMPFILE fileout "rp_header" "INFO" "Reverse proxy banner(s) found: $rp_banners" fi @@ -1190,7 +1190,7 @@ run_application_banner() { first=false fi emphasize_stuff_in_headers "$line" - app_banners="$app_bannersline" + app_banners="${app_banners}${line}" done fileout "app_banner" "WARN" "Application Banners found: $app_banners" fi From 6a6d4880d6b576055d3088a298a1715d2b556a60 Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Wed, 14 Sep 2016 12:16:37 +0200 Subject: [PATCH 7/8] run_application_banner(): Fix modified in subshell bug. Refactor the while loop so it doesn't use a subshell anymore. Also use "read -r" to prevent backslash escaping. ``` In testssl.sh line 1193: app_banners="$app_bannersline" ^-- SC2030: Modification of app_banners is local (to subshell caused by pipeline). In testssl.sh line 1195: fileout "app_banner" "WARN" "Application Banners found: $app_banners" ^-- SC2031: app_banners was modified in a subshell. That change might be lost. ``` Found by ShellCheck. --- testssl.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testssl.sh b/testssl.sh index fdf2f21..8ae0b3a 100755 --- a/testssl.sh +++ b/testssl.sh @@ -1182,7 +1182,7 @@ run_application_banner() { outln "--" fileout "app_banner" "INFO" "No Application Banners found" else - cat $TMPFILE | while read line; do + while IFS='' read -r line; do line=$(strip_lf "$line") if ! $first; then out "$spaces" @@ -1191,7 +1191,7 @@ run_application_banner() { fi emphasize_stuff_in_headers "$line" app_banners="${app_banners}${line}" - done + done < "$TMPFILE" fileout "app_banner" "WARN" "Application Banners found: $app_banners" fi tmpfile_handle $FUNCNAME.txt From 42e9406ee1e92d02889fa8a5dd605efd0cafc5cc Mon Sep 17 00:00:00 2001 From: Karsten Weiss Date: Wed, 14 Sep 2016 12:23:18 +0200 Subject: [PATCH 8/8] run_rp_banner(): Fix indentation. --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 8ae0b3a..d9a1679 100755 --- a/testssl.sh +++ b/testssl.sh @@ -1147,7 +1147,7 @@ run_rp_banner() { if [[ $? -ne 0 ]]; then outln "--" fileout "rp_header" "INFO" "No reverse proxy banner found" - else + else while read line; do line=$(strip_lf "$line") if ! $first; then