Skip to content

Commit

Permalink
WIP: Fix problems with early errors
Browse files Browse the repository at this point in the history 
This PR addresses the issues raised in drwetter#1397. Rather than just deal with the Bash error messages that occur if there is a problem with the command line and either $do_json or $do_pretty_json is set to true, this PR addresses the larger problem.

html_header() sets the name for the HTML file and ensures that it is okay to write to that file. So, nothing should be written to $HTMLFILE until html_header() has been called and has checked that $HTMLFILE may be written to. So, this PR defines, $HTMLFILENAME_SET which is false until html_header() successfully completes. Any place that wants to write to $HTMLFILE then checks that $HTMLFILENAME_SET is true before performing the write.

The same issues apply for the $CSVFILE, $JSONFILE, and $LOGFILE. So, similar changes are made for each of these files.

I believe that this PR fixes the issues raised in drwetter#1397. However, given the potential impact of the changes this PR makes, it needs to be tested very thoroughly before it is merged. So, I marked it as "WIP" until I've had a chance to perform more testing.
  • Loading branch information
@dcooper16
dcooper16 committed Dec 16, 2019
1 parent 710f915 commit c9e3d77
Showing 1 changed file with 33 additions and 23 deletions.
56 changes: 33 additions & 23 deletions testssl.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ FIRST_FINDING=true # is this the first finding we are outpu
JSONHEADER=true # include JSON headers and footers in HTML file, if one is being created
CSVHEADER=true # same for CSV
HTMLHEADER=true # same for HTML
JSONFILENAME_SET=false # Indicates whether json_header() has been called to set up JSONFILE.
CSVFILENAME_SET=false # Indicates whether csv_header() has been called to set up CSVFILE.
HTMLFILENAME_SET=false # Indicates whether html_header() has been called to set up HTMLFILE.
LOGFILENAME_SET=false # Indicates whether prepare_logging() has been called to set up LOGFILE.
SECTION_FOOTER_NEEDED=false # kludge for tracking whether we need to close the JSON section object
GIVE_HINTS=false # give an additional info to findings
SERVER_SIZE_LIMIT_BUG=false # Some servers have either a ClientHello total size limit or a 128 cipher limit (e.g. old ASAs)
Expand Down Expand Up @@ -496,7 +500,7 @@ html_reserved(){

html_out() {
"$do_html" || return 0
[[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]] && printf -- "%b" "${1//%/%%}" >> "$HTMLFILE"
"$HTMLFILENAME_SET" && [[ -n "$HTMLFILE" ]] && [[ ! -d "$HTMLFILE" ]] && printf -- "%b" "${1//%/%%}" >> "$HTMLFILE"
# here and other printf's: a little bit of sanitizing with bash internal search&replace -- otherwise printf will hiccup at '%'. '--' and %b do the rest.
}

Expand Down Expand Up @@ -737,7 +741,7 @@ strip_quote() {
#################### JSON FILE FORMATTING ####################

fileout_json_footer() {
if "$do_json"; then
if "$do_json" && "$JSONFILENAME_SET"; then
if [[ "$SCAN_TIME" -eq 0 ]]; then
fileout_json_finding "scanTime" "WARN" "Scan interrupted" "" "" ""
elif [[ $SEVERITY_LEVEL -lt $LOW ]] ; then
Expand All @@ -746,7 +750,7 @@ fileout_json_footer() {
fi
printf "]\n" >> "$JSONFILE"
fi
if "$do_pretty_json"; then
if "$do_pretty_json" && "$JSONFILENAME_SET"; then
if [[ "$SCAN_TIME" -eq 0 ]]; then
echo -e " ],\n \"scanTime\" : \"Scan interrupted\"\n}" >> "$JSONFILE"
else
Expand Down Expand Up @@ -776,14 +780,14 @@ fileout_json_section() {
fileout_section_header() {
local str=""
"$2" && str="$(fileout_section_footer false)"
"$do_pretty_json" && FIRST_FINDING=true && (printf "%s%s\n" "$str" "$(fileout_json_section "$1")") >> "$JSONFILE"
"$do_pretty_json" && "$JSONFILENAME_SET" && FIRST_FINDING=true && (printf "%s%s\n" "$str" "$(fileout_json_section "$1")") >> "$JSONFILE"
SECTION_FOOTER_NEEDED=true
}

# arg1: whether to end object too
fileout_section_footer() {
"$do_pretty_json" && printf "\n ]" >> "$JSONFILE"
"$do_pretty_json" && "$1" && echo -e "\n }" >> "$JSONFILE"
"$do_pretty_json" && "$JSONFILENAME_SET" && printf "\n ]" >> "$JSONFILE"
"$do_pretty_json" && "$JSONFILENAME_SET" && "$1" && echo -e "\n }" >> "$JSONFILE"
SECTION_FOOTER_NEEDED=false
}

Expand All @@ -810,7 +814,7 @@ fileout_json_finding() {
local cwe="$5"
local hint="$6"

if "$do_json"; then
if "$do_json" && "$JSONFILENAME_SET"; then
"$FIRST_FINDING" || echo -n "," >> "$JSONFILE"
echo -e " {" >> "$JSONFILE"
fileout_json_print_parameter "id" " " "$1" true
Expand All @@ -823,7 +827,7 @@ fileout_json_finding() {
fileout_json_print_parameter "finding" " " "$finding" false
echo -e "\n }" >> "$JSONFILE"
fi
if "$do_pretty_json"; then
if "$do_pretty_json" && "$JSONFILENAME_SET"; then
if [[ "$1" == service ]]; then
if [[ $SERVER_COUNTER -gt 1 ]]; then
echo " ," >> "$JSONFILE"
Expand Down Expand Up @@ -874,14 +878,14 @@ fileout_pretty_json_banner() {
fileout_banner() {
if "$JSONHEADER"; then
# "$do_json" && # here we maybe should add a banner, too
"$do_pretty_json" && (printf "%s\n" "$(fileout_pretty_json_banner)") >> "$JSONFILE"
"$do_pretty_json" && "$JSONFILENAME_SET" && (printf "%s\n" "$(fileout_pretty_json_banner)") >> "$JSONFILE"
fi
}

fileout_separator() {
if "$JSONHEADER"; then
"$do_pretty_json" && echo " ," >> "$JSONFILE"
"$do_json" && echo -n "," >> "$JSONFILE"
"$do_pretty_json" && "$JSONFILENAME_SET" && echo " ," >> "$JSONFILE"
"$do_json" && "$JSONFILENAME_SET" && echo -n "," >> "$JSONFILE"
fi
}

Expand All @@ -905,13 +909,13 @@ fileout_insert_warning() {
[[ "$CMDLINE=" =~ -iL ]] && return 0
# Note we still have the message on screen + in HTML which is not as optimal as it could be

if "$do_pretty_json"; then
if "$do_pretty_json" && "$JSONFILENAME_SET"; then
echo -e " \"clientProblem${CLIENT_PROB_NO}\" : [" >>"$JSONFILE"
CLIENT_PROB_NO=$((CLIENT_PROB_NO + 1))
FIRST_FINDING=true # make sure we don't have a comma here
fi
fileout "$1" "$2" "$3"
if "$do_pretty_json"; then
if "$do_pretty_json" && "$JSONFILENAME_SET"; then
echo -e "\n ]," >>"$JSONFILE"
fi
}
Expand Down Expand Up @@ -941,8 +945,8 @@ fileout() {

if ( "$do_pretty_json" && [[ "$1" == service ]] ) || show_finding "$severity"; then
local finding=$(strip_lf "$(newline_to_spaces "$(strip_quote "$3")")") # additional quotes will mess up screen output
[[ -e "$JSONFILE" ]] && [[ ! -d "$JSONFILE" ]] && fileout_json_finding "$1" "$severity" "$finding" "$cve" "$cwe" "$hint"
"$do_csv" && [[ -n "$CSVFILE" ]] && [[ ! -d "$CSVFILE" ]] && \
"$JSONFILENAME_SET" && [[ -e "$JSONFILE" ]] && [[ ! -d "$JSONFILE" ]] && fileout_json_finding "$1" "$severity" "$finding" "$cve" "$cwe" "$hint"
"$do_csv" && "$CSVFILENAME_SET" && [[ -n "$CSVFILE" ]] && [[ ! -d "$CSVFILE" ]] && \
fileout_csv_finding "$1" "$NODE/$NODEIP" "$PORT" "$severity" "$finding" "$cve" "$cwe" "$hint"
"$FIRST_FINDING" && FIRST_FINDING=false
fi
Expand All @@ -960,7 +964,7 @@ json_header() {
# * this is an individual test within a mass test and all output is being placed in a single file.
! "$do_json" && ! "$do_pretty_json" && JSONHEADER=false && return 0
"$do_mass_testing" && ! "$filename_provided" && JSONHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && JSONHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && JSONHEADER=false && JSONFILENAME_SET=true && return 0

if "$do_display_only"; then
fname_prefix="local-ciphers"
Expand Down Expand Up @@ -989,6 +993,7 @@ json_header() {
"$do_json" && echo "[" > "$JSONFILE"
"$do_pretty_json" && echo "{" > "$JSONFILE"
fi
JSONFILENAME_SET=true
return 0
}

Expand All @@ -1001,7 +1006,7 @@ csv_header() {
# CSV similar to JSON
! "$do_csv" && CSVHEADER=false && return 0
"$do_mass_testing" && ! "$filename_provided" && CSVHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && CSVHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && CSVHEADER=false && CSVFILENAME_SET=true && return 0

if "$do_display_only"; then
fname_prefix="local-ciphers"
Expand All @@ -1025,8 +1030,10 @@ csv_header() {
fi
if "$APPEND"; then
CSVHEADER=false
CSVFILENAME_SET=true
else
[[ -s "$CSVFILE" ]] && fatal "non-empty \"$CSVFILE\" exists. Either use \"--append\" or (re)move it" $ERR_FCREATE
CSVFILENAME_SET=true
touch "$CSVFILE"
if "$GIVE_HINTS"; then
fileout_csv_finding "id" "fqdn/ip" "port" "severity" "finding" "cve" "cwe" "hint"
Expand All @@ -1051,7 +1058,7 @@ html_header() {
# * this is an individual test within a mass test and all HTML output is being placed in a single file.
! "$do_html" && HTMLHEADER=false && return 0
"$do_mass_testing" && ! "$filename_provided" && HTMLHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && HTMLHEADER=false && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && HTMLHEADER=false && HTMLFILENAME_SET=true && return 0

if "$do_display_only"; then
fname_prefix="local-ciphers"
Expand All @@ -1075,8 +1082,10 @@ html_header() {
fi
if "$APPEND"; then
HTMLHEADER=false
HTMLFILENAME_SET=true
else
[[ -s "$HTMLFILE" ]] && fatal "non-empty \"$HTMLFILE\" exists. Either use \"--append\" or (re)move it" $ERR_FCREATE
HTMLFILENAME_SET=true
html_out "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
html_out "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n"
html_out "<!-- This file was created with testssl.sh. https://testssl.sh -->\n"
Expand Down Expand Up @@ -1121,7 +1130,7 @@ prepare_logging() {
# Similar to html_header():
! "$do_logging" && return 0
"$do_mass_testing" && ! "$filename_provided" && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && return 0
"$CHILD_MASS_TESTING" && "$filename_provided" && LOGFILENAME_SET=true && return 0

[[ -z "$fname_prefix" ]] && fname_prefix="${FNAME_PREFIX}${NODE}_p${PORT}"

Expand All @@ -1137,6 +1146,7 @@ prepare_logging() {
if ! "$APPEND"; then
[[ -s "$LOGFILE" ]] && fatal "non-empty \"$LOGFILE\" exists. Either use \"--append\" or (re)move it" $ERR_FCREATE
fi
LOGFILENAME_SET=true
tmln_out "## Scan started as: \"$PROG_NAME $CMDLINE\"" >>"$LOGFILE"
tmln_out "## at $HNAME:$OPENSSL_LOCATION" >>"$LOGFILE"
tmln_out "## version testssl: $VERSION ${GIT_REL_SHORT:-$CVS_REL_SHORT} from $REL_DATE" >>"$LOGFILE"
Expand Down Expand Up @@ -17335,10 +17345,10 @@ child_error() {
fatal() {
outln
prln_magenta "Fatal error: $1" >&2
[[ -n "$LOGFILE" ]] && prln_magenta "Fatal error: $1" >>$LOGFILE
"$LOGFILENAME_SET" && [[ -n "$LOGFILE" ]] && prln_magenta "Fatal error: $1" >>$LOGFILE
if [[ -n "$3" ]]; then
outln "$3" >&2
[[ -n "$LOGFILE" ]] && outln "$3" >>$LOGFILE
"$LOGFILENAME_SET" && [[ -n "$LOGFILE" ]] && outln "$3" >>$LOGFILE
fi
fileout "scanProblem" "FATAL" "$1"
exit $2
Expand All @@ -17351,7 +17361,7 @@ fatal() {
ip_fatal() {
outln
prln_magenta "Fatal error: $1, proceeding with next IP (if any)" >&2
[[ -n "$LOGFILE" ]] && prln_magenta "Fatal error: $1, proceeding with next IP (if any)" >>$LOGFILE
"$LOGFILENAME_SET" && [[ -n "$LOGFILE" ]] && prln_magenta "Fatal error: $1, proceeding with next IP (if any)" >>$LOGFILE
outln
fileout "scanProblem" "FATAL" "$1, proceeding with next IP (if any)"
return 0
Expand All @@ -17363,7 +17373,7 @@ ip_fatal() {
generic_nonfatal() {
prln_magenta "$1" >&2
[[ -n $2 ]] && outln "$2"
[[ -n "$LOGFILE" ]] && prln_magenta "$1" >>$LOGFILE && [[ -n $2 ]] && outln "$2" >>$LOGFILE
"$LOGFILENAME_SET" && [[ -n "$LOGFILE" ]] && prln_magenta "$1" >>$LOGFILE && [[ -n $2 ]] && outln "$2" >>$LOGFILE
outln
fileout "scanProblem" "WARN" "$1"
return 0
Expand Down

0 comments on commit c9e3d77

Please sign in to comment.