-
Notifications
You must be signed in to change notification settings - Fork 100
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
Update tokio-rustls dependency #263
Conversation
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.
Looks great! Had a couple comments on your insecure verifier update.
RE: the private key: agreed that it seems like the variable name/docs are misleading. Since the value is fed through rustls_pemfile::private_key()
I would describe it as something like the "file path to a PEM file containing the DER encoding of a PKCS8, SEC1 or PKCS1 private key". The supported formats/keys depends on the crypto provider but that's the default set. There's no support in Rustls-land for a private key with a password.
src/client/conn.rs
Outdated
|
||
impl DangerousAcceptAllVerifier { | ||
fn new() -> Self { | ||
DangerousAcceptAllVerifier(CryptoProvider::get_default().unwrap().as_ref().clone()) |
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 life will be easier in this case if you have DangerousAcceptAllVerifier
hold Arc<CryptoProvider>
instead of CryptoProvider
. Afterwards this can boil down to just CryptoProvider::get_default().unwrap()
.
Or, for maximal debuggability, CryptoProvider::get_default().expect("no process default crypto provider has been set - application must call CryptoProvider::install_default()")
Co-authored-by: Daniel McCarney <[email protected]>
@aatxe any way I can help here? :) are you looking for a co-maintainer? |
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.
Looks good, thanks!
hehe I can relate. For now a new crates.io release would suffice for me, but feel free to invite me to the repo/crate. :) |
I've attempted to port this crate to the latest version of rustls. There are two things I'm not 100% sure about:
config.client_cert_pass()
is being used as a private key DER, but according to the docs it's supposed to be a private key password (e.g. pkcs8)(cc @cpu, in case you care about this) :)