From b7cd397c98746fb8f71e6e5f8086dedc05b181e4 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Tue, 5 Jan 2021 22:05:54 +0100 Subject: [PATCH 01/11] Add MTA-STS as a PoC This commit adds a first PoC implementation of MTA-STS (RFC 8461), see also issue #1646. What works: - test a hostname which is equal to a MX record and a domainname and has a MTS-STS setup (dev.testssl.sh) - check _mta-sts TXT record + https://mta-sts.$NODE/.well-known/mta-sts.txt - check also _smtp._tls TXT record - screen output What doesn't work - test a hostname which is not equal to domainname - test a hostname which has not mx record - fileout put - any parsing of TXT record + .well-known/mta-sts.txt - when no TXT records or .well-known/mta-sts.txt are there - fileoutput - colored screen output There's a stub function for DANE. There are also two stub functions splitting HTTP body from HTTP header which I couldn't get to work and will be removed later. Besides to avoid confusion it changes from all GET requests over HTTPS tm_out to safe_echo. It's actually exactly the same only the name is different. --- testssl.sh | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 126 insertions(+), 4 deletions(-) diff --git a/testssl.sh b/testssl.sh index 38c4f0c25..3488737f1 100755 --- a/testssl.sh +++ b/testssl.sh @@ -2210,7 +2210,7 @@ service_detection() { # trying with sockets is better than not even trying. tls_sockets "04" "$TLS13_CIPHER" "all+" "" "" false if [[ $? -eq 0 ]]; then - plaintext="$(tm_out "$GET_REQ11" | hexdump -v -e '16/1 "%02X"')" + plaintext="$(safe_echo "$GET_REQ11" | hexdump -v -e '16/1 "%02X"')" plaintext="${plaintext%%[!0-9A-F]*}" send_app_data "$plaintext" if [[ $? -eq 0 ]]; then @@ -2225,7 +2225,7 @@ service_detection() { fi else # SNI is not standardized for !HTTPS but fortunately for other protocols s_client doesn't seem to care - tm_out "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$1 -quiet $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE & + safe_echo "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$1 -quiet $BUGS -connect $NODEIP:$PORT $PROXY $SNI") >$TMPFILE 2>$ERRFILE & wait_kill $! $HEADER_MAXSLEEP was_killed=$? fi @@ -2321,12 +2321,12 @@ run_http_header() { pr_bold " HTTP Status Code " [[ -z "$1" ]] && url="/" || url="$1" - tm_out "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$OPTIMAL_PROTO $BUGS -quiet -ign_eof -connect $NODEIP:$PORT $PROXY $SNI") >$HEADERFILE 2>$ERRFILE & + safe_echo "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$OPTIMAL_PROTO $BUGS -quiet -ign_eof -connect $NODEIP:$PORT $PROXY $SNI") >$HEADERFILE 2>$ERRFILE & wait_kill $! $HEADER_MAXSLEEP if [[ $? -eq 0 ]]; then # Issue HTTP GET again as it properly finished within $HEADER_MAXSLEEP and didn't hang. # Doing it again in the foreground to get an accurate header time - tm_out "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$OPTIMAL_PROTO $BUGS -quiet -ign_eof -connect $NODEIP:$PORT $PROXY $SNI") >$HEADERFILE 2>$ERRFILE + safe_echo "$GET_REQ11" | $OPENSSL s_client $(s_client_options "$OPTIMAL_PROTO $BUGS -quiet -ign_eof -connect $NODEIP:$PORT $PROXY $SNI") >$HEADERFILE 2>$ERRFILE NOW_TIME=$(date "+%s") HTTP_TIME=$(awk -F': ' '/^date:/ { print $2 } /^Date:/ { print $2 }' $HEADERFILE) HAD_SLEPT=0 @@ -7354,6 +7354,124 @@ tls_time() { return 0 } +# rfc8461 +sub_mta_sts() { + local mta_sts_record="" + local policy="" + local smtp_tls_record="" + local spaces="$1" + local useragent="$UA_STD" + $SNEAKY && useragent="$UA_SNEAKY" + + [[ ! "$STARTTLS_PROTOCOL" =~ smtp ]] && return 0 + + # This works currently only when the MX record is equal the domainname like with the testcase dev.testssl.sh + # So either we must only execute this when called --mx or we must deduce the domain name from $NODE somehow. + # For the latter we could reverse check again with get_mx_record whether the name passed later passed + # to this function is an mx record from this domain. + # So the plan is to chek whether $CMDLINE matches --mx. If not we check whether there is an MX record + # for $NODE which matches the current $NODE. If not we subsequently remove the leading hostname part of + # the $NODE and check whether this is a domainname and has a MX which matches the original node. + # If we end up @ DOMAIN.TLD and didn't find anything we emit a message and return. + + pr_bold " MTA-STS Policy " + + mta_sts_record="$(get_txt_record _mta-sts.$NODE)" + # look for exact match for 'v=STSv1' + # look for exact match for 'id=' + + # echo "$mta_sts_record"; echo + + policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$NODE\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect $NODEIP:443 $PROXY $SNI") 2>$ERRFILE)" + # here also the openssl return val needs to be checked + + #tmp="$(printf "$policy" | awk '/^$/ { p=1;next } { if(!p) { print } }')" + # policy="$(awk '/^$/ { p=1;next } { if(!p) { print } }' <<< "$policy")" + policy="$(print_after_blankline "$policy")" + #echo "POLICY2: $tmp " + # echo "$policy"; echo + + # header needs to be stripped. Either the lower bytes which come after Content-Length in the header. + # or starting from version or starting after blank line + + # check policy: + # - grep -Ew 'version|mode|mx|max_age' + # - version.*STSv1$ + # - grep 'mode:.*testing|mode:.*enforce' + # - grep 'max_age:.*[0-9](5-10)' + # - max_age should be sufficient otherwise caching it is ~useless, see HSTS + # - whether mx record matches + + if [[ $DEBUG -ge 1 ]]; then + echo "$mta_sts_record" >$TMPFILE/_mta-sts.$NODE.txt + echo "$policy" >$TMPFILE/$NODE.mta-sts.well-known_mta-sts.txt + echo "$smtp_tls_record" > $TMPFILE/_smtp._tls.$NODE + fi + + smtp_tls_record="$(get_txt_record _smtp._tls.$NODE)" + + outln "valid _mta-sts TXT record \"$mta_sts_record\"" + out "$spaces" + outln "valid enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + out "$spaces" + outln "optional _smtp._tls TXT record \"$smtp_tls_record\"" + + return 0 +} + +# e.g. for removing the HTTP header +# +print_after_blankline() { + # doesn't work (oneliner with $1 instead of multiline): + #awk '/^$/ { p=1;next } { if(p) { print } }' <<< $1 + local first=true + local line="" + + while read -r line; do + if ! "$first"; then + safe_echo "$line\n" + else + # ignore everything until we hit an empty line or a line with a blank or a CR / LF + if [[ -z "$line" ]] || [[ "$line" =~ ^[[:space:]]$ ]]; then + first=false + continue + fi + fi + done <<< $1 +set +x +} + +# e.g. for removing the body +# +print_before_blankline() { + # doesn't work (oneliner with $1 instead of multiline): + awk '/^$/ { p=1;next } { if(!p) { print } }' <<< $1 +} + + +# RFC 6394 +# RFC 6698 +# RFC 7218 +# RFC 7671 +# RFC 7672 +# RFC 7673 +sub_dane() { + local tlsa_record="" + local rrsig_record="" + local spaces="$1" + + # Not yet implemeted + return 0 + + pr_bold " DANE / DNSSEC " + + tlsa_record="$(get_tlsa_record _$PORT._tcp.$NODE)" + # parsing TLSA certificate usage, TLSA selector, TLSA matching type, hash + rrsig_record="$(get_rrsig_record $NODE)" + + # return 0 +} + # core function determining whether handshake succeeded or not # arg1: return value of "openssl s_client connect" # arg2: temporary file with the server hello @@ -9475,6 +9593,7 @@ run_server_defaults() { local -a -i success local cn_nosni cn_sni sans_nosni sans_sni san tls_extensions local using_sockets=true + local spaces=" " "$SSL_NATIVE" && using_sockets=false @@ -9821,6 +9940,9 @@ run_server_defaults() { tls_time + sub_mta_sts "$spaces" + sub_dane "$spaces" + if [[ -n "$SNI" ]] && [[ $certs_found -ne 0 ]] && [[ ! -e $HOSTCERT.nosni ]]; then # no cipher suites specified here. We just want the default vhost subject if ! "$HAS_TLS13" && [[ $(has_server_protocol "tls1_3") -eq 0 ]]; then From 43b05b082c933ad0555bef318db17afafd94663c Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Wed, 6 Jan 2021 14:12:12 +0100 Subject: [PATCH 02/11] Added fileout, raw good/bad/info checks --- testssl.sh | 64 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/testssl.sh b/testssl.sh index 3488737f1..4bfd8f9f8 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7354,12 +7354,15 @@ tls_time() { return 0 } -# rfc8461 +# rfc8461, rfc8460 sub_mta_sts() { local mta_sts_record="" local policy="" local smtp_tls_record="" local spaces="$1" + # we might reconsider this as booleans arent very flexible: + local mta_sts_record_ok=false policy_ok=false smtp_tls_record_ok=false + local jsonID="smtp_mtasts" local useragent="$UA_STD" $SNEAKY && useragent="$UA_SNEAKY" @@ -7377,23 +7380,19 @@ sub_mta_sts() { pr_bold " MTA-STS Policy " mta_sts_record="$(get_txt_record _mta-sts.$NODE)" - # look for exact match for 'v=STSv1' - # look for exact match for 'id=' - + # look for exact match for 'v=STSv1' and 'id=' + if [[ "$mta_sts_record" =~ v=STSv1 ]] && [[ "$mta_sts_record" =~ id= ]] && [[ "$mta_sts_record" =~ \; ]]; then + # id check needs to improved , see sts-id in https://tools.ietf.org/html/rfc8461#section-3.1 + mta_sts_record_ok=true + fi # echo "$mta_sts_record"; echo policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$NODE\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect $NODEIP:443 $PROXY $SNI") 2>$ERRFILE)" # here also the openssl return val needs to be checked - #tmp="$(printf "$policy" | awk '/^$/ { p=1;next } { if(!p) { print } }')" - # policy="$(awk '/^$/ { p=1;next } { if(!p) { print } }' <<< "$policy")" policy="$(print_after_blankline "$policy")" - #echo "POLICY2: $tmp " # echo "$policy"; echo - # header needs to be stripped. Either the lower bytes which come after Content-Length in the header. - # or starting from version or starting after blank line - # check policy: # - grep -Ew 'version|mode|mx|max_age' # - version.*STSv1$ @@ -7402,6 +7401,9 @@ sub_mta_sts() { # - max_age should be sufficient otherwise caching it is ~useless, see HSTS # - whether mx record matches + # for the time being: + [[ -n "$policy" ]] && policy_ok=true + if [[ $DEBUG -ge 1 ]]; then echo "$mta_sts_record" >$TMPFILE/_mta-sts.$NODE.txt echo "$policy" >$TMPFILE/$NODE.mta-sts.well-known_mta-sts.txt @@ -7409,21 +7411,44 @@ sub_mta_sts() { fi smtp_tls_record="$(get_txt_record _smtp._tls.$NODE)" + # for the time being: + [[ -n "$smtp_tls_record" ]] && smtp_tls_record_ok=true - outln "valid _mta-sts TXT record \"$mta_sts_record\"" + if "$mta_sts_record_ok"; then + pr_svrty_good "valid" + fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" + else + pr_svrty_low "invalid" + fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" + fi + outln " _mta-sts TXT record \"$mta_sts_record\"" out "$spaces" - outln "valid enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + + if "$policy_ok"; then + pr_svrty_good "valid and enforced" + fileout "${jsonID}_policy" "OK" "valid and enforced policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + else + # missing: too short, not enforced, etc.. + pr_svrty_low "invalid" + fileout "${jsonID}_policy" "LOW" "invalid policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + fi + outln " policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" out "$spaces" - outln "optional _smtp._tls TXT record \"$smtp_tls_record\"" + + if "$smtp_tls_record_ok"; then + outln "optional _smtp._tls TXT record \"$smtp_tls_record\"" + fileout "${jsonID}_tlsrpt" "INFO" "optional _smtp._tls TXT record \"$smtp_tls_record\"" + else + outln "No TLS RPT record" + fileout "${jsonID}_tlsrpt" "INFO" "no or invalid optional _smtp._tls TXT record \"$smtp_tls_record\"" + fi return 0 } -# e.g. for removing the HTTP header +# e.g. for removing the HTTP header. To be moved to the top # print_after_blankline() { - # doesn't work (oneliner with $1 instead of multiline): - #awk '/^$/ { p=1;next } { if(p) { print } }' <<< $1 local first=true local line="" @@ -7438,15 +7463,8 @@ print_after_blankline() { fi fi done <<< $1 -set +x } -# e.g. for removing the body -# -print_before_blankline() { - # doesn't work (oneliner with $1 instead of multiline): - awk '/^$/ { p=1;next } { if(!p) { print } }' <<< $1 -} # RFC 6394 From d96cfddf9b663e753ece8b64f59a63387773df09 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Tue, 12 Jan 2021 17:44:12 +0100 Subject: [PATCH 03/11] Add better mta_sts_record / mts-sts policy validation Fix temp diretoty for debugging --- testssl.sh | 107 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 84 insertions(+), 23 deletions(-) diff --git a/testssl.sh b/testssl.sh index 4bfd8f9f8..855643350 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7362,12 +7362,18 @@ sub_mta_sts() { local spaces="$1" # we might reconsider this as booleans arent very flexible: local mta_sts_record_ok=false policy_ok=false smtp_tls_record_ok=false + local -a failreason_policy=() + local -a failreason_mtasts_rec=() local jsonID="smtp_mtasts" + local mx_record="" local useragent="$UA_STD" $SNEAKY && useragent="$UA_SNEAKY" [[ ! "$STARTTLS_PROTOCOL" =~ smtp ]] && return 0 + # rather move to the caller? + mx_record=$(get_mx_record "$NODE") + # This works currently only when the MX record is equal the domainname like with the testcase dev.testssl.sh # So either we must only execute this when called --mx or we must deduce the domain name from $NODE somehow. # For the latter we could reverse check again with get_mx_record whether the name passed later passed @@ -7380,40 +7386,89 @@ sub_mta_sts() { pr_bold " MTA-STS Policy " mta_sts_record="$(get_txt_record _mta-sts.$NODE)" - # look for exact match for 'v=STSv1' and 'id=' - if [[ "$mta_sts_record" =~ v=STSv1 ]] && [[ "$mta_sts_record" =~ id= ]] && [[ "$mta_sts_record" =~ \; ]]; then - # id check needs to improved , see sts-id in https://tools.ietf.org/html/rfc8461#section-3.1 - mta_sts_record_ok=true - fi # echo "$mta_sts_record"; echo + mta_sts_record_ok=true + if [[ -z "$mta_sts_record" ]]; then + failreason_mtasts_rec+=("no record") + mta_sts_record_ok=false + else + if [[ $(count_lines "$(safe_echo "$mta_sts_record" | tr ';' '\n')") -ne 2 ]]; then + failreason_mtasts_rec+=("number of ; should be 2") + mta_sts_record_ok=false + fi + IFS=';' read v id <<< "${mta_sts_record}" + if [[ ! "$v" =~ v=STSv1 ]] ; then + failreason_mtasts_rec+=("v seems wrong") + mta_sts_record_ok=false + fi + if [[ ! "$id" =~ id= ]]; then + failreason_mtasts_rec+=("id seems wrong") + mta_sts_record_ok=false + else + id="${id#*=}" # strip key + if [[ ! "$id" =~ ^[[:alnum:]]{1,32}$ ]]; then + failreason_mtasts_rec+=("should be up to 32 alnum chars ") + mta_sts_record_ok=false + fi + fi + fi + policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$NODE\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect $NODEIP:443 $PROXY $SNI") 2>$ERRFILE)" # here also the openssl return val needs to be checked policy="$(print_after_blankline "$policy")" # echo "$policy"; echo + # Syntax check, keep in mind $policy appears in bash as a single line with Unix LF (0a). + policy_ok=true + if [[ -z "$policy" ]]; then + failreason_policy+=("policy https://mta-sts.$NODE/.well-known/mta-sts.txt not found") + policy_ok=false + else + for c in version mode mx max_age; do + if [[ ! "$policy" =~ $c: ]] && [[ ! "$policy" =~ $c\ : ]]; then + policy_ok=false + failreason_policy+=("$c wrong formatted/missing") + fi + done + + # we use at most 10 spaces. ToDo: look into the policy + if "$policy_ok"; then + if [[ ! "$policy" =~ version[\ ]{0,10}:[\ ]{0,10}STSv1 ]]; then + failreason_policy+=("version should be STSv1 ") + fi + if [[ ! "$policy" =~ max_age[\ ]{0,10}:[\ ]{0,10}[0-9]{1,20} ]]; then + failreason_policy+=("max age is not a number") + fi + if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing) ]]; then + failreason_policy+=("policy is neither testing or enforce") + fi + if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}testing ]]; then + policy_mode=testing + fi + fi + fi + [[ -n "$failreason_policy" ]] && policy_ok=false + # get max_age: + # check policy: - # - grep -Ew 'version|mode|mx|max_age' - # - version.*STSv1$ - # - grep 'mode:.*testing|mode:.*enforce' - # - grep 'max_age:.*[0-9](5-10)' # - max_age should be sufficient otherwise caching it is ~useless, see HSTS # - whether mx record matches - - # for the time being: - [[ -n "$policy" ]] && policy_ok=true + # - check against https://tools.ietf.org/html/rfc8461#section-3.2 if [[ $DEBUG -ge 1 ]]; then - echo "$mta_sts_record" >$TMPFILE/_mta-sts.$NODE.txt - echo "$policy" >$TMPFILE/$NODE.mta-sts.well-known_mta-sts.txt - echo "$smtp_tls_record" > $TMPFILE/_smtp._tls.$NODE + echo "$mta_sts_record" >$TEMPDIR/_mta-sts.$NODE.txt + echo "$policy" >$TEMPDIR/$NODE.policy_mta-sts.txt + echo "$smtp_tls_record" > $TEMPDIR/_smtp._tls.$NODE fi smtp_tls_record="$(get_txt_record _smtp._tls.$NODE)" # for the time being: [[ -n "$smtp_tls_record" ]] && smtp_tls_record_ok=true + + # now the verdicts if "$mta_sts_record_ok"; then pr_svrty_good "valid" fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" @@ -7425,22 +7480,28 @@ sub_mta_sts() { out "$spaces" if "$policy_ok"; then - pr_svrty_good "valid and enforced" - fileout "${jsonID}_policy" "OK" "valid and enforced policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + if [[ $policy_mode == testing ]]; then + out "valid but not enforced" + fileout "${jsonID}_policy" "INFO" "valid but not enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + else + pr_svrty_good "valid and enforced" + fileout "${jsonID}_policy" "OK" "valid and enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + fi + outln " policy https://mta-sts.$NODE/.well-known/mta-sts.txt" else # missing: too short, not enforced, etc.. - pr_svrty_low "invalid" - fileout "${jsonID}_policy" "LOW" "invalid policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + pr_svrty_low "invalid policy" + fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + outln " \"https://mta-sts.$NODE/.well-known/mta-sts.txt\": ${failreason_policy[@]}" fi - outln " policy file \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" out "$spaces" if "$smtp_tls_record_ok"; then - outln "optional _smtp._tls TXT record \"$smtp_tls_record\"" - fileout "${jsonID}_tlsrpt" "INFO" "optional _smtp._tls TXT record \"$smtp_tls_record\"" + outln "found (optional) TLS RPT TXT record \"$smtp_tls_record\"" + fileout "${jsonID}_tlsrpt" "INFO" "optional TLS-RPT TXT record \"$smtp_tls_record\"" else outln "No TLS RPT record" - fileout "${jsonID}_tlsrpt" "INFO" "no or invalid optional _smtp._tls TXT record \"$smtp_tls_record\"" + fileout "${jsonID}_tlsrpt" "INFO" "no or invalid (optional) TLS RPT record \"$smtp_tls_record\"" fi return 0 From 4f1da9b192911645f6ef0a7b448e1fc3cba90a9a Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Wed, 13 Jan 2021 11:23:36 +0100 Subject: [PATCH 04/11] Trying to address some of the domain issues for MTA-STS There are checks now whether testssl.sh was started with --max and whether we aim at a target which is an MX record. It has not been thoutoughly tested but works for a couple of scenarios. There were cases being identififed where this fails, see comments in the code. Also this commit addresses an error in the URL handling: for DNS queries a trailing dot is fine in the variable $NODE. For HTTP queries it is not. --- testssl.sh | 98 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 31 deletions(-) diff --git a/testssl.sh b/testssl.sh index 855643350..d8e615380 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7366,26 +7366,51 @@ sub_mta_sts() { local -a failreason_mtasts_rec=() local jsonID="smtp_mtasts" local mx_record="" + local domain="" + local fqdnparts=$(count_words "${URI//./ }") local useragent="$UA_STD" $SNEAKY && useragent="$UA_SNEAKY" [[ ! "$STARTTLS_PROTOCOL" =~ smtp ]] && return 0 - # rather move to the caller? - mx_record=$(get_mx_record "$NODE") - - # This works currently only when the MX record is equal the domainname like with the testcase dev.testssl.sh - # So either we must only execute this when called --mx or we must deduce the domain name from $NODE somehow. - # For the latter we could reverse check again with get_mx_record whether the name passed later passed - # to this function is an mx record from this domain. - # So the plan is to chek whether $CMDLINE matches --mx. If not we check whether there is an MX record - # for $NODE which matches the current $NODE. If not we subsequently remove the leading hostname part of - # the $NODE and check whether this is a domainname and has a MX which matches the original node. - # If we end up @ DOMAIN.TLD and didn't find anything we emit a message and return. +# echo "NODE: $NODE / URI: $URI / CMDLINE: ${CMDLINE[@]}" pr_bold " MTA-STS Policy " - mta_sts_record="$(get_txt_record _mta-sts.$NODE)" + if [[ "${CMDLINE[@]}" =~ \ --mx\ ]]; then + domain="$URI" + elif [[ fqdnparts -eq 2 ]] && [[ "$NODE" == ${URI%:*} ]]; then + # remove the port an check whether bot are the same when there's no subdomain + domain="$NODE" + else + # What's left is a sub.domain.tld or sub.sub.domain.tld + # But we implement first a safety measure to prevent querying a tld + if [[ $(count_words "${NODE//./ }") -ge 3 ]]; then + # we query sub.domain.tld first whether is has a _mta-sts TXT record + domain=$NODE + mta_sts_record="$(get_txt_record _mta-sts.$domain)" + if [[ -z "$mta_sts_record" ]]; then + # strip the (first sub): + domain=${NODE#*.} + mta_sts_record="$(get_txt_record _mta-sts.$domain)" + fi + if [[ -z "$mta_sts_record" ]]; then + # unset to signal we didn't have success + domain="" + fi + else + echo "#FIXME" + echo "NODE: $NODE / URI: $URI / CMDLINE: ${CMDLINE[@]}" + fi + fi + # 2+ level of subdomains? + # we check only for the TXT record in subdomains and give up if there's nothing?? + # Possible that TXT record for domain overrides sub domain. if so: when ? + # error: ./testssl.sh -S --mx gmail.com --> no _mta-sts TXT record + # --mx does this test for every single MX. We need to save the values + + + [[ -z "mta_sts_record" ]] && mta_sts_record="$(get_txt_record _mta-sts.$domain)" # echo "$mta_sts_record"; echo mta_sts_record_ok=true @@ -7414,7 +7439,7 @@ sub_mta_sts() { fi fi - policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$NODE\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect $NODEIP:443 $PROXY $SNI") 2>$ERRFILE)" + policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$domain\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect mta-sts.$domain:443 $PROXY -servername mta-sts.$domain") 2>$ERRFILE)" # here also the openssl return val needs to be checked policy="$(print_after_blankline "$policy")" @@ -7423,7 +7448,6 @@ sub_mta_sts() { # Syntax check, keep in mind $policy appears in bash as a single line with Unix LF (0a). policy_ok=true if [[ -z "$policy" ]]; then - failreason_policy+=("policy https://mta-sts.$NODE/.well-known/mta-sts.txt not found") policy_ok=false else for c in version mode mx max_age; do @@ -7437,12 +7461,15 @@ sub_mta_sts() { if "$policy_ok"; then if [[ ! "$policy" =~ version[\ ]{0,10}:[\ ]{0,10}STSv1 ]]; then failreason_policy+=("version should be STSv1 ") + policy_ok=false fi if [[ ! "$policy" =~ max_age[\ ]{0,10}:[\ ]{0,10}[0-9]{1,20} ]]; then failreason_policy+=("max age is not a number") + policy_ok=false fi if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing) ]]; then failreason_policy+=("policy is neither testing or enforce") + policy_ok=false fi if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}testing ]]; then policy_mode=testing @@ -7458,41 +7485,49 @@ sub_mta_sts() { # - check against https://tools.ietf.org/html/rfc8461#section-3.2 if [[ $DEBUG -ge 1 ]]; then - echo "$mta_sts_record" >$TEMPDIR/_mta-sts.$NODE.txt - echo "$policy" >$TEMPDIR/$NODE.policy_mta-sts.txt - echo "$smtp_tls_record" > $TEMPDIR/_smtp._tls.$NODE + echo "$mta_sts_record" >$TEMPDIR/_mta-sts.$domain.txt + echo "$policy" >$TEMPDIR/$domain.policy_mta-sts.txt + echo "$smtp_tls_record" > $TEMPDIR/_smtp._tls.$domain fi - smtp_tls_record="$(get_txt_record _smtp._tls.$NODE)" + smtp_tls_record="$(get_txt_record _smtp._tls.$domain)" # for the time being: [[ -n "$smtp_tls_record" ]] && smtp_tls_record_ok=true - # now the verdicts if "$mta_sts_record_ok"; then pr_svrty_good "valid" fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" + outln " _mta-sts TXT record \"$mta_sts_record\"" + elif [[ -z "$mta_sts_record" ]]; then + pr_svrty_low "no" + fileout "${jsonID}_txtrecord" "LOW" "no _mta-sts TXT record" + outln " _mta-sts TXT record" else pr_svrty_low "invalid" - fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" + fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record \"$mta_sts_record\"" + outln " _mta-sts TXT record \"$mta_sts_record\"" fi - outln " _mta-sts TXT record \"$mta_sts_record\"" out "$spaces" if "$policy_ok"; then if [[ $policy_mode == testing ]]; then out "valid but not enforced" - fileout "${jsonID}_policy" "INFO" "valid but not enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "INFO" "valid but not enforced policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" else pr_svrty_good "valid and enforced" - fileout "${jsonID}_policy" "OK" "valid and enforced policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "OK" "valid and enforced policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" fi - outln " policy https://mta-sts.$NODE/.well-known/mta-sts.txt" + outln " policy https://mta-sts.$domain/.well-known/mta-sts.txt" + elif [[ -z "$policy" ]]; then + pr_svrty_low "no policy" + fileout "${jsonID}_policy" "LOW" "no policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" else # missing: too short, not enforced, etc.. pr_svrty_low "invalid policy" - fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$NODE/.well-known/mta-sts.txt\"" - outln " \"https://mta-sts.$NODE/.well-known/mta-sts.txt\": ${failreason_policy[@]}" + fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\": ${failreason_policy[@]}" fi out "$spaces" @@ -19631,11 +19666,11 @@ prepare_debug() { cat >$TEMPDIR/environment.txt << EOF -CVS_REL: $CVS_REL GIT_REL: $GIT_REL PID: $$ -commandline: "$CMDLINE" +commandline: ${CMDLINE[@]} +URI: $URI bash version: ${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}.${BASH_VERSINFO[2]} status: ${BASH_VERSINFO[4]} machine: ${BASH_VERSINFO[5]} @@ -20019,7 +20054,7 @@ parse_hn_port() { NODE="$1" NODE="${NODE/https\:\/\//}" # strip "https" NODE="${NODE%%/*}" # strip trailing urlpath - NODE="${NODE%%.}" # strip trailing "." if supplied + if grep -q ':$' <<< "$NODE"; then if grep -wq http <<< "$NODE"; then fatal "\"http\" is not what you meant probably" $ERR_CMDLINE @@ -20041,6 +20076,7 @@ parse_hn_port() { grep -q ':' <<< "$NODE" && \ PORT=$(sed 's/^.*\://' <<< "$NODE") && NODE=$(sed 's/\:.*$//' <<< "$NODE") fi + NODE="${NODE%%.}" # strip trailing "." if supplied # We check for non-ASCII chars now. If there are some we'll try to convert it if IDN/IDN2 is installed # If not, we'll continue. Hoping later that dig can use it. If not the error handler will tell @@ -20364,8 +20400,8 @@ get_mx_record() { } # arg1: domain / hostname. Returned will be the TXT record as a string which can be multilined -# (one entry per line), for e.g. non-MTA-STS records. -# Is supposed to be used by MTA STS in the future like get_txt_record _mta-sts.DOMAIN.TLD +# (one entry per line), for e.g. MTA-STS records. +# get_txt_record() { local record="" local saved_openssl_conf="$OPENSSL_CONF" From 429a8cf64382366dcdc39d50c5fdd3865ba06ed4 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Thu, 14 Jan 2021 14:30:13 +0100 Subject: [PATCH 05/11] Fixed two more errors for MTA-STS and domain identification * for sub.domain.tld $domain was empty * typo for checking empty variable mta_sts_record led to a missing query for some type of domains --- testssl.sh | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/testssl.sh b/testssl.sh index d8e615380..cd0ecd140 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7394,23 +7394,19 @@ sub_mta_sts() { domain=${NODE#*.} mta_sts_record="$(get_txt_record _mta-sts.$domain)" fi - if [[ -z "$mta_sts_record" ]]; then - # unset to signal we didn't have success - domain="" - fi else echo "#FIXME" echo "NODE: $NODE / URI: $URI / CMDLINE: ${CMDLINE[@]}" fi fi + # 2+ level of subdomains? # we check only for the TXT record in subdomains and give up if there's nothing?? # Possible that TXT record for domain overrides sub domain. if so: when ? - # error: ./testssl.sh -S --mx gmail.com --> no _mta-sts TXT record - # --mx does this test for every single MX. We need to save the values - + # - ./testssl.sh -S --mx gmail.com --> no _mta-sts TXT record ? + # - --mx does this for every single MX. As the values are domain specific: global array? - [[ -z "mta_sts_record" ]] && mta_sts_record="$(get_txt_record _mta-sts.$domain)" + [[ -z "$mta_sts_record" ]] && mta_sts_record="$(get_txt_record _mta-sts.$domain)" # echo "$mta_sts_record"; echo mta_sts_record_ok=true @@ -7457,7 +7453,7 @@ sub_mta_sts() { fi done - # we use at most 10 spaces. ToDo: look into the policy + # we use at most 10 spaces. ToDo: check with RFC wrt to the format of the policy if "$policy_ok"; then if [[ ! "$policy" =~ version[\ ]{0,10}:[\ ]{0,10}STSv1 ]]; then failreason_policy+=("version should be STSv1 ") @@ -7468,7 +7464,7 @@ sub_mta_sts() { policy_ok=false fi if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing) ]]; then - failreason_policy+=("policy is neither testing or enforce") + failreason_policy+=("policy should be either testing or enforce") policy_ok=false fi if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}testing ]]; then From aa3b12a54377d75d1e68040558ed21e21d7defcc Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sun, 17 Jan 2021 11:02:47 +0100 Subject: [PATCH 06/11] Save work, MTA-STS * Imporved handling of quotes in TXT records. Previously we just stripped all quotes. Now get_txt_record includes ALL of them. sub_mta_sts() then removed the first and last double quote. (this need to be adjusted) - get_txt_record() has been tested better for every binary so that it should return always the correct string --- testssl.sh | 73 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/testssl.sh b/testssl.sh index cd0ecd140..cf95a6490 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7383,15 +7383,15 @@ sub_mta_sts() { # remove the port an check whether bot are the same when there's no subdomain domain="$NODE" else - # What's left is a sub.domain.tld or sub.sub.domain.tld + # What's left now is a sub.domain.tld or sub.sub.domain.tld or ... # But we implement first a safety measure to prevent querying a tld if [[ $(count_words "${NODE//./ }") -ge 3 ]]; then # we query sub.domain.tld first whether is has a _mta-sts TXT record - domain=$NODE + domain="$NODE" mta_sts_record="$(get_txt_record _mta-sts.$domain)" if [[ -z "$mta_sts_record" ]]; then # strip the (first sub): - domain=${NODE#*.} + domain="${NODE#*.}" mta_sts_record="$(get_txt_record _mta-sts.$domain)" fi else @@ -7405,31 +7405,43 @@ sub_mta_sts() { # Possible that TXT record for domain overrides sub domain. if so: when ? # - ./testssl.sh -S --mx gmail.com --> no _mta-sts TXT record ? # - --mx does this for every single MX. As the values are domain specific: global array? + # How about policy delegation (CNAME?) --> Not tested yet [[ -z "$mta_sts_record" ]] && mta_sts_record="$(get_txt_record _mta-sts.$domain)" # echo "$mta_sts_record"; echo mta_sts_record_ok=true + if [[ -z "$mta_sts_record" ]]; then failreason_mtasts_rec+=("no record") mta_sts_record_ok=false else + # The TXT record string is enclosed in double quotes. We check this and remove them, only there! +#FIXME: probably wrong place --> get_txt_record() + if [[ "${mta_sts_record:0:1}" == \" ]] && [[ "${mta_sts_record:$((${#mta_sts_record}-1)):1}" ]]; then + # remove first char (here: double quote) and last (here: double quote) + mta_sts_record="${mta_sts_record:1:$((${#mta_sts_record}-1))}" + mta_sts_record="${mta_sts_record:0:$((${#mta_sts_record}-1))}" + else + failreason_mtasts_rec+=("record is not enclosed in double quotes") + mta_sts_record_ok=false + fi if [[ $(count_lines "$(safe_echo "$mta_sts_record" | tr ';' '\n')") -ne 2 ]]; then failreason_mtasts_rec+=("number of ; should be 2") mta_sts_record_ok=false fi IFS=';' read v id <<< "${mta_sts_record}" - if [[ ! "$v" =~ v=STSv1 ]] ; then + if [[ ! "$v" == v=STSv1 ]] ; then failreason_mtasts_rec+=("v seems wrong") mta_sts_record_ok=false fi - if [[ ! "$id" =~ id= ]]; then - failreason_mtasts_rec+=("id seems wrong") + if [[ ! "$id" =~ ^id= ]]; then + failreason_mtasts_rec+=("id seems wrong: $id") mta_sts_record_ok=false else id="${id#*=}" # strip key if [[ ! "$id" =~ ^[[:alnum:]]{1,32}$ ]]; then - failreason_mtasts_rec+=("should be up to 32 alnum chars ") + failreason_mtasts_rec+=("\'id\' should be up to 32 alnum chars ") mta_sts_record_ok=false fi fi @@ -7463,13 +7475,16 @@ sub_mta_sts() { failreason_policy+=("max age is not a number") policy_ok=false fi - if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing) ]]; then - failreason_policy+=("policy should be either testing or enforce") + if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing|none) ]]; then + failreason_policy+=("policy should be either testing, enforce or none") policy_ok=false fi if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}testing ]]; then policy_mode=testing fi + if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}none ]]; then + policy_mode=none + fi fi fi [[ -n "$failreason_policy" ]] && policy_ok=false @@ -7493,21 +7508,26 @@ sub_mta_sts() { # now the verdicts if "$mta_sts_record_ok"; then pr_svrty_good "valid" - fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record \"$mta_sts_record\"" - outln " _mta-sts TXT record \"$mta_sts_record\"" + outln " _mta-sts TXT record $mta_sts_record" + # quotes! + fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record $mta_sts_record" elif [[ -z "$mta_sts_record" ]]; then pr_svrty_low "no" - fileout "${jsonID}_txtrecord" "LOW" "no _mta-sts TXT record" outln " _mta-sts TXT record" + fileout "${jsonID}_txtrecord" "LOW" "no _mta-sts TXT record" else pr_svrty_low "invalid" - fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record \"$mta_sts_record\"" - outln " _mta-sts TXT record \"$mta_sts_record\"" + # quotes! + fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record" + outln " _mta-sts TXT record ${mta_sts_record}: ${failreason_mtasts_rec[@]}" fi out "$spaces" if "$policy_ok"; then if [[ $policy_mode == testing ]]; then + out "\"none\" is a valid policy but why are you using it?" + fileout "${jsonID}_policy" "INFO" "none is valid but not a helpful policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + elif [[ $policy_mode == testing ]]; then out "valid but not enforced" fileout "${jsonID}_policy" "INFO" "valid but not enforced policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" else @@ -7517,13 +7537,13 @@ sub_mta_sts() { outln " policy https://mta-sts.$domain/.well-known/mta-sts.txt" elif [[ -z "$policy" ]]; then pr_svrty_low "no policy" - fileout "${jsonID}_policy" "LOW" "no policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "LOW" "no policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" else # missing: too short, not enforced, etc.. pr_svrty_low "invalid policy" - fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\": ${failreason_policy[@]}" + fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" fi out "$spaces" @@ -20244,7 +20264,7 @@ get_a_record() { ip4=$(filter_ip4_address $(strip_lf "$(nslookup -querytype=a "$1" 2>/dev/null | awk '/^Name/ { getline; print $NF }')")) fi OPENSSL_CONF="$saved_openssl_conf" # see https://github.com/drwetter/testssl.sh/issues/134 - echo "$ip4" + safe_echo "$ip4" } # arg1: a host name. Returned will be 0-n IPv6 addresses @@ -20285,7 +20305,7 @@ get_aaaa_record() { fi fi OPENSSL_CONF="$saved_openssl_conf" # see https://github.com/drwetter/testssl.sh/issues/134 - echo "$ip6" + safe_echo "$ip6" } # RFC6844: DNS Certification Authority Authorization (CAA) Resource Record @@ -20407,23 +20427,30 @@ get_txt_record() { OPENSSL_CONF="" # see https://github.com/drwetter/testssl.sh/issues/134 # we need the last two columns here and strip any remaining double quotes later if "$HAS_HOST"; then - record="$(host -t TXT "$1" 2>/dev/null | awk -F\" '/descriptive text/ { print $(NF-1) }')" + record="$(host -t TXT "$1" 2>/dev/null | grep 'descriptive text')" elif "$HAS_DIG"; then record="$(dig +short $noidnout -t TXT "$1" 2>/dev/null)" + record="${record% from*}" elif "$HAS_DRILL"; then - record="$(drill txt $1 | awk -F\" '/^[a-z0-9].*TXT/ { print $(NF-1) }')" + record="$(drill txt $1 | grep "^$1.*TXT")" elif "$HAS_NSLOOKUP"; then - record="$(strip_lf "$(nslookup -type=MX "$1" 2>/dev/null | awk -F= '/text/ { print $(NF-1), $NF }')")" + record="$(nslookup -type=TXT "$1" 2>/dev/null | grep -w text)" else # shouldn't reach this, as we checked in the top fatal "No dig, host, drill or nslookup" $ERR_DNSBIN fi OPENSSL_CONF="$saved_openssl_conf" - echo "${record//\"/}" + # Now, strip everything until the first double quote. Attention: $record may contain a couple of quotes! + # Also we readd the leading double quote. That is wrong if the record is empty. So we need to fix that + record="$(printf "%s" "\"${record#*\"}")" + if [[ "${record}" == \" ]]; then + echo + else + safe_echo "${record}" + fi } - # set IPADDRs and IP46ADDRs # determine_ip_addresses() { From 911ac8380f25246dcc2e00d7810808607726f54a Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sun, 17 Jan 2021 21:54:12 +0100 Subject: [PATCH 07/11] Several minor updates to MTA-STA * stripping quotes moved to get_txt_record() * fixing concatenation of errors: strings though need proper formatting * new count_char_occurence() function as a general helper func * better parsing of blanks in pattern (removed also where rfc states it's not allowed) --- testssl.sh | 57 +++++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/testssl.sh b/testssl.sh index cf95a6490..ec6a43818 100755 --- a/testssl.sh +++ b/testssl.sh @@ -817,6 +817,13 @@ count_chars() { echo $(wc -c <<< "$1") } +# arg1: string to search within +# arg2: char to count (or pattern like 1|2 ) +count_char_occurence() { + local nr=${1//[^$2]} + echo ${#nr} +} + newline_to_spaces() { tr '\n' ' ' <<< "$1" | sed 's/ $//' } @@ -7413,35 +7420,24 @@ sub_mta_sts() { mta_sts_record_ok=true if [[ -z "$mta_sts_record" ]]; then - failreason_mtasts_rec+=("no record") - mta_sts_record_ok=false + failreason_mtasts_rec="no record" else - # The TXT record string is enclosed in double quotes. We check this and remove them, only there! -#FIXME: probably wrong place --> get_txt_record() - if [[ "${mta_sts_record:0:1}" == \" ]] && [[ "${mta_sts_record:$((${#mta_sts_record}-1)):1}" ]]; then - # remove first char (here: double quote) and last (here: double quote) - mta_sts_record="${mta_sts_record:1:$((${#mta_sts_record}-1))}" - mta_sts_record="${mta_sts_record:0:$((${#mta_sts_record}-1))}" - else - failreason_mtasts_rec+=("record is not enclosed in double quotes") - mta_sts_record_ok=false - fi - if [[ $(count_lines "$(safe_echo "$mta_sts_record" | tr ';' '\n')") -ne 2 ]]; then - failreason_mtasts_rec+=("number of ; should be 2") + if [[ $(count_char_occurence "$mta_sts_record" ';') -ne 2 ]]; then + failreason_mtasts_rec+="number of ; should be 2" mta_sts_record_ok=false fi IFS=';' read v id <<< "${mta_sts_record}" if [[ ! "$v" == v=STSv1 ]] ; then - failreason_mtasts_rec+=("v seems wrong") + failreason_mtasts_rec+="v seems wrong" mta_sts_record_ok=false fi - if [[ ! "$id" =~ ^id= ]]; then - failreason_mtasts_rec+=("id seems wrong: $id") + if [[ ! "$id" =~ ^[\ ]+id= ]]; then + failreason_mtasts_rec+="id seems wrong: $id" mta_sts_record_ok=false else id="${id#*=}" # strip key if [[ ! "$id" =~ ^[[:alnum:]]{1,32}$ ]]; then - failreason_mtasts_rec+=("\'id\' should be up to 32 alnum chars ") + failreason_mtasts_rec+="\'id\' should be up to 32 alnum chars " mta_sts_record_ok=false fi fi @@ -7467,22 +7463,22 @@ sub_mta_sts() { # we use at most 10 spaces. ToDo: check with RFC wrt to the format of the policy if "$policy_ok"; then - if [[ ! "$policy" =~ version[\ ]{0,10}:[\ ]{0,10}STSv1 ]]; then + if [[ ! "$policy" =~ version:[\ ]+STSv1 ]]; then failreason_policy+=("version should be STSv1 ") policy_ok=false fi - if [[ ! "$policy" =~ max_age[\ ]{0,10}:[\ ]{0,10}[0-9]{1,20} ]]; then + if [[ ! "$policy" =~ max_age:[\ ]+[0-9]{1,20} ]]; then failreason_policy+=("max age is not a number") policy_ok=false fi - if [[ ! "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}(enforce|testing|none) ]]; then + if [[ ! "$policy" =~ mode:[\ ]+(enforce|testing|none) ]]; then failreason_policy+=("policy should be either testing, enforce or none") policy_ok=false fi - if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}testing ]]; then + if [[ "$policy" =~ mode:[\ ]+testing ]]; then policy_mode=testing fi - if [[ "$policy" =~ mode[\ ]{0,10}:[\ ]{0,10}none ]]; then + if [[ "$policy" =~ mode:[\ ]+none ]]; then policy_mode=none fi fi @@ -7518,8 +7514,8 @@ sub_mta_sts() { else pr_svrty_low "invalid" # quotes! - fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record" - outln " _mta-sts TXT record ${mta_sts_record}: ${failreason_mtasts_rec[@]}" + fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record ${failreason_mtasts_rec[@]}" + outln " _mta-sts TXT record ${mta_sts_record} | ${failreason_mtasts_rec[@]}" fi out "$spaces" @@ -7542,6 +7538,7 @@ sub_mta_sts() { else # missing: too short, not enforced, etc.. pr_svrty_low "invalid policy" +#FIXME: for multiple failures we need to format ${failreason_policy[@]}, here? outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\": ${failreason_policy[@]}" fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" fi @@ -20444,10 +20441,14 @@ get_txt_record() { # Also we readd the leading double quote. That is wrong if the record is empty. So we need to fix that record="$(printf "%s" "\"${record#*\"}")" if [[ "${record}" == \" ]]; then - echo - else - safe_echo "${record}" + record='' + fi + if [[ "${record:0:1}" == \" ]] && [[ "${record:$((${#record}-1)):1}" == \" ]]; then + # remove first char (here: double quote) and last (here: double quote) + record="${record:1:$((${#record}-1))}" + record="${record:0:$((${#record}-1))}" fi + safe_echo "${record}" } From d3e592579ecda92ed2ed692192c9c48a4ce3411d Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Thu, 21 Jan 2021 22:46:46 +0100 Subject: [PATCH 08/11] Better output formating for MTA-STS * failreason_mtasts_rec additions is now adding not to the first array index only * arrays for formatting error has now separators (but also at the last index) hint from https://web.archive.org/web/20101114051536/http://codesnippets.joyent.com/posts/show/1826 * replaced a couple of quoted double quotes by single quotes * replaced a couple of quoted single quotes by single quotes Unrelated to mta-sts: * HAS_DIG_NOIDNOUT was moved to places where we need dig * echo "$mx" --> safe_echo "$mx" The latter two should be backported to 3.0 --- testssl.sh | 53 ++++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/testssl.sh b/testssl.sh index ec6a43818..84f1d2401 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7423,21 +7423,21 @@ sub_mta_sts() { failreason_mtasts_rec="no record" else if [[ $(count_char_occurence "$mta_sts_record" ';') -ne 2 ]]; then - failreason_mtasts_rec+="number of ; should be 2" + failreason_mtasts_rec+=("number of ; should be 2") mta_sts_record_ok=false fi IFS=';' read v id <<< "${mta_sts_record}" if [[ ! "$v" == v=STSv1 ]] ; then - failreason_mtasts_rec+="v seems wrong" + failreason_mtasts_rec+=("v seems wrong") mta_sts_record_ok=false fi if [[ ! "$id" =~ ^[\ ]+id= ]]; then - failreason_mtasts_rec+="id seems wrong: $id" + failreason_mtasts_rec+=("id seems wrong: $id") mta_sts_record_ok=false else id="${id#*=}" # strip key if [[ ! "$id" =~ ^[[:alnum:]]{1,32}$ ]]; then - failreason_mtasts_rec+="\'id\' should be up to 32 alnum chars " + failreason_mtasts_rec+=("'id' should be up to 32 alnum chars ") mta_sts_record_ok=false fi fi @@ -7504,7 +7504,7 @@ sub_mta_sts() { # now the verdicts if "$mta_sts_record_ok"; then pr_svrty_good "valid" - outln " _mta-sts TXT record $mta_sts_record" + outln " _mta-sts TXT record \'$mta_sts_record\'" # quotes! fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record $mta_sts_record" elif [[ -z "$mta_sts_record" ]]; then @@ -7513,43 +7513,46 @@ sub_mta_sts() { fileout "${jsonID}_txtrecord" "LOW" "no _mta-sts TXT record" else pr_svrty_low "invalid" - # quotes! - fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record ${failreason_mtasts_rec[@]}" - outln " _mta-sts TXT record ${mta_sts_record} | ${failreason_mtasts_rec[@]}" + # Append to every element a string, here: '|'. n would work as well but we'd need $spaces. + # Catch is also for the last, so maybe we should avoid arrays + # Hint is from http://web.archive.org/web/20101114051536/http://codesnippets.joyent.com/posts/show/1826 + failreason_mtasts_rec=( "${failreason_mtasts_rec[@]/%/ | }" ) + fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record $(printf '%s | ' "{failreason_mtasts_rec[@]}")" + outln " _mta-sts TXT record '${mta_sts_record}'\n${spaces} $(printf '%s' "${failreason_mtasts_rec[@]}")" fi out "$spaces" if "$policy_ok"; then if [[ $policy_mode == testing ]]; then out "\"none\" is a valid policy but why are you using it?" - fileout "${jsonID}_policy" "INFO" "none is valid but not a helpful policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "INFO" "none is valid but not a helpful policy at https://mta-sts.$domain/.well-known/mta-sts.txt" elif [[ $policy_mode == testing ]]; then out "valid but not enforced" - fileout "${jsonID}_policy" "INFO" "valid but not enforced policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "INFO" "valid but not enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" else pr_svrty_good "valid and enforced" - fileout "${jsonID}_policy" "OK" "valid and enforced policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + fileout "${jsonID}_policy" "OK" "valid and enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" fi outln " policy https://mta-sts.$domain/.well-known/mta-sts.txt" elif [[ -z "$policy" ]]; then pr_svrty_low "no policy" - outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" - fileout "${jsonID}_policy" "LOW" "no policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + outln " at https://mta-sts.$domain/.well-known/mta-sts.txt" + fileout "${jsonID}_policy" "LOW" "no policy at https://mta-sts.$domain/.well-known/mta-sts.txt" else # missing: too short, not enforced, etc.. pr_svrty_low "invalid policy" -#FIXME: for multiple failures we need to format ${failreason_policy[@]}, here? - outln " \"https://mta-sts.$domain/.well-known/mta-sts.txt\": ${failreason_policy[@]}" - fileout "${jsonID}_policy" "LOW" "invalid policy \"https://mta-sts.$domain/.well-known/mta-sts.txt\"" + failreason_policy=( "${failreason_policy[@]/%/ | }" ) + fileout "${jsonID}_policy" "LOW" "invalid policy https://mta-sts.$domain/.well-known/mta-sts.txt: $(printf '%s' ${failreason_policy[@]})" + outln " at https://mta-sts.$domain/.well-known/mta-sts.txt: $(printf '%s' ${failreason_policy[@]})" fi out "$spaces" if "$smtp_tls_record_ok"; then - outln "found (optional) TLS RPT TXT record \"$smtp_tls_record\"" - fileout "${jsonID}_tlsrpt" "INFO" "optional TLS-RPT TXT record \"$smtp_tls_record\"" + outln "found (optional) TLS RPT TXT record '$smtp_tls_record'" + fileout "${jsonID}_tlsrpt" "INFO" "optional TLS-RPT TXT record '$smtp_tls_record'" else outln "No TLS RPT record" - fileout "${jsonID}_tlsrpt" "INFO" "no or invalid (optional) TLS RPT record \"$smtp_tls_record\"" + fileout "${jsonID}_tlsrpt" "INFO" "no or invalid (optional) TLS RPT record '$smtp_tls_record'" fi return 0 @@ -20226,7 +20229,6 @@ get_a_record() { local saved_openssl_conf="$OPENSSL_CONF" local noidnout="" - "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" [[ "$NODNS" == none ]] && return 0 # if no DNS lookup was instructed, leave here if [[ "$1" == localhost ]]; then # This is a bit ugly but prevents from doing DNS lookups which could fail @@ -20249,6 +20251,7 @@ get_a_record() { fi fi if [[ -z "$ip4" ]] && "$HAS_DIG"; then + "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" ip4=$(filter_ip4_address $(dig +short +timeout=2 +tries=2 $noidnout -t a "$1" 2>/dev/null | awk '/^[0-9]/ { print $1 }')) fi if [[ -z "$ip4" ]] && "$HAS_HOST"; then @@ -20271,7 +20274,6 @@ get_aaaa_record() { local saved_openssl_conf="$OPENSSL_CONF" local noidnout="" - "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" [[ "$NODNS" == none ]] && return 0 # if no DNS lookup was instructed, leave here OPENSSL_CONF="" # see https://github.com/drwetter/testssl.sh/issues/134 if is_ipv6addr "$1"; then @@ -20292,6 +20294,7 @@ get_aaaa_record() { fatal "Local hostname given but no 'avahi-resolve' or 'dig' available." $ERR_DNSBIN fi elif "$HAS_DIG"; then + "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" ip6=$(filter_ip6_address $(dig +short +timeout=2 +tries=2 $noidnout -t aaaa "$1" 2>/dev/null | awk '/^[0-9]/ { print $1 }')) elif "$HAS_HOST"; then ip6=$(filter_ip6_address $(host -t aaaa "$1" | awk '/address/ { print $NF }')) @@ -20317,7 +20320,6 @@ get_caa_rr_record() { local all_caa="" local noidnout="" - "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" [[ -n "$NODNS" ]] && return 0 # if minimum DNS lookup was instructed, leave here # if there's a type257 record there are two output formats here, mostly depending on age of distribution @@ -20328,6 +20330,7 @@ get_caa_rr_record() { # caa_property then has key/value pairs, see https://tools.ietf.org/html/rfc6844#section-3 OPENSSL_CONF="" if "$HAS_DIG"; then + "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" raw_caa="$(dig +short +timeout=3 +tries=3 $noidnout type257 "$1" 2>/dev/null | awk '{ print $1" "$2" "$3 }')" # empty if no CAA record elif "$HAS_DRILL"; then @@ -20393,12 +20396,12 @@ get_mx_record() { local saved_openssl_conf="$OPENSSL_CONF" local noidnout="" - "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" OPENSSL_CONF="" # see https://github.com/drwetter/testssl.sh/issues/134 # we need the last two columns here if "$HAS_HOST"; then mx="$(host -t MX "$1" 2>/dev/null | awk '/is handled by/ { print $(NF-1), $NF }')" elif "$HAS_DIG"; then + "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" mx="$(dig +short $noidnout -t MX "$1" 2>/dev/null | awk '/^[0-9]/ { print $1" "$2 }')" elif "$HAS_DRILL"; then mx="$(drill mx $1 | awk '/IN[ \t]MX[ \t]+/ { print $(NF-1), $NF }')" @@ -20409,7 +20412,7 @@ get_mx_record() { fatal "No dig, host, drill or nslookup" $ERR_DNSBIN fi OPENSSL_CONF="$saved_openssl_conf" - echo "$mx" + safe_echo "$mx" } # arg1: domain / hostname. Returned will be the TXT record as a string which can be multilined @@ -20420,12 +20423,12 @@ get_txt_record() { local saved_openssl_conf="$OPENSSL_CONF" local noidnout="" - "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" OPENSSL_CONF="" # see https://github.com/drwetter/testssl.sh/issues/134 # we need the last two columns here and strip any remaining double quotes later if "$HAS_HOST"; then record="$(host -t TXT "$1" 2>/dev/null | grep 'descriptive text')" elif "$HAS_DIG"; then + "$HAS_DIG_NOIDNOUT" && noidnout="+noidnout" record="$(dig +short $noidnout -t TXT "$1" 2>/dev/null)" record="${record% from*}" elif "$HAS_DRILL"; then From eee6e77201dac914829f33a714742d8e55c9342d Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Sat, 23 Jan 2021 14:24:11 +0100 Subject: [PATCH 09/11] https instead of http, also in the comment --- testssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 84f1d2401..256da0eea 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7515,7 +7515,7 @@ sub_mta_sts() { pr_svrty_low "invalid" # Append to every element a string, here: '|'. n would work as well but we'd need $spaces. # Catch is also for the last, so maybe we should avoid arrays - # Hint is from http://web.archive.org/web/20101114051536/http://codesnippets.joyent.com/posts/show/1826 + # Hint is from https://web.archive.org/web/20101114051536/http://codesnippets.joyent.com/posts/show/1826 failreason_mtasts_rec=( "${failreason_mtasts_rec[@]/%/ | }" ) fileout "${jsonID}_txtrecord" "LOW" "invalid _mta-sts TXT record $mta_sts_record $(printf '%s | ' "{failreason_mtasts_rec[@]}")" outln " _mta-sts TXT record '${mta_sts_record}'\n${spaces} $(printf '%s' "${failreason_mtasts_rec[@]}")" From c832e8b12a42bd0f9093f7927ce4f66edd6c2684 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Tue, 26 Jan 2021 14:24:31 +0100 Subject: [PATCH 10/11] Screen output correction (quotes), logic error when MTA-STS record was missing fixed --- testssl.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testssl.sh b/testssl.sh index 256da0eea..74ce6395e 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7421,6 +7421,7 @@ sub_mta_sts() { if [[ -z "$mta_sts_record" ]]; then failreason_mtasts_rec="no record" + mta_sts_record_ok=false else if [[ $(count_char_occurence "$mta_sts_record" ';') -ne 2 ]]; then failreason_mtasts_rec+=("number of ; should be 2") @@ -7442,6 +7443,7 @@ sub_mta_sts() { fi fi fi +set +x policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$domain\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect mta-sts.$domain:443 $PROXY -servername mta-sts.$domain") 2>$ERRFILE)" # here also the openssl return val needs to be checked @@ -7504,7 +7506,7 @@ sub_mta_sts() { # now the verdicts if "$mta_sts_record_ok"; then pr_svrty_good "valid" - outln " _mta-sts TXT record \'$mta_sts_record\'" + outln " _mta-sts TXT record '$mta_sts_record'" # quotes! fileout "${jsonID}_txtrecord" "OK" "valid _mta-sts TXT record $mta_sts_record" elif [[ -z "$mta_sts_record" ]]; then From 3fa9b16982335484aac4da1cb4e36bd3a89bb925 Mon Sep 17 00:00:00 2001 From: Dirk Wetter Date: Mon, 10 May 2021 11:44:29 +0200 Subject: [PATCH 11/11] Just commit old work ... just comments and minor corrections in terminal output --- testssl.sh | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/testssl.sh b/testssl.sh index 74ce6395e..951fc01a6 100755 --- a/testssl.sh +++ b/testssl.sh @@ -7362,6 +7362,7 @@ tls_time() { } # rfc8461, rfc8460 +# sub_mta_sts() { local mta_sts_record="" local policy="" @@ -7387,7 +7388,7 @@ sub_mta_sts() { if [[ "${CMDLINE[@]}" =~ \ --mx\ ]]; then domain="$URI" elif [[ fqdnparts -eq 2 ]] && [[ "$NODE" == ${URI%:*} ]]; then - # remove the port an check whether bot are the same when there's no subdomain + # remove the port and check whether bot are the same when there's no subdomain domain="$NODE" else # What's left now is a sub.domain.tld or sub.sub.domain.tld or ... @@ -7443,10 +7444,11 @@ sub_mta_sts() { fi fi fi -set +x policy="$(safe_echo "GET /.well-known/mta-sts.txt HTTP/1.1\r\nHost: mta-sts.$domain\r\nUser-Agent: $useragent\r\nAccept-Encoding: identity\r\nAccept: text/*\r\nConnection: Close\r\n\r\n" | $OPENSSL s_client $(s_client_options "-quiet -ign_eof -connect mta-sts.$domain:443 $PROXY -servername mta-sts.$domain") 2>$ERRFILE)" - # here also the openssl return val needs to be checked + # echo "${PIPESTATUS[0]} ${PIPESTATUS[1]} ${PIPESTATUS[2]}" + # set -o pipefail? --> https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another + # here also the openssl return val needs to be checked policy="$(print_after_blankline "$policy")" # echo "$policy"; echo @@ -7463,7 +7465,7 @@ set +x fi done - # we use at most 10 spaces. ToDo: check with RFC wrt to the format of the policy + #TODO: check with RFC wrt to the format of the policy if "$policy_ok"; then if [[ ! "$policy" =~ version:[\ ]+STSv1 ]]; then failreason_policy+=("version should be STSv1 ") @@ -7527,13 +7529,13 @@ set +x if "$policy_ok"; then if [[ $policy_mode == testing ]]; then out "\"none\" is a valid policy but why are you using it?" - fileout "${jsonID}_policy" "INFO" "none is valid but not a helpful policy at https://mta-sts.$domain/.well-known/mta-sts.txt" + fileout "${jsonID}_policy" "INFO" "none is valid but not a helpful policy at https://mta-sts.$domain/.well-known/mta-sts.txt" elif [[ $policy_mode == testing ]]; then out "valid but not enforced" - fileout "${jsonID}_policy" "INFO" "valid but not enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" + fileout "${jsonID}_policy" "INFO" "valid but not enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" else pr_svrty_good "valid and enforced" - fileout "${jsonID}_policy" "OK" "valid and enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" + fileout "${jsonID}_policy" "OK" "valid and enforced policy at https://mta-sts.$domain/.well-known/mta-sts.txt" fi outln " policy https://mta-sts.$domain/.well-known/mta-sts.txt" elif [[ -z "$policy" ]]; then @@ -7550,11 +7552,11 @@ set +x out "$spaces" if "$smtp_tls_record_ok"; then - outln "found (optional) TLS RPT TXT record '$smtp_tls_record'" + outln "found optional TLS RPT TXT record '$smtp_tls_record'" fileout "${jsonID}_tlsrpt" "INFO" "optional TLS-RPT TXT record '$smtp_tls_record'" else outln "No TLS RPT record" - fileout "${jsonID}_tlsrpt" "INFO" "no or invalid (optional) TLS RPT record '$smtp_tls_record'" + fileout "${jsonID}_tlsrpt" "INFO" "no or invalid optional TLS RPT record '$smtp_tls_record'" fi return 0