-
Notifications
You must be signed in to change notification settings - Fork 39
Cargo: update to rustls 0.22, associated updates #42
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
Conversation
The main blocker that had this in draft status was the Reqwest compatibility unit test, but I think we should land #43 to address that. I've marked this branch ready for review and updated the PR description to remove the TODO notes. |
djc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Flipping this to draft - needs a rebase :-) |
Rebased and ready to go. |
Ah, another Rustls 0.21 vs 0.22 breakage snuck in. I'll fix shortly. |
Done. Besides fixing the webpki-roots version I had to tweak the As we know already, in FreeBSD CI the |
complexspaces
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good for the most part, I just have some nits/questions regarding the new types and crypto backends.
rustls-platform-verifier/src/tests/verification_real_world/mod.rs
Outdated
Show resolved
Hide resolved
complexspaces
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great now, thanks!
There's just the one small thread left about maybe abstracting some OnceCell boilerplate away, but its not a blocker.
For the time being, this branch continues to unconditionally use *ring* as the crypto provider. Follow-up work to expose this as a choice (e.g allowing aws-lc-rs as a provider) may be interesting. Deps: * updated rustls 0.21 -> 0.22.1 Linux deps: * rustls-native-certs 0.6 -> 0.7 * webpki 0.101 -> 0.102 Android deps: * webpki 0.101 -> 0.102 WASM32 deps: * webpki-roots 0.25 -> 0.26 Summary of breaking change updates: * We use rustls 0.22.1 in specific to benefit from the `pki_types` re-export, removing the need to add that as our own dep with matching version. * `ServerName`, `Certificate`, and `OwnedTrustAnchor` types are now sourced from `pki_types`, with an associated generic lifetime. The `OwnedTrustAnchor` type is now just `TrustAnchor`. * The 'dangerous' rustls crate feature was removed, and associated items moved into new locations with the import path emphasizing danger. * "Other error" types changed to use a specific `rustls::OtherError` inner variant. * `SystemTime` for verifiers replaced with `pki_types::UnixTime`. * Default fns on `ServerCertVerifier` trait were removed, must be reconstituted with `rustls::verify_tls12_signature`, `rustls::verify_tls13_signature` and `WebPkiSupportedAlgorithms.supported_schemes` using a `CryptoProvider`. * `ServerName` now supports a `to_str` operation, avoiding the need to `match` and handle unsupported name types. * `WebPkiVerifier` was renamed to `WebPkiServerVerifier`, handled as an `Arc` and constructed with a builder.
|
The latest cleanup changes still look good to me, so LGTM. |
Description
This branch updates to Rustls 0.22 and brings in the associated dependency updates this requires. For the time being, this branch continues to unconditionally use ring as the crypto provider. Follow-up work to expose this as a choice (e.g allowing aws-lc-rs as a provider) may be interesting.
Deps:
Linux deps:
Android deps:
WASM32 deps:
Summary of breaking change updates:
pki_typesre-export, removing the need to add that as our own dep with matching version.ServerName,Certificate, andOwnedTrustAnchortypes are now sourced frompki_types, with an associated generic lifetime. TheOwnedTrustAnchortype is now justTrustAnchor.rustls::OtherErrorinner variant.SystemTimefor verifiers replaced withpki_types::UnixTime.ServerCertVerifiertrait were removed, must be reconstituted withrustls::verify_tls12_signature,rustls::verify_tls13_signatureandWebPkiSupportedAlgorithms.supported_schemesusing aCryptoProvider.ServerNamenow supports ato_stroperation, avoiding the need tomatchand handle unsupported name types.WebPkiVerifierwas renamed toWebPkiServerVerifier, handled as anArcand constructed with a builder.Reviewers will likely be most interested in some of the changes associated with the new
pki_types::UnixTimetype. Since we no longer receive aSystemTimefrom Rustls some of the code adjusting the verification timestamp into platform specific representations needed to change. I think I've handled this correctly but would appreciate more detailed review on this aspect of the branch.