Enable selecting system certificate stores. #787
Merged
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
alilleybrinker
commented
May 9, 2024
alilleybrinker
commented
May 9, 2024
epage
reviewed
May 10, 2024
epage
reviewed
May 10, 2024
epage
reviewed
May 10, 2024
epage
reviewed
May 10, 2024
|
Ended up opting to rebase down to one commit, and also fixed the table reformatting so the table is no longer reformatted at all, just has a row added for the new flag. If you'd still like that in its own commit separately, let me know. |
epage
reviewed
Jun 6, 2024
Pull Request Test Coverage Report for Build 9064780768Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
epage
reviewed
Jun 6, 2024
Pull Request Test Coverage Report for Build 9568133721Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
epage
reviewed
Jun 21, 2024
|
Rebased on upstream. Doing local tests now and may have additional fixes to keep things running. Prior security audit failed so I'd like to address that. |
Pull Request Test Coverage Report for Build 9618092826Details
💛 - Coveralls |
|
Regarding the |
|
The audit is non-blocking |
This commit adds the ability to select what certificate root store to use when making HTTPS connections. Previously, `cargo-release` was configured to use the default for `tame-index`, which is the "webpki" root of trust maintained by Mozilla. This is a good and reasonable set of certificates, _except_ for users on networks which substitute certificates. Substituting certificates is something enterprise networks will frequently do so they can man-in-the-middle HTTPS connections made on their network and thus maintain visibility into the network activities of their employees. In this setup, the users' devices will generally be running enterprise-managed software which replaces certificates used by public websites with ones provided by the network software the enterprise uses, with the root certificates for these substituted chains being placed in the users' local system certificate store. In that case, with only the "webpki" certificate store loaded for `cargo-release`, the substituted certificates will fail to validate, and publication of new versions (indeed, even checking publication status of the crate attempting to be published) will fail with an HTTPS error about an untrusted certificate. The solution chosen here was to add a configuration element, and a CLI flag which lets the user pick between "webpki" (Mozilla) or "native" (local system) certificate trust stores. The portion of the code in `src/ops/index.rs` which handles connecting to the registry index has been updated to configure which certificate store to use based on the user's selection. The one final wrinkle is that we get `reqwest`, the dependency which actually handles HTTPS connections, through the `tame-index` crate, which re-exports it. To enable the APIs in `reqwest` for configuring what TLS certs to pick up, we have to enable the "native-certs" feature on `tame-index`, while leaving the default features on. This is something `tame-index` normally recommends _against_, because it assumes you want to exclusively activate one of them at compile time. In our case, we need the selection to happen at runtime, so we need both to be compiled in. Signed-off-by: Andrew Lilley Brinker <[email protected]>
Pull Request Test Coverage Report for Build 9618448278Details
💛 - Coveralls |
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 commit adds the ability to select what certificate root store to use when making HTTPS connections. Previously,
cargo-releasewas configured to use the default fortame-index, which is the "webpki" root of trust maintained by Mozilla. This is a good and reasonable set of certificates, except for users on networks which substitute certificates.Substituting certificates is something enterprise networks will frequently do so they can man-in-the-middle HTTPS connections made on their network and thus maintain visibility into the network activities of their employees. In this setup, the users' devices will generally be running enterprise-managed software which replaces certificates used by public websites with ones provided by the network software the enterprise uses, with the root certificates for these substituted chains being placed in the users' local system certificate store. In that case, with only the "webpki" certificate store loaded for
cargo-release, the substituted certificates will fail to validate, and publication of new versions (indeed, even checking publication status of the crate attempting to be published) will fail with an HTTPS error about an untrusted certificate.The solution chosen here was to add a configuration element, and a CLI flag under the "publish" section, which lets the user pick between "webpki" (Mozilla) or "native" (local system) certificate trust stores. The portion of the code in
src/ops/index.rswhich handles connecting to the registry index has been updated to configure which certificate store to use based on the user's selection.The one final wrinkle is that we get
reqwest, the dependency which actually handles HTTPS connections, through thetame-indexcrate, which re-exports it. To enable the APIs inreqwestfor configuring what TLS certs to pick up, we have to enable the "native-certs" feature ontame-index, while leaving the default features on. This is somethingtame-indexnormally recommends against, because it assumes you want to exclusively activate one of them at compile time. In our case, we need the selection to happen at runtime, so we need both to be compiled in.Fixes #764