From f3cfb53546cca0cbea60beda12087cf6dd06afae Mon Sep 17 00:00:00 2001 From: David Cooper Date: Fri, 31 Aug 2018 13:14:28 -0400 Subject: [PATCH 1/5] Checking for DH groups in run_pfs() For cipher suites that use ephemeral DH groups, run_pfs() currently only displays information about the group(s) used if the server complies with RFC 7919. In the case of TLSv1.3 this is appropriate, since server can only use the values from this RFC and only if they are offered by the client in the supported_groups extension. For TLSv1.2 and earlier, however, servers are free to use whatever DH group they want, but run_pfs() only provides information about the group the server uses if the server complies with RFC 7919. (The information is, however, provided by run_logjam()). However, so far no servers comply with RFC 7919's requirement to refuse to negotiate a TLS_DHE cipher if the supported groups extension is present, included DH groups, but none that are supported by the server. There is also reason to believe that this will not change: https://www.ietf.org/mail-archive/web/tls/current/msg26378.html. So, this PR proposes to change the way that run_pfs() searches for DH groups for TLSv1.2 and earlier. (Note that run_pfs() only checks for TLSv1.2 or earlier if the $EXPERIMENTAL flag is set to true.) First, it removes the test to see if the server will reject a ClientHello that only specifies TLS_DHE cipher suites if it includes a supported_groups extension that only specifies an unrecognized DH group. Instead, if the server supports TLS_DHE cipher suites (at TLSv1.2 or earlier) and the $EXPERIMENTAL flag is true, it will try to find out what group(s) the server uses. Second, it will report the group(s) found even if the server uses a group that does not come from RFC 7919. The result is that if the server supports selecting groups from the supported_groups extension, it will print all of the groups that the server supports. If the server ignores the supported_groups extension and always uses the same group, it will print essentially the same information as is already printed by run_logjam(). One discrepancy, however, is that this code use pr_dh_quality() to determine how good a DH group is, based on the length of the prime, and pr_dh_quality() has differs from run_logjam() in terms of how it rates groups based on the lengths of their primes. --- testssl.sh | 64 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/testssl.sh b/testssl.sh index e924e87..1ea1de0 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8435,6 +8435,9 @@ run_pfs() { local -i nr_supported_ciphers=0 nr_curves=0 nr_ossl_curves=0 i j low high local pfs_ciphers curves_offered="" curves_to_test temp local len1 len2 curve_found + local key_bitstring dh_p + local -i lineno_matched len_dh_p + local common_primes_file="$TESTSSL_INSTALL_DIR/etc/common-primes.txt" local has_dh_bits="$HAS_DH_BITS" local using_sockets=true local jsonID="PFS" @@ -8786,7 +8789,7 @@ run_pfs() { fi fi if "$using_sockets" && ( "$pfs_tls13_offered" || ( "$ffdhe_offered" && "$EXPERIMENTAL" ) ); then - # find out what groups from RFC 7919 are supported. + # find out what groups are supported. nr_curves=0 for curve in "${ffdhe_groups_output[@]}"; do supported_curve[nr_curves]=false @@ -8795,17 +8798,13 @@ run_pfs() { protos_to_try="" "$pfs_tls13_offered" && protos_to_try="04" if "$ffdhe_offered" && "$EXPERIMENTAL"; then - # Check to see whether RFC 7919 is supported (see Section 4 of RFC 7919) - tls_sockets "03" "${ffdhe_cipher_list_hex:2}, 00,ff" "ephemeralkey" "00, 0a, 00, 04, 00, 02, 01, fb" - sclient_success=$? - if [[ $sclient_success -ne 0 ]] && [[ $sclient_success -ne 2 ]]; then - if "$pfs_tls13_offered"; then - protos_to_try="04 03" - else - protos_to_try="03" - fi + if "$pfs_tls13_offered"; then + protos_to_try="04 03" + else + protos_to_try="03" fi fi + curve_found="" for proto in $protos_to_try; do while true; do curves_to_test="" @@ -8833,10 +8832,49 @@ run_pfs() { for (( i=0; i < nr_curves; i++ )); do "${supported_curve[i]}" && curves_offered+="${ffdhe_groups_output[i]} " done + curves_offered="$(strip_trailing_space "$curves_offered")" + if "$ffdhe_offered" && "$EXPERIMENTAL" && [[ -z "$curves_offered" ]] && [[ -z "$curve_found" ]]; then + # Some servers will fail if the supported_groups extension is present. + tls_sockets "03" "${ffdhe_cipher_list_hex:2}, 00,ff" "ephemeralkey" + sclient_success=$? + if [[ $sclient_success -eq 0 ]] || [[ $sclient_success -eq 2 ]]; then + temp=$(awk -F': ' '/^Server Temp Key/ { print $2 }' "$TEMPDIR/$NODEIP.parse_tls_serverhello.txt") + curve_found="${temp#*, }" + curve_found="${curve_found%%,*}" + fi + fi + if [[ -z "$curves_offered" ]] && [[ -n "$curve_found" ]] && [[ -s "$common_primes_file" ]]; then + # The server is not using one of the groups from RFC 7919. + key_bitstring="$(awk '/-----BEGIN PUBLIC KEY/,/-----END PUBLIC KEY/ { print $0 }' $TEMPDIR/$NODEIP.parse_tls_serverhello.txt)" + dh_p="$($OPENSSL pkey -pubin -text -noout 2>>$ERRFILE <<< "$key_bitstring" | awk '/prime:/,/generator:/' | egrep -v "prime|generator")" + dh_p="$(strip_spaces "$(colon_to_spaces "$(newline_to_spaces "$dh_p")")")" + [[ "${dh_p:0:2}" == "00" ]] && dh_p="${dh_p:2}" + len_dh_p=$((4*${#dh_p})) + debugme tmln_out "len(dh_p): $len_dh_p | dh_p: $dh_p" + dh_p="$(toupper "$dh_p")" + # In the previous line of the match is bascially the hint we want to echo + # the most elegant thing to get the previous line [ awk '/regex/ { print x }; { x=$0 }' ] doesn't work with gawk + lineno_matched=$(grep -n "$dh_p" "$common_primes_file" 2>/dev/null | awk -F':' '{ print $1 }') + if [[ "$lineno_matched" -ne 0 ]]; then + curves_offered="$(awk "NR == $lineno_matched-1" "$common_primes_file" | awk -F'"' '{ print $2 }')" + else + curves_offered="unrecognized group" + fi + fi if [[ -n "$curves_offered" ]]; then - pr_bold " RFC 7919 DH groups offered: " - outln "$curves_offered" - fileout "RFC7919_DH_groups" "INFO" "$curves_offered" + if [[ ! "$curves_offered" =~ ffdhe ]] || [[ ! "$curves_offered" =~ \ ]]; then + pr_bold " Finite field group offered: " + else + pr_bold " Finite field groups offered: " + fi + if [[ "$curves_offered" =~ ffdhe ]]; then + pr_svrty_good "$curves_offered" + else + out "$curves_offered (" + pr_dh_quality "$len_dh_p" "$len_dh_p bits" + out ")" + fi + fileout "DHE_groups" "INFO" "$curves_offered" fi fi outln From 93116f38e788c57b01d99e1d1878ffdd9f6df327 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Thu, 4 Oct 2018 13:19:28 -0400 Subject: [PATCH 2/5] Send DHE quality to fileout() In run_pfs(), when information about the finite field groups offered is printed, the color used is based on the length of the key. This information should also be conveyed to fileout() in the severity parameter. --- testssl.sh | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index 1ea1de0..f1d2220 100755 --- a/testssl.sh +++ b/testssl.sh @@ -5419,20 +5419,28 @@ run_cipherlists() { return $ret } +# The return value is an indicator of the quality of the DH key length in $1: +# 1 = pr_svrty_critical, 2 = pr_svrty_high, 3 = pr_svrty_medium, 4 = pr_svrty_low +# 5 = neither good nor bad, 6 = pr_svrty_good, 7 = pr_svrty_best pr_dh_quality() { local bits="$1" local string="$2" if [[ "$bits" -le 600 ]]; then pr_svrty_critical "$string" + return 1 elif [[ "$bits" -le 800 ]]; then pr_svrty_high "$string" + return 2 elif [[ "$bits" -le 1280 ]]; then pr_svrty_medium "$string" + return 3 elif [[ "$bits" -ge 2048 ]]; then pr_svrty_good "$string" + return 6 else out "$string" + return 5 fi } @@ -8435,8 +8443,8 @@ run_pfs() { local -i nr_supported_ciphers=0 nr_curves=0 nr_ossl_curves=0 i j low high local pfs_ciphers curves_offered="" curves_to_test temp local len1 len2 curve_found - local key_bitstring dh_p - local -i lineno_matched len_dh_p + local key_bitstring dh_p quality_str + local -i lineno_matched len_dh_p quality local common_primes_file="$TESTSSL_INSTALL_DIR/etc/common-primes.txt" local has_dh_bits="$HAS_DH_BITS" local using_sockets=true @@ -8869,12 +8877,26 @@ run_pfs() { fi if [[ "$curves_offered" =~ ffdhe ]]; then pr_svrty_good "$curves_offered" + quality=6 else out "$curves_offered (" pr_dh_quality "$len_dh_p" "$len_dh_p bits" + quality=$? out ")" fi - fileout "DHE_groups" "INFO" "$curves_offered" + case "$quality" in + 1) quality_str="CRITICAL" ;; + 2) quality_str="HIGH" ;; + 3) quality_str="MEDIUM" ;; + 4) quality_str="LOW" ;; + 5) quality_str="INFO" ;; + 6|7) quality_str="OK" ;; + esac + if [[ "$curves_offered" == "unrecognized group" ]]; then + fileout "DHE_groups" "$quality_str" "$curves_offered ($len_dh_p bits)" + else + fileout "DHE_groups" "$quality_str" "$curves_offered" + fi fi fi outln From 1fddbc3b4449cefb03ed2d0d2a335b3846060c62 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 17 Oct 2018 09:56:54 -0400 Subject: [PATCH 3/5] Use get_common_prime() This commit changes the code in run_pfs() to use the get_common_prime() helper function. --- testssl.sh | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/testssl.sh b/testssl.sh index f1d2220..64d0def 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8443,9 +8443,8 @@ run_pfs() { local -i nr_supported_ciphers=0 nr_curves=0 nr_ossl_curves=0 i j low high local pfs_ciphers curves_offered="" curves_to_test temp local len1 len2 curve_found - local key_bitstring dh_p quality_str - local -i lineno_matched len_dh_p quality - local common_primes_file="$TESTSSL_INSTALL_DIR/etc/common-primes.txt" + local key_bitstring quality_str + local -i len_dh_p quality local has_dh_bits="$HAS_DH_BITS" local using_sockets=true local jsonID="PFS" @@ -8851,23 +8850,11 @@ run_pfs() { curve_found="${curve_found%%,*}" fi fi - if [[ -z "$curves_offered" ]] && [[ -n "$curve_found" ]] && [[ -s "$common_primes_file" ]]; then + if [[ -z "$curves_offered" ]] && [[ -n "$curve_found" ]]; then # The server is not using one of the groups from RFC 7919. key_bitstring="$(awk '/-----BEGIN PUBLIC KEY/,/-----END PUBLIC KEY/ { print $0 }' $TEMPDIR/$NODEIP.parse_tls_serverhello.txt)" - dh_p="$($OPENSSL pkey -pubin -text -noout 2>>$ERRFILE <<< "$key_bitstring" | awk '/prime:/,/generator:/' | egrep -v "prime|generator")" - dh_p="$(strip_spaces "$(colon_to_spaces "$(newline_to_spaces "$dh_p")")")" - [[ "${dh_p:0:2}" == "00" ]] && dh_p="${dh_p:2}" - len_dh_p=$((4*${#dh_p})) - debugme tmln_out "len(dh_p): $len_dh_p | dh_p: $dh_p" - dh_p="$(toupper "$dh_p")" - # In the previous line of the match is bascially the hint we want to echo - # the most elegant thing to get the previous line [ awk '/regex/ { print x }; { x=$0 }' ] doesn't work with gawk - lineno_matched=$(grep -n "$dh_p" "$common_primes_file" 2>/dev/null | awk -F':' '{ print $1 }') - if [[ "$lineno_matched" -ne 0 ]]; then - curves_offered="$(awk "NR == $lineno_matched-1" "$common_primes_file" | awk -F'"' '{ print $2 }')" - else - curves_offered="unrecognized group" - fi + get_common_prime "$jsonID" "$key_bitstring" "" + [[ $? -eq 0 ]] && curves_offered="$DH_GROUP_OFFERED" && len_dh_p=$DH_GROUP_LEN_P fi if [[ -n "$curves_offered" ]]; then if [[ ! "$curves_offered" =~ ffdhe ]] || [[ ! "$curves_offered" =~ \ ]]; then @@ -8892,7 +8879,7 @@ run_pfs() { 5) quality_str="INFO" ;; 6|7) quality_str="OK" ;; esac - if [[ "$curves_offered" == "unrecognized group" ]]; then + if [[ "$curves_offered" =~ Unknown ]]; then fileout "DHE_groups" "$quality_str" "$curves_offered ($len_dh_p bits)" else fileout "DHE_groups" "$quality_str" "$curves_offered" From df6870a92b5664cb9534f59da3d6e695ac974d27 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 17 Oct 2018 11:10:11 -0400 Subject: [PATCH 4/5] Use results from run_pfs() in run_logjam() If run_pfs() has already determined the DH group(s) offered by the server, then use this in run_logjam() rather than querying the server again. --- testssl.sh | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/testssl.sh b/testssl.sh index 64d0def..b739417 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8855,6 +8855,9 @@ run_pfs() { key_bitstring="$(awk '/-----BEGIN PUBLIC KEY/,/-----END PUBLIC KEY/ { print $0 }' $TEMPDIR/$NODEIP.parse_tls_serverhello.txt)" get_common_prime "$jsonID" "$key_bitstring" "" [[ $? -eq 0 ]] && curves_offered="$DH_GROUP_OFFERED" && len_dh_p=$DH_GROUP_LEN_P + elif [[ -n "$curves_offered" ]]; then + DH_GROUP_OFFERED="$curves_offered" + [[ ! "$curves_offered" =~ \ ]] && DH_GROUP_LEN_P="${DH_GROUP_OFFERED#ffdhe}" fi if [[ -n "$curves_offered" ]]; then if [[ ! "$curves_offered" =~ ffdhe ]] || [[ ! "$curves_offered" =~ \ ]]; then @@ -13925,7 +13928,11 @@ out_common_prime() { local cwe="$3" # now size matters -- i.e. the bit size ;-) - if [[ $DH_GROUP_LEN_P -le 512 ]]; then + [[ "$DH_GROUP_OFFERED" == ffdhe* ]] && [[ ! "$DH_GROUP_OFFERED" =~ \ ]] && DH_GROUP_OFFERED="RFC7919/$DH_GROUP_OFFERED" + if [[ "$DH_GROUP_OFFERED" =~ ffdhe ]] && [[ "$DH_GROUP_OFFERED" =~ \ ]]; then + out "common primes detected: "; pr_italic "$DH_GROUP_OFFERED" + fileout "$jsonID2" "INFO" "$DH_GROUP_OFFERED" "$cve" "$cwe" + elif [[ $DH_GROUP_LEN_P -le 512 ]]; then pr_svrty_critical "VULNERABLE (NOT ok):"; out " common prime "; pr_italic "$DH_GROUP_OFFERED"; out " detected ($DH_GROUP_LEN_P bits)" fileout "$jsonID2" "CRITICAL" "$DH_GROUP_OFFERED" "$cve" "$cwe" elif [[ $DH_GROUP_LEN_P -le 1024 ]]; then @@ -14016,7 +14023,13 @@ run_logjam() { fi # Try all ciphers that use an ephemeral DH key. If successful, check whether the key uses a weak prime. - if "$using_sockets"; then + if [[ -n "$DH_GROUP_OFFERED" ]]; then + if [[ "$DH_GROUP_OFFERED" =~ Unknown ]]; then + subret=0 # no common DH key detected + else + subret=1 # known prime/DH key + fi + elif "$using_sockets"; then tls_sockets "03" "$all_dh_ciphers, 00,ff" "ephemeralkey" sclient_success=$? if [[ $sclient_success -eq 0 ]] || [[ $sclient_success -eq 2 ]]; then @@ -14047,10 +14060,8 @@ run_logjam() { fi fi - # FIXME: The following logic comes too late if we have already a check for DH groups in run_pfs() - # now the final test for common primes, -n "$DH_GROUP_OFFERED" indicates we did this already if [[ -n "$key_bitstring" ]]; then - if [[ -z "$DH_GROUP_OFFERED" ]]; then + if [[ -z "$DH_GROUP_OFFERED" ]]; then get_common_prime "$jsonID2" "$key_bitstring" "$spaces" ret=$? # no common primes file would be ret=1 --> we should treat that some place else before fi @@ -14059,7 +14070,7 @@ run_logjam() { else subret=1 # known prime/DH key fi - else + elif [[ -z "$DH_GROUP_OFFERED" ]]; then subret=3 fi From e0021c041616215fa5221e0cab07e649d702af85 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Wed, 24 Oct 2018 11:12:56 -0400 Subject: [PATCH 5/5] Only update DH_GROUP_OFFERED for non-TLSv1.3 ciphers run_logjam() is only related to TLSv1.2 and earlier ciphers. So, run_pfs() should only update $DH_GROUP_OFFERED if a DH group was found using a non-TLSv1.3 cipher. On the other side, if run_logjam() happened to have been run first, and it found an ffdhe cipher, then there is no need for run_pfs() to test for it. --- testssl.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index b739417..c78b6c7 100755 --- a/testssl.sh +++ b/testssl.sh @@ -8800,6 +8800,7 @@ run_pfs() { nr_curves=0 for curve in "${ffdhe_groups_output[@]}"; do supported_curve[nr_curves]=false + [[ "$DH_GROUP_OFFERED" =~ "$curve" ]] && supported_curve[nr_curves]=true nr_curves+=1 done protos_to_try="" @@ -8827,6 +8828,10 @@ run_pfs() { temp=$(awk -F': ' '/^Server Temp Key/ { print $2 }' "$TEMPDIR/$NODEIP.parse_tls_serverhello.txt") curve_found="${temp#*, }" curve_found="${curve_found%%,*}" + if [[ "$proto" == "03" ]] && [[ -z "$DH_GROUP_OFFERED" ]] && [[ "$curve_found" =~ ffdhe ]]; then + DH_GROUP_OFFERED="RFC7919/$curve_found" + DH_GROUP_LEN_P="${curve_found#ffdhe}" + fi [[ ! "$curve_found" =~ ffdhe ]] && break for (( i=0; i < nr_curves; i++ )); do ! "${supported_curve[i]}" && [[ "${ffdhe_groups_output[i]}" == "$curve_found" ]] && break @@ -8855,9 +8860,6 @@ run_pfs() { key_bitstring="$(awk '/-----BEGIN PUBLIC KEY/,/-----END PUBLIC KEY/ { print $0 }' $TEMPDIR/$NODEIP.parse_tls_serverhello.txt)" get_common_prime "$jsonID" "$key_bitstring" "" [[ $? -eq 0 ]] && curves_offered="$DH_GROUP_OFFERED" && len_dh_p=$DH_GROUP_LEN_P - elif [[ -n "$curves_offered" ]]; then - DH_GROUP_OFFERED="$curves_offered" - [[ ! "$curves_offered" =~ \ ]] && DH_GROUP_LEN_P="${DH_GROUP_OFFERED#ffdhe}" fi if [[ -n "$curves_offered" ]]; then if [[ ! "$curves_offered" =~ ffdhe ]] || [[ ! "$curves_offered" =~ \ ]]; then