Update rustls to 0.23 and ring to 0.17

[getong]

I think you might also change the code in directory quinn/examples, like server.rs and client.rs.

rustls::Certificate has been replaced with CertificateDer from the new rustls-pki-types crate. Likewise, rustls::PrivateKey has been replaced with rustls_pki_types::PrivateKeyDer.

The code in examples does not compile.

[djc]

@getong there is a reason the PR is in draft state.

[PikuZheng]

In order to make ring support some architectures, I hope to upgrade ring to 0.17 as soon as possible

[cpu]

In order to make ring support some architectures, I hope to upgrade ring to 0.17 as soon as possible

@PikuZheng I believe ring 0.17 is already available with Quinn. The existing Quinn 0.10 release uses Rustls 0.21, which has upgraded to ring 0.17 as of v0.21.8. You don't need to wait for Quinn to have Rustls 0.22 support for this.

[PikuZheng]

In order to make ring support some architectures, I hope to upgrade ring to 0.17 as soon as possible

@PikuZheng I believe ring 0.17 is already available with Quinn. The existing Quinn 0.10 release uses Rustls 0.21, which has upgraded to ring 0.17 as of v0.21.8. You don't need to wait for Quinn to have Rustls 0.22 support for this.

├── quinn v0.10.2
│   ├── bytes v1.5.0
│   ├── pin-project-lite v0.2.13
│   ├── quinn-proto v0.10.6
│   │   ├── bytes v1.5.0
│   │   ├── rand v0.8.5
│   │   │   ├── libc v0.2.151
│   │   │   ├── rand_chacha v0.3.1
│   │   │   │   ├── ppv-lite86 v0.2.17
│   │   │   │   └── rand_core v0.6.4
│   │   │   │       └── getrandom v0.2.11 (*)
│   │   │   └── rand_core v0.6.4 (*)
│   │   ├── ring v0.16.20
│   │   │   ├── libc v0.2.151
│   │   │   ├── once_cell v1.19.0
│   │   │   ├── spin v0.5.2
│   │   │   └── untrusted v0.7.1
│   │   ├── rustc-hash v1.1.0
│   │   ├── rustls v0.21.10 (*)
│   │   ├── slab v0.4.9 (*)
│   │   ├── thiserror v1.0.52 (*)
│   │   ├── tinyvec v1.6.0
│   │   │   └── tinyvec_macros v0.1.1
│   │   └── tracing v0.1.40 (*)

anything i'm missing?

[cpu]

anything i'm missing?

No, this was my mistake. I forgot Quinn also has ring in its public API and didn't take the breaking change to update to 0.17 in the 0.10 release like Rustls did.

Sorry for the confusion. There is a change required in this repo to remove ring 0.16 from the requirements.

[djc]

We can now compile quinn-proto -- but there's still a bunch of test failures:

failures:
    tests::handshake_anti_deadlock_probe
    tests::large_initial
    tests::reject_missing_client_cert
    tests::server_can_send_3_inital_packets

If someone wants to lend a hand diagnosing/fixing these, that would be great. Interestingly, some of these seem to be triggering panics in the rustls deframer:

thread 'tests::server_can_send_3_inital_packets' panicked at /Users/djc/.cargo/git/checkouts/rustls-29bde2ca1adbde2d/aef485d/rustls/src/msgs/deframer.rs:490:8:
range end index 4468 out of range for slice of length 4096

[Ralith]

This patch to rustls fixes 3/4 failing tests:

diff --git a/rustls/src/msgs/deframer.rs b/rustls/src/msgs/deframer.rs
index 6da2d9d4..5d9d1fdd 100644
--- a/rustls/src/msgs/deframer.rs
+++ b/rustls/src/msgs/deframer.rs
@@ -253,7 +253,7 @@ impl MessageDeframer {
                 // We're joining a handshake message to the previous one here.
                 // Write it into the buffer and update the metadata.

-                DeframerBuffer::<QUIC>::copy(buffer, payload, meta.payload.end);
+                DeframerBuffer::<QUIC>::copy(buffer, payload, 0);
                 meta.message.end = end;
                 meta.payload.end += payload.len();

DeframerBuffer::<true>::copy automatically copies into the end of the payload, so passing meta.payload.end as the third argument double-counts the offset. This branch is probably only covered by tests which use multiple calls to read_hs to pass in a single handshake message.

That API seems a bit obscure. Maybe there's a clearer/better normalized way to express the abstraction? I don't fully understand why QUIC is a special case here to begin with, though.

[Ralith]

This fixes the remaining test:

diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs
index f809ba1c..1b3b28de 100644
--- a/quinn-proto/src/tests/mod.rs
+++ b/quinn-proto/src/tests/mod.rs
@@ -379,9 +379,15 @@ fn reject_missing_client_cert() {
     let key = PrivatePkcs8KeyDer::from(CERTIFICATE.serialize_private_key_der());
     let cert = CertificateDer::from(util::CERTIFICATE.serialize_der().unwrap());

+    let mut store = RootCertStore::empty();
+    // `WebPkiClientVerifier` requires a nonempty store, so we stick our own certificate into it
+    // because it's convenient.
+    store
+        .add(CERTIFICATE.serialize_der().unwrap().into())
+        .unwrap();
     let config = rustls::ServerConfig::builder()
         .with_client_cert_verifier(
-            WebPkiClientVerifier::builder(Arc::new(RootCertStore::empty()))
+            WebPkiClientVerifier::builder(Arc::new(store))
                 .build()
                 .unwrap(),
         )

[djc]

I think this is blocked on the rustls-platform-verifier upgrade to rustls 0.23 for now.

[gabrik]

Hi @djc , should I do a fork and then open a PR targeting this branch or should I ask for access? (I already tried the gh tool but I think it uses the same permission and usual git)

[djc]

Probably just a fork + PR targeting this branch?

[gabrik]

Done PR here: https://github.com/quinn-rs/quinn/pull/1808

[gabrik]

PR: https://github.com/quinn-rs/quinn/pull/1809

[Ralith]

Made a deep pass; looks like only some minor cosmetic issues left.

[djc]

@Ralith I think this is ready for another round -- hoping it will be the last one!

[djc]

Release progress is tracked in #1737.