Update rustls to 0.23

[Ralith]

Required for Quinn to update past rustls 0.21, so a release soon after would be appreciated. Also relaxes default feature requirements on rustls.

[djc]

I think I tried this and it won't work on macOS...

[Ralith]

This is difficult to test without CI working by default.

[complexspaces]

@Ralith I added you with the Triage role on the repository (it grants minimal issue and PR permissions). I can remove it once you've merged this and GitHub doesn't see you as an "untrusted contributor" here anymore but hopefully it doesn't bug you for now.

[Ralith]

Android CI failing:

> Task :rustls-platform-verifier:connectedDebugAndroidTest

[DDMLIB]: An unexpected packet was received before the handshake.
[DDMLIB]: An unexpected packet was received before the handshake.
[DDMLIB]: An unexpected packet was received before the handshake.
[DDMLIB]: An unexpected packet was received before the handshake.

> Task :rustls-platform-verifier:connectedDebugAndroidTest
Starting 3 tests on test(AVD) - 9

org.rustls.platformverifier.CertificateVerifierTests > runRealWorldTestSuite[test(AVD) - 9] FAILED 
    org.junit.ComparisonFailure: A test failed. Check the logs above for Rust panics. expected:<[success]> but was:<[test failed!]>
    at org.junit.Assert.assertEquals(Assert.java:115)
Tests on test(AVD) - 9 failed: There was 1 failure(s).

Not sure how to interpret that. There aren't any panics that I can see. "An unexpected packet was received" sounds like a rustls-side issue. There seems to be some sort of test result artifact that might have details, but it doesn't look human-readable.

[zh-jq-b]

It seems to be the same issue as reported here https://github.com/rustls/rustls-platform-verifier/issues/68.

[cpu]

"An unexpected packet was received" sounds like a rustls-side issue.

I think these lines are unrelated noise. The same output has been happening on passing builds in the past. "ddmlib" looks like it's specific to the Android emulator<->Host debug bridge and so I think likely to be inconsequential to the Rustls tests.

It seems to be the same issue as reported here https://github.com/rustls/rustls-platform-verifier/issues/68.

Yes, it is.

[Ralith]

If the only CI failure is #68, can we consider this unblocked?

[cpu]

If the only CI failure is https://github.com/rustls/rustls-platform-verifier/issues/68, can we consider this unblocked?

I don't know how @complexspaces feels but I think we wouldn't want to cut a release until main is passing CI so even if we merged this it would be blocked on #68.

I've made some progress and think I have a fix. Will update #68 shortly

[cpu]

I've made some progress and think I have a fix. Will update https://github.com/rustls/rustls-platform-verifier/issues/68 shortly

I think cherry-picking in 98725337daa56ebe98db7877670b3886884539e8 from https://github.com/rustls/rustls-platform-verifier/pull/71 will sort the mac task out.

[Ralith]

macos is hitting:

rustls default CryptoProvider not set

which is from the expect I added on CryptoProvider::get_default() calls. This stopped happening on other platforms when the rustls ring feature was enabled. I'd expect to have to call install_default either everywhere or nowhere; having that requirement vary across platforms is baffling.

[complexspaces]

That is extraordinarily strange. I tried comparing the output of cargo tree --edges features between macOS and Linux and nothing jumps out as "broken" in rustls direct dependency tree. The only difference is that there's no rustls-webpki.

[Ralith]

Perhaps it's a test case ordering/global state issue. rustls tests suggest that None is the expected return value even when the feature is enabled:

https://github.com/rustls/rustls/blob/0398ac50fe18521da038aa39b55eec6c83450523/rustls/tests/process_provider.rs#L47-L58

I'll try setting defaults explicitly.

[complexspaces]

It looks like its back to the Android failures 🎉 If you rebase on main Ci should be green at least. I merged #71 a few minutes ago.

[Ralith]

I've added a commit that makes calls to CryptoProvider::get_default() lazy, which should ensure a good user experience without hard-coding a specific provider or breaking the API, because rustls will not attempt to verify a certificate until after it's been fully configured. The rare case of users who were previously passing in a provider explicitly and never touching the default implicitly might still see a panic, but can easily resolve it.

Long-term, I think it would be better for rustls to expose CryptoProvider::get_default_or_install_from_crate_features directly, since its use in e.g. WebPkiServerVerifier seems to be a tacit admission that it's the most user-friendly way for library code to get a provider. However, this change will let rustls-platform-verifier update and unblock downstream users like Quinn immediately without regressing usability.

[Lewiscowles1986]

Please merge