Support cipher suite selection by cyang1 · Pull Request #167 · rust-native-tls/rust-native-tls

cyang1 · 2020-05-11T19:28:23Z

The exported API is implemented as an explicit whitelist with a consistent default across platforms based on the allowed ciphersuites in rust-openssl, which is itself based on the list from Python.

The implementation for each backend is roughly what you'd expect.

For OpenSSL, it constructs a cartesian product of the selected algorithms, joined together with +.
For Schannel, it just maps the chosen algorithms to the correct Schannel equivalent.
For the Security Framework, it doesn't look like there's any structure behind the ciphersuite constants or more user-friendly APIs for interacting with them, so I've combed through the ciphersuite list and manually compiled lists of cipher suites that use each algorithm.

SHA512 is not included as a possible hash algorithm because it appears to be only available in the Security Framework.

Closes #4.

sfackler · 2020-05-12T00:02:01Z

src/imp/openssl.rs

+        cipher_suites.bulk_encryption.iter().map(|alg| match alg {
+            TlsBulkEncryptionAlgorithm::Aes128 => "AES128",
+            TlsBulkEncryptionAlgorithm::Aes256 => "AES256",
+            TlsBulkEncryptionAlgorithm::Des => "DES",


Do we really need to offer historical curiosities like DES and RC2?

I lean towards not being opinionated about what is available. I can remove them if that's your preference, but I can imagine that someone might have some use for them for connecting to old, insecure servers.

sfackler · 2020-05-12T00:02:29Z

src/imp/openssl.rs

+            TlsBulkEncryptionAlgorithm::Des => "DES",
+            TlsBulkEncryptionAlgorithm::Rc2 => "RC2",
+            TlsBulkEncryptionAlgorithm::Rc4 => "RC4",
+            TlsBulkEncryptionAlgorithm::TripleDes => "3DES",


This should include ChaCha20 IMO.

That would be nice, but AFAICT Schannel doesn't have any ChaCha20 ciphersuites, and I've just included the GCD of the available algorithms across the different backends.

I think it's fine that schannel doesn't support it yet - many versions of OpenSSL don't either. As long as it's paired with another algorithm that is supported it'll work.

Sure, what's the experience you had in mind, though, if the user chooses to only specify ChaCha20 on Windows? As currently written, nothing here returns a Result, and from some testing, it looks passing no bulk encryption algorithms to the supported_algorithms call in Schannel will result in Windows choosing some (most likely system-wide) defaults.

On Windows 10 19h1 where I'm trying this out, it looks like it chose to allow AES-128, AES-256, and 3DES, which seems consistent with https://docs.microsoft.com/en-us/windows/win32/secauthn/tls-cipher-suites-in-windows-10-v1903, but probably not ideal as far as the behavior of this API.

It would need to fail to handshake, yeah. If that's not easily doable though we can leave it out for now.

sfackler · 2020-05-12T00:04:06Z

src/imp/openssl.rs

+    cipher_suite_strings = cartesian_product(
+        cipher_suite_strings,
+        cipher_suites.signature.iter().map(|alg| match alg {
+            TlsSignatureAlgorithm::Dss => "aDSS",


Like below, do we really need DSS?

sfackler · 2020-05-12T00:05:10Z

src/imp/security_framework.rs

+    CipherSuite::TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA,
+    CipherSuite::TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA,
+    CipherSuite::TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA,
+    // CipherSuite::TLS_PSK_WITH_3DES_EDE_CBC_SHA,


Why are some of these commented out?

All the commented out ones are PSK. I can remove them, they're mostly left in as I was checking my work while manually compiling the lists, and for anyone else checking it over to see that it wasn't forgotten, but explicitly excluded.

sfackler · 2020-05-12T00:07:33Z

src/imp/security_framework.rs

+        .iter()
+        .flat_map(key_exchange_alg_to_cipher_suites)
+        .copied();
+    let rest: &[HashSet<_>] = &[


It might be a bit better to load the key exchange ciphers into a hashset and then iterate through the the signature, bulk_encryption, etc ciphers to progressively remove bad ciphers from the set, and then load the remainder into the returned vec.

That's a good point, I can change this.

Made an attempt at this, although I'm not entirely convinced that it's better that it was before.

sfackler · 2020-05-12T00:09:05Z

src/test.rs

+    #[test]
+    fn badssl_cipher_suites_no_rsa() {
+        let builder = p!(TlsConnector::builder().supported_cipher_suites(
+            // Oddly, on Windows, allowing RSA key exchange, but not RSA signature algorithms still


This is the kind of edge case I'm worried about in exposing this level of control over cipher suites.

FWIW, RSA is the only one that's valid in two different parts of the ciphersuite specification, so it's the only one this can happen to. Did some searching and it seems like it might be an expected quirk of Schannel that RSA_KEYX implies RSA_SIGN, although all the discussions that I found were on tangential topics (1, 2).

cyang1 · 2021-03-26T02:06:29Z

Picking this up again. Added some handling for AES-GCM ciphersuites for OpenSSL specifically, as they aren't included in AES128/AES256, and specifying AESGCM doesn't allow you to choose the bitwidth.

FWIW, I've been using a locally patched version of this for a while (with that change), and have had no problems so far.

rneswold · 2021-08-02T16:37:55Z

Can this be merged? I'm getting flagged by my site security department because I'm advertising 3DES. It would be nice to be able to restrict the ciphers.

(I didn't see any way, in native_tls, to specify the cipher suite.)

k3d3 · 2021-12-21T04:27:42Z

I'm interested in this getting merged as well. For the insecure ciphers, perhaps it would be a good idea to call them DANGER_DES or something similar, to keep with the trend of prepending danger to things that most people would not want to do?

ta3pks · 2022-04-09T09:26:11Z

any updates on this ?

kornelski · 2026-02-18T12:30:30Z

If you'd like to revive it, I'd be happy to merge a minimal version that only allows upgrading security from the baseline.
Don't allow any ancient cruft like MD5, SHA1, DES, or RC4.

sfackler reviewed May 12, 2020

View reviewed changes

cyang1 force-pushed the ciphersuites branch from f2be4ea to 3d20834 Compare March 26, 2021 02:00

Support cipher suite selection

3e9c1b5

cyang1 force-pushed the ciphersuites branch from 3d20834 to 3e9c1b5 Compare March 26, 2021 02:03

stevedoyle mentioned this pull request Mar 26, 2024

TLS (Transport Layer Security) WebAssembly/wasi-sockets#100

Closed

kornelski marked this pull request as draft February 18, 2026 12:30

Comments

Conversation

cyang1 commented May 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyang1 commented Mar 26, 2021

Uh oh!

rneswold commented Aug 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k3d3 commented Dec 21, 2021

Uh oh!

ta3pks commented Apr 9, 2022

Uh oh!

kornelski commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rneswold commented Aug 2, 2021 •

edited

Loading