Remove support for weak TLS cipher suites

[jsparber]

Is your feature request related to a problem? Please describe. We support all possible TLS ciper suites (with the default http client implementation) even those considered insecure.

Describe the solution you'd like We should limit it to the ciphers that are supported by TLSv1.3:

TLS_AES_256_GCM_SHA384 TLS_CHACHA20_POLY1305_SHA256 TLS_AES_128_GCM_SHA256 TLS_AES_128_CCM_8_SHA256 TLS_AES_128_CCM_SHA256

Additional context For context Fractal (and some if it's deps) is undergoing a security audit. They recommend to limit the ciphers to the above list.

[poljar]

This can be achieved by configuring reqwest: https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.min_tls_version

[jsparber]

A value of tls::Version::TLS_1_3 will cause an error with the native-tls/default-tls backend. This does not mean the version isn’t supported, just that it can’t be set as a minimum due to technical limitations.

Looks like we can't set the min tls version to 1.3 when we arn't using rust-tls.

[daenney]

Limiting to TLSv1.3 might not be very desirable. There's a huge variety of Matrix servers out in the wild, deployed on different platforms with different supported TLS versions in their libraries, http reverse proxies etc.

The "intermediate" configuration as present on https://ssl-config.mozilla.org/ tends to be a good default to follow, striking a more reasonable balance between security and backwards compatibility. It's probably also close to what server admins might have configured in practice (but without TLSv1.3 if that's not available). It allows for TLSv1.2 but disables known problematic ciphers. That might also be a more reasonable default for matrix-rust-sdk or applications in the Matrix ecosystem.

If the rustls backend is used, this is already the case since it has a list of hardcoded safe TLSv1.2 ciphers.

[MTRNord]

I agree with the above comment. While it would be nice I don't think that the adoption of tls1.3 is high enough yet to justify this. At the very least there should be a way to enable tls1.2 even if the default is 1.3.

[jplatte]

In any case, we already support customizing the reqwest client and I don't think there's anything more we can do without replacing reqwest with something else entirely.

[jplatte]

Oh, I had gotten a notification about this but the mobile app didn't show the actual activity re. project tracking. Just checked in a browser. I guess I might be wrong then.

[disconnect3d]

Sorry, the activity was made by a comment I sent but I deleted it because it was about Rust/reqwest documentation and not your project. (I complained that they don’t explain the reasons why you can’t set Tls 1.3 as minimum)

On Fri, 18 Aug 2023 at 19:12, Jonas Platte @.***> wrote:

Reopened #611 https://github.com/matrix-org/matrix-rust-sdk/issues/611.

— Reply to this email directly, view it on GitHub https://github.com/matrix-org/matrix-rust-sdk/issues/611#event-10131999548, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACMLWCUXF3RNRLT3A4PSCB3XV6O7HANCNFSM5ULYHHEA . You are receiving this because you commented.Message ID: @.***>

[Hywan]

We can close the issue again I guess :-).