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

Relax recommendation on cipher order #1311

Closed
exploide opened this issue Aug 30, 2019 · 18 comments
Closed

Relax recommendation on cipher order #1311

exploide opened this issue Aug 30, 2019 · 18 comments

Comments

@exploide
Copy link

Currently, testssl prints a big red warning when a server has no server preferred cipher order.

Mozilla recently relaxed their recommendations regarding cipher order. If only strong cipher suites are supported anyway, why not deciding according to the client's preferences. Maybe it's a phone that wants to optimize for performance on low hardware. Additionally, often browsers are better maintained than web servers updated. So the client's preferences are probably not bad.

So Mozilla decided to prefer server order only for the "old" compatibility configuration.

There were several discussions about this, but this was the outcome so far.
https://github.com/mozilla/ssl-config-generator

I propose that testssl also relaxes its recommendation regarding cipher order.

To be more precise: Either print the warning just in yellow instead of red, or make it dependent on the supported TLS versions and algorithms. E.g. when no cipher concerns are present, make the order setting also no concern.

@drwetter
Copy link
Owner

Mozilla recently relaxed their recommendations regarding cipher order.

you have a pointer?

Please keep in mind that testssl.sh tests not only HTTP services but almost everything. Realistically nowdays browsers are not a big concern, but I can tell you that there are really bad clients out there.

If only strong cipher suites are supported anyway, why not deciding according to the client's preferences.

yes, if . Means, it would need a test for every cipher upfront.

@exploide
Copy link
Author

exploide commented Aug 31, 2019

Thanks for your reply.

you have a pointer?

See https://ssl-config.mozilla.org/ for the outcome of the popular Mozilla config generator. Note that server cipher order is off for modern and intermediate and only on for old.

Discussion is hard to follow because spread over several issues. E.g. mozilla/ssl-config-generator#48, mozilla/ssl-config-generator#34 and starting at mozilla/server-side-tls#178 (comment)

but I can tell you that there are really bad clients out there

I agree. I do not propose to remove the cipher order rating altogether. But maybe if it would be only written in yellow. Or if it is a bit more explanatory in which cases it is important.

Means, it would need a test for every cipher upfront.

Not necessarily. It might be sufficient to utilize the already existing cipher "category" rating from the top. I.e. only warn about cipher order when already warned about NULL, ANON, EXPORT, or LOW.

@drwetter
Copy link
Owner

drwetter commented Sep 1, 2019

Thanks for the pointers.

From a server side when you already know what you configure, things might seem easier. Albeit still I think to set server side preference should be best practice in TLS <= 1.2.

For testssl.sh in 3.0 I am hesitant to make bigger changes, i.e. introducing dependencies between checks. The implementation problem with your (in theory good) suggestion is that not necessarily the cipher category has been run already. And also if this is the case somehow the worst ciphers from the client side need to be deducted and this needs to be color encoded.

OTOH to relax the color code without the knowledge what bad ciphers are configured might reflect 90% of the use cases but I wouldn't like the 10% to get no suggestion like "please look at the cipher order". This is where the config generator has an easier decision.

So at the moment the results may seem overly pessimistic but that doesn't hurt as much.

@drwetter drwetter added this to the 3.1dev milestone Sep 13, 2019
@gzurowski
Copy link

👍 Thanks for bringing this up @exploide.

From a server side when you already know what you configure, things might seem easier. Albeit still I think to set server side preference should be best practice in TLS <= 1.2.

The Mozilla recommendation is currently to only allow client preferred cipher order for TLS 1.2 and 1.3 ("intermediate" and "modern" configuration profiles).

It would be great if the original request for relaxing the recommendation on cipher orders could be incorporated into future testssl.sh versions.

@dcooper16
Copy link
Contributor

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.

Hi Dirk,

I haven't had time to work on this issue at all, but the current output looks something like this:

 Testing server's cipher preferences 

 Has server cipher order?     no (NOT ok)
 Negotiated protocol          TLSv1.3
 Negotiated cipher            TLS_AES_256_GCM_SHA384, 253 bit ECDH (X25519) (limited sense as client will pick)
 Cipher per protocol

Hexcode  Cipher Suite Name (OpenSSL)       KeyExch.   Encryption  Bits     Cipher Suite Name (IANA/RFC)
-----------------------------------------------------------------------------------------------------------------------------
SSLv2 (listed by strength)
 - 
SSLv3 (no server order, thus listed by strength)
...                      
TLSv1 (no server order, thus listed by strength)
...                      
TLSv1.2 (no server order, thus listed by strength)
...                      
TLSv1.3 (no server order, thus listed by strength)
...

In order for the "Has server cipher order?" rating to depend on which ciphers the server supports, it seems that we have two options:

  • We could collect and store all of the information about which ciphers are supported with each protocol and then print all of the information at the end (fairly big change to the code); or
  • We could reverse the order of the output so that "Cipher per protocol" is printed first and the three other items come afterwards (see below). This would make the implementation relatively easy, as we could just change ciphers_by_strength() to report the rating of the strongest cipher that the server supports and the weakest cipher that the server supports. (A server-enforced cipher order isn't needed if the server only supports strong ciphers, but it seems that it wouldn't help if the server only supports weak ciphers.)

Would it make sense to output the "Cipher per protocol" information first in order to make implementing this feature easier?

 Testing server's cipher preferences 

 Cipher per protocol

Hexcode  Cipher Suite Name (OpenSSL)       KeyExch.   Encryption  Bits     Cipher Suite Name (IANA/RFC)
-----------------------------------------------------------------------------------------------------------------------------
SSLv2 (listed by strength)
 - 
SSLv3 (no server order, thus listed by strength)
...                      
TLSv1 (no server order, thus listed by strength)
...                      
TLSv1.2 (no server order, thus listed by strength)
...                      
TLSv1.3 (no server order, thus listed by strength)
...


 Has server cipher order?     no (NOT ok)
 Negotiated protocol          TLSv1.3
 Negotiated cipher            TLS_AES_256_GCM_SHA384, 253 bit ECDH (X25519) (limited sense as client will pick)

@drwetter
Copy link
Owner

Good suggestion in general. Off the top of my head (as you indicated) the second option sounds better. In addition I would suggest to remove the line "Cipher per protocol".

I would keep the statements in the round brackets after each protocol. For penalizing (or not) the "Has server cipher oder" best probably is to determine the weakest cipher and label it accordingly to that.

For STARTTLS servoces: not sure yet.

Still then I don't know how the remaining 3 lines will look like. Let me have a look. Hope I have internet access the next days.

@dcooper16
Copy link
Contributor

Still then I don't know how the remaining 3 lines will look like. Let me have a look. Hope I have internet access the next days.

No problem. I don't have plans to start working on this at the moment, just thinking about how it might be done.

For penalizing (or not) the "Has server cipher oder" best probably is to determine the weakest cipher and label it accordingly to that.

It seems that this could penalize a server unnecessarily. At the moment, pr_cipher_quality() and run_cipherlists() rate non-AEAD ciphers as 4 or lower. However, AEAD ciphers cannot be used with TLS 1.1 or earlier. So, if a server supports TLS 1.1 and does not enforce a cipher order, run_server_preference() would issue a warning about not enforcing a cipher order even if every cipher supported with TLS 1.1 has a rating of 4. Maybe this isn't a big deal, since testssl.sh is already warning about the use of TLS 1.1 and about the use of CBC ciphers. But, it would be a bit misleading since simply configuring the server to enforce a cipher order wouldn't make it any more secure.

@drwetter
Copy link
Owner

drwetter commented May 25, 2020

Sorry about the delay.

For STARTTLS: I thought in my previous comment that any encryption is probably better than opportunistic encryption. But as we didn't label that anywhere, there should be no reason to start here. 1

As far as the general server's cipher preferences are concerned, here's how it I think it could look like:
Screenshot_20200525_121630

The question is whether we should leave it like that or whether we want to remove redundant information -- basically the three summary lines (now) in the bottom are (almost) in the information before or (negotiated cipher/protocol) have been retrieved under special conditions and not the client handshake. Or whether at least whether we should explain better under which circumstances the Negotiated protocol + cipher have been retrieved.

1 And BTW: for some daemons like postfix you can configure postfix not to accept plaintext connections (downgrade scenario of MiTM) and validate the certificate. Also see e.g. MTA-STS, see #1073, and TLS-RPT.

dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Jul 6, 2020
This commit separates pr_cipher_quality() into two functions, one that returns the quality of a cipher as a numeric rating (get_cipher_quality()) and one that prints a cipher based on its quality (pr_cipher_quality()). This separation allows get_cipher_quality() to be used to determine how good a cipher is without having to print anything. Having this ability would be helpful in implementing the changes suggested in drwetter#1311.
@chrisdlangton
Copy link

chrisdlangton commented Dec 28, 2020

I don't know if you should explain under which circumstances the negotiated protocol + cipher had been retrieved, but rather log out verbose information about outbound (tests) performed to squash a bunch of feature requests current and future. This way users who understand this information can get the context themselves but most users wouldn't care or find it useful

@drwetter
Copy link
Owner

NOt sure I understand rather log out verbose information about outbound (tests) performed

Suggestions:

  • The line 'Has server cipher order?' should be omitted. It's redundant information
  • 'Negotiated protocol' + 'Negotiated cipher' should be either completely removed or put into one line saying ~Best Protocol / cipher negotiated, supposed when there was a server cipher order. The latter though are too much chars and it's kind of redundant too.

Opinons?

dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Mar 22, 2022
This commit reorders the output of run_server_preference() as discussed in drwetter#1311.
drwetter pushed a commit that referenced this issue Apr 1, 2022
This commit reorders the output of run_server_preference() as discussed in #1311.
@drwetter
Copy link
Owner

drwetter commented Jun 2, 2022

Reminder: when we implement this, we should work out before what the best approach is in order to save time. One idea would be to use the standard cipher category, unfortunately that would be protocol independent. But at least when there's no deprecated cipher it can be used at a starting point.

An alternative would be to enumerate ciphers per protocol completely and when each protocol is done, start the output and penalize or not the missing cipher order along with the protocol.

dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Jun 6, 2022
This commit modifies ciphers_by_strength() and run_server_preference() so that the message indicating that ciphers are listed by strength is not printed until the list of supported ciphers has been determined. This is in support of drwetter#1311, as it will allow the message to be modified based on the set of supported ciphers.

This commit also modifies both ciphers_by_strength() and cipher_pref_check() so that the order in which ciphers are listed (by strength or server preference) is not printed if the server does not support the protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Jun 6, 2022
This commit modifies ciphers_by_strength() and run_server_preference() so that the message indicating that ciphers are listed by strength is not printed until the list of supported ciphers has been determined. This is in support of drwetter#1311, as it will allow the message to be modified based on the set of supported ciphers.

This commit also modifies both ciphers_by_strength() and cipher_pref_check() so that the order in which ciphers are listed (by strength or server preference) is not printed if the server does not support the protocol.
@dcooper16
Copy link
Contributor

I just submitted PR #2194 to help address this issue. Now, with #2129 and #2194, no information about cipher order is printed or sent to fileout() until the complete list of supported ciphers is known.

My suggestion for the rating for each protocol when a server cipher order is not enforced would be as follows. Use get_cipher_quality() to get the rating for each supported cipher and then determine the difference between the cipher with the highest rating and the cipher with the lowest rating. A difference of 0 would result in no warning, as all supported ciphers are of the same quality (even if they are not good).

For the "Has server cipher order?" printed by run_server_preference(), we could get rid of it (if we add a fileout() for each protocol) or we could make the rating for this one be the lowest rating among all of the supported protocols for which an order is not enforced.

dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 7, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
@dcooper16 dcooper16 mentioned this issue Sep 7, 2022
@doggodanubus
Copy link

I can't offer much help with this. As a user I'd just want to know that it's not a critical issue if XYZ is done. Now that I am aware of it I'll quiet down. The level of coding to dance around this makes my head hurt. I wish I could do that well but I'm very poor at it.

Thanks for your efforts and thanks for the response.

dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 14, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 16, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 19, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 28, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Sep 29, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Oct 11, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Oct 13, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
dcooper16 added a commit to dcooper16/testssl.sh that referenced this issue Oct 19, 2022
This commit fixes drwetter#1311 by only rating the lack of a server-enforced ciper order negatively if there is a difference in the quality rating of the ciphers offered for a particular protocol.
drwetter added a commit that referenced this issue Oct 21, 2022
@jult
Copy link

jult commented Feb 27, 2023

Perhaps, you know, instead of giving users that warning and leaving them stranded with it; Just propose a proper cipher order, and relate that to the software detected (i.e. exim, dovecot, nginx etc.). All this discussion, 3 years ago and it still is not 'fixed' or improved.

@polarathene
Copy link
Contributor

polarathene commented Feb 27, 2023

@jult If you're only serving modern web clients where TLS 1.3 should always be available to your users, then server order won't matter.

If you serve TLS 1.2, if your cipher list is all AEAD / PFS compatible, IIRC the server order doesn't provide much benefit either. If you need a list, Mozilla has a generator and documents best practices around that. OWASP at least did too at some point.

This is pretty much discussed in the issue already.


All this discussion, 3 years ago and it still is not 'fixed' or improved.

It's not like there's been no activity for 3 years. If you read through the recent comments there was work related to improvements for this.

It was closed as fixed by the PR merged in Oct 2022. If you don't like the warning, then open a new issue about how that could be improved.

Personally I don't think it's the responsibility of testssl to attempt to detect software and output a server order for you. The purpose is to make you aware of anything that might warrant you to investigate further, these tools often act as guides, don't chase ratings for the sake of it.

If you need to enforce server order, go to the mozilla resource and reference a cipher list from them, and adapt your ordering based on that. If you don't understand any of it however, I'd advise to read an article on cipher suites so that you know what you're doing beyond copy / paste.

@jult
Copy link

jult commented Feb 27, 2023

Responsibility or not, it's the ONLY red warning anyone gets at this stage. So they'll all end up here, because it's also the only informative hit for this 'warning'. Basically, it's yet another alarmist nonsensical admin-fear-mongering featuritis idea for those who decide to 'test' their ssl config, usually mostly by mail-server admins. ALL or most default configs will end up giving you this red alarm. Maybe not make it so red? Or illustrate how totally unimportant or unalarming it is.

@polarathene
Copy link
Contributor

ALL or most default configs will end up giving you this red alarm. Maybe not make it so red? Or illustrate how totally unimportant or unalarming it is.

Open a new issue about it.

@drwetter
Copy link
Owner

@jult, as @polarathene said this should be no issue anymore, at least when you're using 3.2rcX.

Many mail servers have a good TLS config. If the default of a piece of mail server software is too relaxed, why don't you raise your voice there?

If it's not the preference alone you're complaining about: Mail servers might be different in real life as most of those species rather need to prefer availability (mail gets delivered) over confidentiality. A mail server admin should know this. We won't change a security tool to make those folks feel better.

Also, as @polarathene said: testssl's sole purpose at this moment is scanning. Same as other security tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants