schannel: fix user-set older TLS ciphers in Windows 10 & 11

[jay]

• If CURLOPT_TLS13_CIPHERS is not set then use CURLOPT_SSL_CIPHER_LIST ciphers instead.

Prior to this change libcurl would ignore older ciphers (TLS <= 1.2) in Windows 10 1809 and later. For example, if the user set anything via CURLOPT_SSL_CIPHER_LIST it was ignored in favor of CURLOPT_TLS13_CIPHERS even if the latter wasn't set.

Unresolved: CURLOPT_TLS13_CIPHERS and CURLOPT_SSL_CIPHER_LIST are still mutually exclusive for Schannel. I'm not sure how to solve this. Since this commit, if CURLOPT_TLS13_CIPHERS is set then SCH_CREDENTIALS is used to set banned ciphers based on what is missing from the user's TLS 1.3 cipher list; however if CURLOPT_TLS13_CIPHERS is not set then SCH_CREDENTIALS is used to set allowed TLS <= 1.2 ciphers, if any, set via CURLOPT_SSL_CIPHER_LIST. Can they be combined somehow into a single credentials struct?

Reported-by: zhihaoy@users.noreply.github.com

Fixes #10741
Closes #xxxx

---

SCH_CREDENTIALS is what we use if CURLOPT_TLS13_CIPHERS is set.
https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-sch_credentials

SCHANNEL_CRED is what we use if CURLOPT_SSL_CIPHER_LIST is set, or if neither is set.
https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-schannel_cred

[wyattoday]

Yeah, there's not an easy way to solve this problem. Well, there's not an easy and secure way to solve this problem. And I don't see it as a problem.

The easy way to solve this problem is if the user specifies any of the billion old cipher list, then "downgrade" them to using a max of TLS 1.2 (regardless of what they specify).

But then they're using an old, less secure, version of TLS. Yes, TLS 1.2 isn't deprecated yet, but give it a couple of years.

My view on this problem is this: if you want to use old insecure ciphers, then stick to old insecure versions of Windows. If you want to use secure versions of Windows you'll also use secure ciphers.

That's the safest option (leaving it as-is).

Insecure options include:

1. Disabling the TLS 1.3 code-path if they specify any old-style cipher from that list of a billion ciphers. (as I said above). The downside is they cannot use TLS 1.3 or newer. Which is a security issue.
2. A second option (and just as bad security-wise) is to do 1, but to show a warning that TLS 1.3 is available but won't be used because they specified old-style ciphers. This is bad because libCurl users won't see the warning, and curl.exe users will likely just ignore the warning.
3. Another option is only allowing old-style ciphers if the user explicitly sets the max TLS version to 1.2. This is a bad option too, but at least in this case the user is explicitly requesting old, soon-to-be deprecated, versions of TLS.

My opinion: this isn't a bug. If they want to use insecure ciphers they should use insecure versions of Windows too. Yes, this might annoy this 1 user (and maybe 1 or 2 others over the next few years), but it's a more secure option that's better for the health of the internet.

Or, if this is a bug, it's a documentation bug, not a code-bug. My suggested fix is to leave the logic as-is but document that specifying the old-style cipher list will only work if they're on old Windows. Otherwise, they need to specify the new style cipher list.

[jay]

Thanks. It's a bug because the user can't set any legacy ciphers in newer versions of Windows. I'm likely to go with 1 or 2.

[zhihaoy]

Insecure options include:

1. Disabling the TLS 1.3 code-path if they specify any old-style cipher from that list of a billion ciphers. (as I said above). The downside is they cannot use TLS 1.3 or newer. Which is a security issue.
2. A second option (and just as bad security-wise) is to do 1, but to show a warning that TLS 1.3 is available but won't be used because they specified old-style ciphers. This is bad because libCurl users won't see the warning, and curl.exe users will likely just ignore the warning.
3. Another option is only allowing old-style ciphers if the user explicitly sets the max TLS version to 1.2. This is a bad option too, but at least in this case the user is explicitly requesting old, soon-to-be deprecated, versions of TLS.

My customers expected 3, but I don't want to justify their expectations.

When the protocol version isn't specified, I tend to say that the semantics of CURLOPT_SSL_CIPHER_LIST should be "these ciphers are okay, but I'm fine if they are not used," like when you're negotiating with servers. I.e., if we have an opportunity of establishing a connection using a more secure protocol, it's fine to kick some ciphers out. I don't know the design of CURLOPT_SSL_CIPHER_LIST though.

[wyattoday]

@zhihaoy

When the protocol version isn't specified, I tend to say that the semantics of CURLOPT_SSL_CIPHER_LIST should be "these ciphers are okay, but I'm fine if they are not used," like when you're negotiating with servers. I.e., if we have an opportunity of establishing a connection using a more secure protocol, it's fine to kick some ciphers out.

