Windows 7 users broken by recent Github change

[retep998]

Issue was originally discovered in an unrelated issue: #5065

Due to a recent change on Github, TLS 1.0 and 1.1 are no longer supported: https://github.com/blog/2507-weak-cryptographic-standards-removed

Unfortunately Windows 7 WinHTTP by default only uses up to TLS 1.0, and because Cargo uses libgit2 which uses WinHTTP, this renders cargo effectively broken whenever it tries to fetch any git repo including the crates.io index.

PS [00:51:51] C:\Users\Arnavion\Desktop> cargo install https://github.com/alexcrichton/wasm-gc
    Updating registry `https://github.com/rust-lang/crates.io-index`
warning: spurious network error (2 tries remaining): unknown certificate check failure; class=Net (12); code=Certificate (-17)
warning: spurious network error (1 tries remaining): unknown certificate check failure; class=Net (12); code=Certificate (-17)
error: failed to fetch `https://github.com/rust-lang/crates.io-index`

Caused by:
  unknown certificate check failure; class=Net (12); code=Certificate (-17)

Fortunately this issue can be fixed by the user by ensuring they have a certain update and also editing their registry appropriately, as described here: https://support.microsoft.com/en-us/help/3140245/update-to-enable-tls-1-1-and-tls-1-2-as-a-default-secure-protocols-in

However users are not going to magically know what the solution is when they run into this problem, so we need to tell them everywhere possible. We need to spread awareness on this issue through the Rust blog, TWiR, Reddit, Twitter, and anywhere else that Rust has a social media presence.

Additionally we need to implement checks in Cargo and Rustup for users that are using Windows 7 and don't have the fix applied, telling the user very LOUDLY that they need to apply that fix, along with providing detailed instructions on how to fix it.

[Michael-F-Bryan]

However users are not going to magically know what the solution is when they run into this problem, so we need to tell them everywhere possible. We need to spread awareness on this issue through the Rust blog, TWiR, Reddit, Twitter, and anywhere else that Rust has a social media presence.

How easy is it to check the error type and try to see if a failure is due to this issue? If it's easy enough to detect we could even emit a helpful error message with a link to the relevant resources.

[aidanhs]

Tweeted at https://twitter.com/RustStatus/status/967081220197814272

[aturon]

@matklad @Eh2406 is this something either of you would be up for addressing?

[matklad]

@aturon not sure I can really fix it, because I don't have the relevant windows experience. However, here are some pointers to different pieces of code which might be useful:

The network errors in Cargo are handled here:

cargo/src/cargo/util/network.rs

Line 14 in e3949fb

git2::ErrorClass::Os => return true,

. I believe the suggestion by @Michael-F-Bryan would work: we need to compare ErrorClass and ErrorCode with those from the error report (12 and 17), and do something special.

However I am not sure that we can use just this code to correctly diagnose the situation. That is, we might give false positive for a genuinely invalid certificate? The relevant code from libgit is

https://github.com/libgit2/libgit2/blob/13c1bf0718363960c1867f35c9ce3ebc7bf74729/include/git2/errors.h#L45 (defenition of the error)

and

https://github.com/libgit2/libgit2/blob/408b16c1b06526f509fdb8997818d648d4d1ea1e/src/transports/winhttp.c#L263 (origin of the error for WinHttp).

I'll try to prepare a patch which checks for this specific combination of error code and OS and emits warning, but, as I've said, I am not sure that this won't give us false-positives :)

[setharnold]

Just curious, how will a cargo update that warns about this issue be distributed to the users that would benefit from it? Is it strictly for fresh installs?

Thanks

[matklad]

@setharnold most likely, it won't be :-)

I think we can and should backport this to current beta.

In theory, we might issue a Rust 1.24.1 as well.

[Eh2406]

Interestingly there may be a 1.24.1 for other reasons rust-lang/rust#48445

[retep998]

An interesting point is that in order to detect that we are on Windows 7 we have to ask Windows what version it is, but Windows will lie and only report at most the newest version we claim to support in our manifest. Since cargo doesn't currently ship with a manifest (the Rust team really needs to stop ignoring this Windows specific stuff), this might make things a bit difficult.

[jethrogb]

The way I read the linked Microsoft issue it's not really that WinHTTP on Windows 7 doesn't support TLS 1.2, it's just a matter of a wrong default configuration. Could we just patch libgit2 to do the right thing on Windows 7?

[retep998]

cc @alexcrichton Since you maintain the Rust libgit2 stuff, do you know if its possible to get libgit2 to internally configure WinHTTP correctly so we don't depend on the user having to do registry edits?

@jethrogb TLS 1.2 support for Windows 7 still requires a rather recent patch and so even if we do fix that, we should still keep this error message because some people use Windows 7 with automatic updates disabled.