Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistencies in run_protocols() #1343

Open
dcooper16 opened this issue Oct 8, 2019 · 1 comment

Comments

@dcooper16
Copy link
Contributor

commented Oct 8, 2019

In run_protocols() there are a few of inconsistencies in how results are reported and rated when a ClientHello results in a downgraded connection.

Downgrade from TLS 1 ClientHello:

     pr_svrty_medium "not offered"
     add_tls_offered tls1 no
     if [[ "$DETECTED_TLS_VERSION" == 0300 ]]; then
          [[ $DEBUG -ge 1 ]] && tm_out " -- downgraded"
          outln
          fileout "$jsonID" "MEDIUM" "not offered, and downgraded to SSL"
     elif ...

As is done in the fileout and as is done with downgrades from TLS 1.3 or TLS 1.2, the printed output should indicate that the connection was downgraded. Otherwise, it might seem confusing that it is considered bad that TLS 1 is "not offered". Also, offering SSL 3 in response to an SSL 3 ClientHello is rated as HIGH, so it seems that downgrading to SSL 3 should rated as least as bad (either HIGH or CRITICAL).

Downgrade from TLS 1.1 ClientHello:

     out "not offered"
     add_tls_offered tls1_1 no
     if [[ "$DETECTED_TLS_VERSION" == "$latest_supported" ]]; then
          [[ $DEBUG -ge 1 ]] && tm_out " -- downgraded"
          outln
          fileout "$jsonID" "CRITICAL" "TLSv1.1 is not offered, and downgraded to a weaker protocol"
     elif

Again, the printed output should note that the connection was downgraded (not just in debug mode). It should also be printed using pr_svrty_critical rather than out, if the fileout rating is CRITICAL. Also, it seems downgrading from TLS 1 to SSL 3 should be rated at least as bad a downgrading from TLS 1.1 to TLS 1. So, either downgrading from TLS 1.1 to TLS 1 should be rated less bad (MEDIUM), or downgrading from TLS 1 to SSL 3 should be rated as CRITICAL. [Given that downgrading from TLS 1.2 is rated MEDIUM, downgrading from TLS 1.1 to TLS 1 shouldn't be rated as less bad than MEDIUM.]

I also noticed that, at least in the case of a TLS 1.2 ClientHello, the rating is different for a return value of 3 or 4 than for a return value of 1, even though all three seem to be a result of the server rejecting the connection.

Finally, there is an inconsistency in the the rating when a server rejects a TLS 1.3 or TLS 1.2 ClientHello.

If a server rejects a TLS 1.2 ClientHello, $latest_supported is empty, and $offers_tls13 is false, this means the server either doesn't support SSL/TLS or it is an SSLv2-only server. The same applies to a server that rejects a TLS 1.3 ClientHello if $latest_supported is empty. In the case of the TLS 1.2 ClientHello the rejection is rated MEDIUM, and in the case of the TLS 1.3 ClientHello the rejection is rated LOW. I'm not sure how such a server should be rated, but the ratings should be the same in both places.

@dcooper16

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

In addition to the above, the ratings for the "Negotiated protocol" in run_server_preference() should be aligned with the ratings in run_protocol(). The "Negotiated protocol" is the result of a TLS 1.3 (or TLS 1.2) ClientHello. Currently a negotiated protocol of TLS 1.1 is rated okay (outln) and a negotiated protocol of TLS 1 is rated good (prln_svrty_good). Both of these protocols are now deprecated in run_protocols() and downgrading to them is rated as MEDIUM (or worse), so it seems that it should be rated as MEDIUM (or worse) if TLS 1.1 or TLS 1 is the negotiated protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.