Unfortunately, CURLOPT_SSL_CIPHER_LIST doesn't currently work like that (even before I added TLS 1.3 support to curl w/ schannel). Rather, it just takes your input and doesn't consider ejecting insecure options. Not providing a cipher list at all is the most secure option.

That's part of my reason why I think this is not a code-bug, but a documentation bug:

My opinion: this isn't a bug. If they want to use insecure ciphers they should use insecure versions of Windows too [...] it's a more secure option that's better for the health of the internet.

Or, if this is a bug, it's a documentation bug, not a code-bug. My suggested fix is to leave the logic as-is but document that specifying the old-style cipher list will only work if they're on old Windows. Otherwise, they need to specify the new style cipher list.

[zhihaoy]

My suggested fix is to leave the logic as-is but document that specifying the old-style cipher list will only work if they're on old Windows. Otherwise, they need to specify the new style cipher list.

Sounds like a user-hostile thing to do -- whether a potentially insecure option is honored now depends on your running environment. If outside Schannel, the behavior is the same (i.e., CURLOPT_SSL_CIPHER_LIST is ignored when using OpenSSL backend on newer versions of Windows), that would at least be consistent.

[zhihaoy]

I talked with my team recently about this issue. We still want a code change.

The point was that, by retaining this regression, we lose the potential to have control over the ciphers to enforce stricter rules and stronger ciphers on most of our users. We were previously unsure whether a change was a good thing because the feature we had, which was broken by this regression, happens to be unimportant for other reasons. libcurl is foolish-proof by default, of course. Still, the knobs are there for a broader range of audiences, and we hope to see those knobs have more predictable behaviors so that library users can develop their applications for the good.

[jay]

Please review the most recent commit:

schannel: fix user-set legacy algorithms in Windows 10 & 11

• If the user set a legacy algorithm list (CURLOPT_SSL_CIPHER_LIST) then use the SCHANNEL_CRED legacy structure to pass the list to Schannel.

• If the user set both a legacy algorithm list and a TLS 1.3 cipher list then abort.

Although MS doesn't document it, Schannel will not negotiate TLS 1.3 when SCHANNEL_CRED is used. That means setting a legacy algorithm list limits the user to earlier versions of TLS.

Prior to this change, since 8beff43 (precedes 7.85.0), libcurl would ignore legacy algorithms in Windows 10 1809 and later.

Reported-by: zhihaoy@users.noreply.github.com

Fixes #10741

[wyattoday]

@jay

I think the changes are good. However, I think a couple of errors should be added:

1. In the case the user explicitly request TLS 1.3 and they still provide old-style ciphers (and thus can't use TLS 1.3 with this change you implemented), an error needs to be shown:

   curl/lib/vtls/schannel.c

   Line 251 in 451ba54

   if(curlx_verify_windows_version(10, 0, 20348, PLATFORM_WINNT,

2. In the case TLS 1.3 is supported (but not explicitly requested), and old-style ciphers are requested, a warning should be shown.

   curl/lib/vtls/schannel.c

   Line 229 in 451ba54

   ssl_version_max = CURL_SSLVERSION_MAX_TLSv1_3;

   . Yes, libcurl users won't see the warning, but curl-cli users will. It's not ideal, but better than nothing.

@zhihaoy

The point was that, by retaining this regression, we lose the potential to have control over the ciphers to enforce stricter rules and stronger ciphers on most of our users.

You don't lose that control, though. It just changes.

See: https://curl.se/docs/manpage.html#--tls13-ciphers
And: https://curl.se/libcurl/c/CURLOPT_TLS13_CIPHERS.html

If you're on modern Windows (Windows 11 and newer) use CURLOPT_TLS13_CIPHERS or --tls13-ciphers. If you're on deprecated-Windows (Windows 10, and all unsupported-by-ms versions like 7, 8, 8.2), use CURLOPT_SSL_CIPHER_LIST or --ciphers

All this is to say (or to reiterate) I don't believe this is a code-bug, but rather it's a documentation bug. Why? Because users like @zhihaoy will need to do what I said in the paragraph above no matter whether @jay's commit is merged or not. Except, with the commit merged, it could open the possibility of users inadvertently shooting themselves in the foot by using older TLS versions.

[jay]

Thanks to both of you for your feedback. I've landed the proposed change (#10746 (comment)) from last month which allows for the user to set legacy algorithms in Windows 10 and 11.

I think the changes are good. However, I think a couple of errors should be added

I added a warning, for verbose mode only, when an algorithm cipher list is used and TLS 1.3 is an enabled protocol:

* schannel: WARNING: This version of Schannel may negotiate a less-secure TLS version than TLS 1.3 because the user set an algorithm cipher list.

Though currently Schannel does not negotiate TLS 1.3 when SCHANNEL_CRED is used, that is undocumented MS behavior and may change.