-
Notifications
You must be signed in to change notification settings - Fork 240
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 rustls to fix RUSTSEC-2023-0053 #298
Conversation
Hi @daniel-abramov @alexheretic , sorry for the ping. Would it be possible to merge this soon and release a new version, so upstream projects can resolve the vulnerability alert? |
@@ -104,8 +104,8 @@ mod encryption { | |||
} | |||
#[cfg(feature = "rustls-tls-webpki-roots")] | |||
{ | |||
root_store.add_server_trust_anchors( | |||
webpki_roots::TLS_SERVER_ROOTS.0.iter().map(|ta| { | |||
root_store.add_trust_anchors( |
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.
The updated version requirements in Cargo.toml are needed because this function does not exist in older versions? If so, only the version of rustls has to be bumped or not? Why also the bump of tokio-rustls?
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.
Yes looks like add_trust_anchors
was added in rustls 0.21.6
and add_server_trust_anchors
deprecated.
I don't know what changed in tokio-rustls 0.24.1
. But it's probably harmless to up the min version anyway 🤷
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.
If there's no reason then it forces users to update for no reason, and that might actually break something for them if for whatever reason they must use an older version. I wouldn't unnecessarily bump minimum versions.
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.
It looks like a refactor happened around commit 07e8da6e52f0eb76323f77d441bd1a06498b2f1c
here's the diff from 0.24.0
to 0.24.1
for tokio-rustls as I could find it.
|
||
[dependencies.webpki-roots] | ||
optional = true | ||
version = "0.23.0" | ||
version = "0.25.2" |
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.
IIRC the webpki-roots API is exported somewhere from tokio-tungstenite, so bumping this would require releasing this as a breaking release (i.e. 0.21.0).
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 don't believe it is exported. I can only see the one usage which is afaics isolated from the public api.
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 misremembered then, sorry :) Thanks for checking
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
It's time to release new version. |
…ON-FAILED-READ Error response on client type returns request body upon non receiving non 101 status code from server
No description provided.