-
Notifications
You must be signed in to change notification settings - Fork 81
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
Support TLS 1.3 #153
Support TLS 1.3 #153
Conversation
Note that rustls does not support TLS 1.0 and 1.1, only 1.2/1.3. We (the rustls maintainers) now suggest using rustls-platform-verifier instead, though (see README for rationale). That is not supported directly within reqwest today but making it work is not that hard. |
@djc thanks for pointing that out! Would you be able to look over the updated code, as I had to tweak the "Disable cert validation" logic to work with |
src-tauri/Cargo.toml
Outdated
@@ -44,8 +44,10 @@ http = "1" | |||
log = "0.4.21" | |||
rand = "0.8.5" | |||
regex = "1.10.2" | |||
reqwest = { workspace = true, features = ["multipart", "cookies", "gzip", "brotli", "deflate", "json", "native-tls-alpn"] } | |||
reqwest = { workspace = true, features = ["multipart", "cookies", "gzip", "brotli", "deflate", "json", "rustls-tls"] } |
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.
I think you could save some dependencies by picking rustls-tls-manual-roots-no-provider
instead of the default rustls-tls
(which pulls in webpki-roots).
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.
Oh, good one! I didn't notice that one in my autocomplete initially 🙏🏻
} else { | ||
// Use rustls to skip validation because rustls_platform_verifier does not have this | ||
// ability | ||
client_builder = client_builder |
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 makes sense to me.
Closes #85
As mentioned in #85, I didn't want to transition to the Reqwest
rustls-tls
feature (overnative-tls
) because I think it's valuable for Yaak to inherit the OS-native root certificates.However, I discovered that Reqwest has a
rustls-tls-native-certs
feature, which usesrustls
AND pulls certs from the OS. Perfect!Here's a test of it working with https://tls13.1d.pw.