drwetter / testssl.sh

Testing TLS/SSL encryption anywhere on any port
https://testssl.sh
GNU General Public License v2.0
8.02k stars 1.03k forks source link

Drop the NOT ok flag for servers using client preferred secure ciphers. #1644

Open FozzieHi opened 4 years ago

FozzieHi commented 4 years ago

If a server presents secure ciphers it shouldn't matter if the client has choice over which one to use, this could bring performance benefits for clients using ChaCha if they do not have hardware accelerated AES.

For example, if a server had this TLS 1.2 cipher suite list:

TLS 1.2 (Client prefers)
- TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
- TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
- OLD_TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256

Then it shouldn't matter if the client chooses as all of the ciphers are secure.

magnuslarsen commented 4 years ago

For reference, Mozilla is arguing for this too: https://wiki.mozilla.org/Security/Server_Side_TLS

The cipher suites are all strong and so we allow the client to choose, as they will know best if they have support for hardware-accelerated AES

FozzieHi commented 4 years ago

It seems like this is implemented for TLS 1.3 but not for TLS 1.2, maybe because all of the TLS 1.3 cipher suites are secure?

dcooper16 commented 4 years ago

This issue has already been raised in #1311, and it has also been discussed in https://github.com/drwetter/testssl.sh/pull/1205#issuecomment-547114597.

FozzieHi commented 4 years ago

This issue has already been raised in #1311, and it has also been discussed in #1205 (comment).

That seems to be related to TLS 1.3 and has been implemented, I'm referring to TLS 1.2 but I'm happy to move there and mark this as a duplicate if needed.

dcooper16 commented 4 years ago

This issue has already been raised in #1311, and it has also been discussed in https://github.com/drwetter/testssl.sh/pull/1205#issuecomment-547114597. That seems to be related to TLS 1.3 and has been implemented, I'm referring to TLS 1.2 but I'm happy to move there and mark this as a duplicate if needed.

I believe you didn't read these very closely. Issue #1311 isn't specific to TLS 1.3 and it is still open. While PR #1205 has been merged and addresses the issue of a server that does not enforce a cipher order for TLS 1.3, but does for earlier protocols, the issue discussed in https://github.com/drwetter/testssl.sh/pull/1205#issuecomment-547114597 has not been implemented:

At the same time, if a server enforced a cipher order for TLS 1.3, but not for earlier protocol versions, that should be considered just as bad as a server that doesn't enforce a cipher order for any protocol version.

In the future, something more sophisticated could be done, where we first determine the full set of cipher suites supported for each protocol version, and then only mark the lack of a server-enforced cipher order as an issue if the server supports any "weak" ciphers. But, that would have to come after 3.0.

.

FozzieHi commented 4 years ago

Again, I'm happy to mark this as a duplicate and discuss steps in the issue you mentioned, although it does seem a fix has just been implemented for that single TLS 1.3 use case and the issue has been stale since then.

drwetter commented 4 years ago

Please again (see #1311) don't forget that testssl.sh isn't for browsers / HTTP only . There's load of badly configured STARTTLS and plain TLS servers (POP,SMTP, IMAP) out there which don't enforce a cipher order and at the same time allow really crappy ciphers. In my experience: The less common the service the more crappy the TLS/SSL setup. Those will disappear over time as modern distros just don't allow weak ciphers any more.

As David mentioned we could do something more sophisticated to take all that into account.

Until then one has to use a portion of common sense.

FozzieHi commented 4 years ago

Exactly what I’m proposing, have a list of secure ciphers that can be client preferred for TLS 1.2 and check if a server only has some or all of these cipher suites, if they have one not in this list then we should fail them for client preferred order.

Something like:

TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
OLD_TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256

I think we should leave out all CBC ciphers due to their inherent weaknesses with oracle padding vulnerabilities.