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

Add Option for Ciphers to QUIC_CREDENTIAL_CONFIG #109

Closed
Tratcher opened this issue Feb 6, 2020 · 16 comments · Fixed by #1430
Closed

Add Option for Ciphers to QUIC_CREDENTIAL_CONFIG #109

Tratcher opened this issue Feb 6, 2020 · 16 comments · Fixed by #1430
Assignees
Labels
Area: API feature request A request for new functionality
Milestone

Comments

@Tratcher
Copy link
Member

Tratcher commented Feb 6, 2020

Compare to CipherSuitesPolicy.

The client and server need the ability in code to restrict which ciphers can be negotiated. Additionally, the server needs to be able to make this decision per connection, after receiving the Client Hello. E.g. HTTP/2 required negotiating from a restricted set of ciphers, but you wouldn't know if the connection was using HTTP/2 until the ALPN information was received.

Historically ciphers have only been configurable at the OS level and that's still true on Windows/Schannel. OpenSsl however provides the ability to specify a list of allowed ciphers in code that overrides the global settings.

This capability is useful when phasing in new ciphers or phasing out old ones. Doing so machine wide risks destabilizing too many applications at once, services want to update apps one at a time.

@nibanks nibanks added the feature request A request for new functionality label Feb 6, 2020
@nibanks
Copy link
Member

nibanks commented Feb 19, 2020

@Tratcher if you could provide a pointer to some example code that shows how you do this with OpenSSL already it would help us evaluate the cost of possibly implementing this.

@Tratcher
Copy link
Member Author

@bartonjs this one was also you?

@bartonjs
Copy link
Member

@Tratcher almost none of TLS is ever me, but I do know where things live 😄.

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_ciphersuites.html

Looking up the openssl cipher name from the ciphersuite is tedious, but trivial: https://github.com/dotnet/runtime/blob/0896dd8204d0942f15be1ea6f42b71aa3a363b01/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c#L383-L441

(It's complex because of the need to set SSL2-TLS1.2 ciphersuites via one method and TLS1.3+ via a different one)

@Tratcher
Copy link
Member Author

Ah, sorry about that. Who should I be blaming? 😁

@blowdart
Copy link

blowdart commented May 4, 2020

For this request? Probably me. We've seen problems in IOT devices where you can't configure the OS, which in turn has SSL2 and SSL3 enabled because security has not been high on their list, but then folks want to use them in environments that do care about security, like hospitals, and it's been painful to do (we've had customers shove nginx in front of .net core apps because nginx gives them the ability to configure cipher suites and protocols whereas .net core could not due to lack of apis).

@nibanks
Copy link
Member

nibanks commented Oct 22, 2020

Now that we have a specific configuration struct/interface for things like this, an optional parameter should be added to QUIC_CREDENTIAL_CONFIG to allow the app to chose the ciphers.

@nibanks nibanks changed the title Controlling the list of ciphers in code Add Option for Ciphers to QUIC_CREDENTIAL_CONFIG Oct 22, 2020
@nibanks
Copy link
Member

nibanks commented Oct 26, 2020

Looking into this a bit, Schannel requires us to pass a list of disabled CRYPTO_SETTINGS (see pDisabledCrypto in TLS_PARAMETERS). For pretty much every other TLS library, it's represented as an "allow list" of ciphers.

@Tratcher, @blowdart, @bartonjs, how do you guys handle this when calling Schannel directly give your current CipherSuitesPolicy design?

@nibanks
Copy link
Member

nibanks commented Oct 26, 2020

After talking with our TLS folks, I'm leaning towards simply aligning with their "disabled list" approach. So, the app would provide a (optional) list of disabled ciphers in the QUIC_CREDENTIAL_CONFIG and that'd simply be a pass-through QUIC to the TLS layer. With Schannel it would be directly passed down. For other TLS libraries (e.g. OpenSSL) we'd have to generate the appropriate "allow list" based on what QUIC supports minus the app-supplied disabled list.

@Tratcher
Copy link
Member Author

@Tratcher, @blowdart, @bartonjs, how do you guys handle this when calling Schannel directly give your current CipherSuitesPolicy design?

CipherSuitesPolicy didn't handle this on Windows last I checked, Windows didn't support allow or disallow lists until recently.

@nibanks
Copy link
Member

