drwetter / testssl.sh

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

[BUG / possible BUG] The difficulty with LUCKY13 / CVE-2013-0169 #2537

Open andy-maier opened 1 month ago

andy-maier commented 1 month ago

Before you open an issue please check which version you are running and whether it is the latest in stable / dev branch

I am running version testssl.sh 3.0.8 from https://testssl.sh/.

This issue is related to several others where LUCKY13 was suspected to be a false positive (#1181, #1011), or where people reported it without providing enough information.

Command line / docker command to reproduce

testssl.sh https://localhost:9291

Port 9291 is the Prometheus server port provided by the zhmc prometheus exporter (https://github.com/zhmcclient/zhmc-prometheus-exporter), configured to use HTTPS. This is a Python project that uses the https://github.com/prometheus/client_python Python library version 0.20.0 for the HTTPS server. That Python library uses the Python ssl module directly, with the default set of ciphers (see below for OpenSSL version). Note that the default set of ciphers at the Python level is a subset of what openssl ciphers reports.

Using the default set of ciphers, testssl.sh reports this:

LUCKY13 (CVE-2013-0169), experimental     potentially VULNERABLE, uses cipher block chaining (CBC) ciphers with TLS. Check patches

I then modified the code of the Prometheus client library to display the default set of ciphers, and that revealed 4 ciphers that are CBC ciphers (according to https://security.stackexchange.com/q/210912/136814):

When modifying the code of the Prometheus client library to remove these 4 ciphers from the Python SSL context, testssl.sh was happy w.r.t. LUCKY13:

LUCKY13 (CVE-2013-0169), experimental     not vulnerable (OK)

See here for the code that removes the ciphers: https://github.com/prometheus/client_python/pull/1051

Expected behavior

I think there are some difficulties for the average user with the current LUCKY13 vulnerability message:

Do you think it makes sense to change the following:

Your system:

Additional context

None

drwetter commented 4 weeks ago

Thanks for the lengthy explanation.

This particular check is admittedly a while back when it was written. CBC ciphers were hard to avoid by that time and there was this difficult to exploit weakness . IIRC all TLS stacks added some milli seconds to avoid exploitation of the timing issue. That's why I believe "potentially" should not be removed. Why do you think it should?

Yeah, we could add the ciphers.

aff commented 3 weeks ago

The LUCKY13 timing attack was mitigated in most TLS implementation a decade ago [1,2]. While able to detect potentially vulnerable CBC ciphers, testssl fails to differentiate between potentially and actually vulnerable implementations. This leads to a very high false positive rate in practice, which in turn reduces the trustworthiness of the otherwise excellent tool.

I propose to deprecate LUCKY13 testing altogether.

[1] https://web.archive.org/web/20200324101422/http://www.isg.rhul.ac.uk/tls/Lucky13.html [2] https://aws.amazon.com/blogs/security/s2n-and-lucky-13/

drwetter commented 2 weeks ago

@aff : yes, and in addition it was not practical to be exploited remotely. Probably it makes sense to remove that at version >3.2 . Or what do you mean by "deprecate"? Bigger changes will take place 3.3+

In the meantime: the finding is labeled as LOW severity finding and it says potentially and is referring to patches to be checked. If you have a more catchy description I am all ears ;-)

k01294 commented 2 days ago

@drwetter; By "deprecate" I mean remove. I will wait patiently if larger changes are coming. Thank you very much for your public service.