-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: start using rustls instead of openssl #255
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.
Super! Thanks @Angelmmiguel! Just a couple of comments :), LGTM :)
@@ -34,7 +34,7 @@ wws-server = { workspace = true } | |||
wws-project = { workspace = true } | |||
|
|||
[dev-dependencies] | |||
reqwest = { version = "0.11", features = ["blocking"] } | |||
reqwest = { version = "0.11", features = ["rustls", "blocking"] } |
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.
reqwest = { version = "0.11", features = ["rustls", "blocking"] } | |
reqwest = { version = "0.11", features = ["rustls-tls", "blocking"] } |
As per https://github.com/seanmonstar/reqwest/blob/master/Cargo.toml#L41
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.
We might want to use rustls-tls-webpki-roots
as well, so that we can provide a static binary that contains the root certificates bundled in the binary itself, without the need for the host system running wws to have ca-certificates or similar installed.
This has its problems of course, as for example, root certificate revocation, but I think it should be fine for now.
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.
Good catch! Unfortunately, I'm going to close this PR due to #256. We still need to find an alternative that works for git2-rs :/
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.
Ouch, right.
Closing for now as this feature is blocked by #256 |
Before,
wws
was using OpenSSL to establish external connections using thereqwest
crate. Initially, we wanted to userustls
, but the compilation was failing for Windows arm64. After closing all the tasks described in #137, we can now switch torustls
.In the past, we had multiple issues and CVEs due to the OpenSSL dependency. In addition to that, we had to compile it from scratch on every build so it was taking extra time. Also, we had to include
ca-certificates
in the containers images as we are usingscratch
. From now on, we will rely onrustls
related certs (webpki).It closes #137