nibanks commented Oct 26, 2020

@Tratcher, then how necessary is this feature request if it's not even used on Windows currently?

@Tratcher
Copy link
Member Author

I've asked @wfurt to chime in here as he owns CipherSuitesPolicy.

@wfurt
Copy link
Member

wfurt commented Oct 26, 2020

CRYPTO_SETTINGS as what we can work with. The CipherSuitesPolicy has list of particular combinations e.g. handshake algorithm/strength + hash and symetric key/strength. What the CRYPTO_SETTINGS offers is ability to restrict particular algorithm in isolation and have min and max for key size. The list is also restricted by SCH_CRED_MAX_SUPPORTED_CRYPTO_SETTINGS (16) so in each session, we can describe up to 16 "rules" e.g. way less than what the current CipherSuitesPolicy offers.

With 5.0, SslStream already use TLS_PARAMETERS as that was only one way how to get TLS 1.3 but I did not figure out how to use CRYPTO_SETTINGS to implement CipherSuitesPolicy.

One option to proceed would be exposing new API to somewhat mimic what Windows offer.
On Linux Mac, we can use CipherSuitesPolicy implementation e.g. for example filer out suites where AES is not within min/Max range.

@nibanks
Copy link
Member

nibanks commented Oct 26, 2020

Hmm.. So we definitely don't want to expose two different interfaces for MsQuic. So, I have two questions for you guys:

  1. Is this feature absolutely necessary? I get the impression from the TLS folks that they really don't like overwriting the defaults involved here, as they put considerable time/effort into those.
  2. If this feature is necessary, is exposing a blocked/disallow list from the MsQuic layer acceptable (for all platforms)?

@bartonjs
Copy link
Member

Is this feature absolutely necessary? I get the impression from the TLS folks that they really don't like overwriting the defaults involved here, as they put considerable time/effort into those.

Given that we improved our "TLS from config" story with .NET 5.0 on Linux, I'd suggest no. (If someone wants a per-app difference they can use a app-specific OpenSSL configuration and specify it with the OPENSSL_CONF environment variable).

If this feature is necessary, is exposing a blocked/disallow list from the MsQuic layer acceptable (for all platforms)?

We had the same positive list vs negative list discussion with the SChannel team, and we decided on a positive list because

  • Almost everywhere that we've seen talk about ciphersuites uses an "only" description.
  • It better matched the model of the providers that supported per-session configuration

It really comes down to a belief that all new ciphersuites would be acceptable. But someone could put up an RFC for extending the ciphersuite for TLS 1.2 to include more RC2/SHA-1 ciphersuites as part of helping phase TLS 1.1/1.0 out of some legacy devices that they feel they can't really add good TLS 1.2 ciphers to.... should those be accepted by someone who has said they wanted to override defaults? We said no, SChannel apparently says yes. (Though they'd say that's covered by their current API model of just rejecting SHA-1 and RC2, which is why they reject segments instead of combinations).

So, personally, I'd say that if you're going to have the feature that it should be the "only" model (an allow list) instead of the "not" model. Sure, "only" gets further reduced by the OS not being able to do some of the particular ciphersuites (e.g. the library doesn't do ChaCha-Poly), but that's OK, it never did anything other than what was permitted... it's similar to the other side of the communication not supporting some of the things in the allow list. But I'd also say that it's probably something that you can do without, and that you stick with the general MS recommendation of "just use the system configuration", and that TLS stuff be managed at the system / domain / whatever level.

@wfurt
Copy link
Member

wfurt commented Oct 26, 2020

One more aspect I forget to mention is that on Windows, once can do the cipher section per ALPN e.g. you can have different rules if you negotiation H/2 vs HTTP/1.1. That allows to follow H/2 RFC and eliminate what they see as weak ciphers but maintain them for HTTP/1.1 clients. I don't think we can do it on other platforms. As @bartonjs mentions, Windows simply took whole different route.

@nibanks nibanks added this to the Cobalt (1.1.0) milestone Dec 3, 2020
@nibanks nibanks assigned ThadHouse and anrossi and unassigned nibanks, anrossi and ThadHouse Feb 24, 2021
@nibanks
Copy link
Member

nibanks commented Feb 24, 2021

Let's just add a simple "allow" list and be done with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: API feature request A request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants