vtls/rustls: 0.14.0 update and assoc. improvements #14889
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't build `config_builder` just to free the resulting config, free the builder directly. When `cr_init_backend` encounters an error condition setting up the Rustls client configuration it must do something with the `config_builder` that was constructed earlier to avoid a memory leak. The previous implementation preferred to use a pattern of building the builder (thus consuming it) and then freeing the built config (to avoid a memory leak). However, the purpose/intent is clearer when we just free the builder directly instead of building it and freeing the result.
It's easier to diagnose a problem when there is one place where the error message can be emitted. For that reason this commit updates two errors that were shared between other fallible operations to use unique messages.
* Documentation is updated to describe new required version, and to link
to the upstream README about cryptography providers.
* GitHub workflow is updated to fetch 0.14.0.
* Breaking changes in`lib/vtls/rustls.c` are addressed:
* The `rustls_client_config_builder_build()` function now uses an out
parameter for the built config instead of returning it directly.
This allows the building process to fail if the default crypto
provider state isn't appropriate, or another error condition occurs.
* Default ciphersuites are collected using renamed functions named to
make it clear the ciphersuites are associated with the default
crypto provider.
* Customization of ciphersuites is now done via
a `rustls_crypto_provider_builder` used to instantiate a
`rustls_crypto_provider`. The customized provider can then can be
used with `rustls_client_config_builder_new_custom` in place of
providing ciphersuites directly.
* `rustls_connection_get_negotiated_ciphersuite()` now returns the
ciphersuite ID directly.
|
(cc @ctz in case you were interested in giving this a 🔍) |
cpu
commented
Sep 12, 2024
ctz
reviewed
Sep 12, 2024
Now that the rustls vtls backend is using rustls 0.14 we can take advantage of `rustls_supported_ciphersuite_protocol_version()` to skip TLS 1.3 and TLS 1.2 ciphersuites as required without needing to interrogate the ciphersuite names as `rustls_str`s.
cpu
force-pushed
the
cpu-rustls-ffi-0.14-tidy/ci
branch
from
750ce4f
to
0d65a9c
Compare
1 task
cpu
force-pushed
the
cpu-rustls-ffi-0.14-tidy/ci
branch
2 times, most recently
from
f2d8a9d
to
2ea09f8
Compare
bagder
reviewed
Sep 12, 2024
bagder
reviewed
Sep 12, 2024
cpu
force-pushed
the
cpu-rustls-ffi-0.14-tidy/ci
branch
from
2ea09f8
to
7b5134b
Compare
Now that the curl rustls vtls backend is using rustls 0.14 we can address the weak random situation by using `rustls_default_crypto_provider_random()` to provide a `Curl_ssl` `random` callback that fills the provided buffer with cryptographically secure random data. The mentions in `docs/` about weak RNG when using rustls are removed as they are no longer applicable.
cpu
force-pushed
the
cpu-rustls-ffi-0.14-tidy/ci
branch
from
7b5134b
to
d3b3480
Compare
bagder
pushed a commit
that referenced
this pull request
Sep 13, 2024
It's easier to diagnose a problem when there is one place where the error message can be emitted. For that reason this commit updates two errors that were shared between other fallible operations to use unique messages. Closes #14889
bagder
pushed a commit
that referenced
this pull request
Sep 13, 2024
* Documentation is updated to describe new required version, and to link
to the upstream README about cryptography providers.
* GitHub workflow is updated to fetch 0.14.0.
* Breaking changes in`lib/vtls/rustls.c` are addressed:
* The `rustls_client_config_builder_build()` function now uses an out
parameter for the built config instead of returning it directly.
This allows the building process to fail if the default crypto
provider state isn't appropriate, or another error condition occurs.
* Default ciphersuites are collected using renamed functions named to
make it clear the ciphersuites are associated with the default
crypto provider.
* Customization of ciphersuites is now done via
a `rustls_crypto_provider_builder` used to instantiate a
`rustls_crypto_provider`. The customized provider can then can be
used with `rustls_client_config_builder_new_custom` in place of
providing ciphersuites directly.
* `rustls_connection_get_negotiated_ciphersuite()` now returns the
ciphersuite ID directly.
Closes #14889
bagder
pushed a commit
that referenced
this pull request
Sep 13, 2024
Now that the rustls vtls backend is using rustls 0.14 we can take advantage of `rustls_supported_ciphersuite_protocol_version()` to skip TLS 1.3 and TLS 1.2 ciphersuites as required without needing to interrogate the ciphersuite names as `rustls_str`s. Closes #14889
bagder
pushed a commit
that referenced
this pull request
Sep 13, 2024
Now that the curl rustls vtls backend is using rustls 0.14 we can address the weak random situation by using `rustls_default_crypto_provider_random()` to provide a `Curl_ssl` `random` callback that fills the provided buffer with cryptographically secure random data. The mentions in `docs/` about weak RNG when using rustls are removed as they are no longer applicable. Closes #14889
|
Thanks! |
|
Thanks for the reviews :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
👋 This branch updates the experimental
rustlsvtls backend forcurlto use the just-released rustls-ffi 0.14.0, and the latest release of the main rust crate, rustls 0.23.13.Along the way I made a handful of other small improvements and so I recommend reviewing commit-by-commit. My goal was to have each commit build independent of the others, and to avoid introducing new functionality at the same time as updating breaking API changes. I've done my best to match local style but I'm still quite green at contributing to
curl: feedback very welcome.In addition to the 0.14.0 update, this branch resolves the "weak RNG" situation described in #14770 and replaces the ciphersuite name string comparison logic from #14840 with a more robust solution based on protocol version constants.