Ensuring that a provider based on the one built-in is used

[stormshield-gt]

Checklist

• I've searched the issue tracker for similar requests

Is your feature request related to a problem? Please describe.

I work on a very big project with multiple applications that uses rustls and which dependencies also use rustls. We have a custom provider based on aws-lc-rs, that we want to be used by all of our crates.
Besides, some of our dependency relies on a static provider has been installed in the current process.

My problem is sometimes, when new codes that need tls is added, the original aws-lc-rs is silently used instead of our custom one.

For instance, if in a crate someone does rustls_platform_verifier::tls_config without installing our custom provider before, no error will be thrown and this will use the classic aws-lc-rs provider

I thought how I could detect this with some tooling, like clippy or cargo deny and I didn't figure it out.

Describe the solution you'd like

I would like that ring and aws-lc-rs provider to be moved on a separate crate. They could still be added as dependency to ruslts under feature flag to keep the current behavior.

If aws-lc-rs provider were in a separated crate, I could use this crate directly for my custom provider. I could then deny the feature aws-lc-rs and ring with cargo deny

[[bans.features]]
name = "rustls"
deny = ["aws-lc-rs","aws_lc_rs", "ring"]
reason = "we use our own provider"

Then if somebody forgets to install the custom provider, it will be caught thanks to a panic in get_default_or_install_from_crate_features() . This can work right now for providers that aren't based on ring or aws-lc-rs, but in my case I must activate the aws-lc-rs feature so I can base my custom provider on it.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Option A: force users to always use builder_with_provider

I can tell users to not use the process default provider and add this to my clippy.toml:

disallowed-methods = [
    { path = "rustls::ClientConfig::builder", reason = "can silently use a wrong provider" },
    { path = "rustls::ServerConfig::builder", reason = "can silently use a wrong provider" },
    # But those methodes can also be called transitively..
    { path = "rustls_platform_verifier::tls_config", reason = "can silently use a wrong provider" },  
]

The problem is that I must also scan dependencies that my use the default provider and so each time a new version is issued.

Besides we have a patched dependency which relies on the process default because it makes the patch a lot simpler.

Option B: reexport rustls and dependencies that use rustls under an inner crate

Another solution is to have an inner crate that has rustls has a private dependency and force use to use my crate with cargo deny

deny = [
    { crate = "rustls", reason = "must not be used directly", use-instead = "inner-crate", wrappers = ["inner-crate"]},
]

This requires less auditing of dependencies, because I only have to do a cargo tree to see the one that depends of rustls. In the clippy solution I would also have to check if they use ClientConfig::builder() internally.

Additional context

I´m not a provider expert and maybe I missed something. If so I would be grateful if you can provide additional guidance. Otherwise the proposed solution, moving the providers to a crate, that I remember to have already been discussed, seems to be a reasonable option.

[stormshield-gt]

If I submit a PR moving the ring and aws-lc-rs provider to their own crate, would you be open to review this?

Based on the naming of rustls-postquantum provider, should I name the new crates rustls-ring and rustls-aws-lc-rs?

[ctz]

That seems like a large architectural change best done by the maintainers.

What are your aims in wanting do to that?

It may be worth revisiting the discussion on #1184 which proposed a similar multiple-crates solution. We didn't go that direction because a) people seemed to dislike using Cargo.toml [patch] directives to control their dependency's dependencies, b) it moves the feature mess into a different but equally annoying dimension of crate selection.

[djc]

Concretely, what are the failure modes you've seen? Do people pull in an extra dependency that builds a ClientConfig/ServerConfig without specifying any provider? Do people write code using ClientConfig::builder() and ServerConfig::builder() without making sure a/your provider has been installed?

Maybe it would be feasible to ask cargo-deny if they could check for features or upstream dependency edges, so you could enforce that no crates (other than perhaps your own) rely on rustls/aws-lc-rs or rustls/ring? (My experience with filing issues against cargo-deny has been pretty good, so it seems worth a try.)

[stormshield-gt]

Concretely, what are the failure modes you've seen? Do people pull in an extra dependency that builds a ClientConfig/ServerConfig without specifying any provider? Do people write code using ClientConfig::builder() and ServerConfig::builder() without making sure a/your provider has been installed?

I've seen people using ClientConfig::builder() but without making sure my provider has been installed. So they were using the aws-lc-rs provider because the feature was enabled at the workspace level.

Maybe it would be feasible to ask cargo-deny if they could check for features or upstream dependency edges, so you could enforce that no crates (other than perhaps your own) rely on rustls/aws-lc-rs or rustls/ring? (My experience with filing issues against cargo-deny has been pretty good, so it seems worth a try.)

Even if I was able to ensure that only my crate activates the aws-ls-rs feature, because of feature unification at the workspace level, people could still use the aws-ls-rs provider. That's actually what happened, they were using tokio_rustls with the default_features = false, so not relying on rustls/aws-lc-rs directly. I also have a good experience filling issues against cargo deny, maybe I can propose a feature to the tool to solve my issue, even if i have no idea which one to propose yet.

It may be worth revisiting the discussion on #1184 which proposed a similar multiple-crates solution. We didn't go that direction because a) people seemed to dislike using Cargo.toml [patch] directives to control their dependency's dependencies, b) it moves the feature mess into a different but equally annoying dimension of crate selection.

After reading through the discussion, I think that my proposition is different. From what I understand, people would have to choose which crate the rustls package is matching, using patch ( ex: [patch] rustls = { package = "rustls-wasi-crypto" } ). I propose that we keep the feature system as it is today, which only one rustls crate and people depending on this one.
What I want to change, is that when we activate the aws-lc-rs feature, instead of activating code inside the rustls crate, it activates a dependency to a rustls-aws-lc-rs crate, and we re-export it inside the rustls code base.
By doing so, I could create a standalone provider that depends on rustls-aws-lc-rs (and rustls for the provider traits) but without activating the feature on the rustls side. By not activating the feature, if people forget to install my provider, this will be sound because get_default_or_install_from_crate_features will panic.

[djc]

What I want to change, is that when we activate the aws-lc-rs feature, instead of activating code inside the rustls crate, it activates a dependency to a rustls-aws-lc-rs crate, and we re-export it inside the rustls code base.
By doing so, I could create a standalone provider that depends on rustls-aws-lc-rs (and rustls for the provider traits) but without activating the feature on the rustls side. By not activating the feature, if people forget to install my provider, this will be sound because get_default_or_install_from_crate_features will panic.

This feels like a very niche concern... I think it might be better to try the avenue of getting cargo-deny to warn about some features/dependency edges.

[stormshield-gt]

I'm not so sure it is so niche, I suppose that the vast majority of people that want to customize the provider will do that based on the ones that are built-in into rustls. For instance, we just remove the AES 128 from the cipher suite, because it's not advised for post-quantum security.
But maybe most people will just use the included provider, and in that case you are right, this can be a niche.

I don't know how cargo deny could detect that, the feature might be activated or not on the dependency edge and the problem would still be the same because of feature unification. Anyway, thanks for taking the time to answer and propose a solution.

[djc]

I don't know how cargo deny could detect that, the feature might be activated or not on the dependency edge and the problem would still be the same because of feature unification. Anyway, thanks for taking the time to answer and propose a solution.

I mean, I think you could "see" whether the optional dependency edge is enabled in Cargo.lock, so I think it might be feasible to do something with that -- if Cargo.lock shows a dependency edge from rustls to aws-lc-rs, fail.

[stormshield-gt]

I think there is not enough information in the Cargo.lock to differentiate the case if the feature is activated by a crate which is not the custom provider:

[[package]]
name = "foo"
version = "0.1.0"
dependencies  = [
  "rustls",
]

[[package]]
name = "foo-custom-provider"
version = "0.1.0"
dependencies  = [
  "rustls",
]

[[package]]
name = "rustls"
version = "0.23.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7fecbfb7b1444f477b345853b1fce097a2c6fb637b2bfb87e6bc5db0f043fae4"
dependencies = [
 "aws-lc-rs", 
 "once_cell",
 "rustls-pki-types",
 "rustls-webpki",
 "subtle",
 "zeroize",
]

We could imagine that cargo deny support adding exception to the features ban. Maybe adding a "skip" or something like that.

[[ban.features]]
crate = "rustls"
deny = ["ring", "aws-lc-rs", "aws_lc_rs"]
skip = [ { crate = "foo-custom-provider" } ] 

But even in that case, maybe it will just detect that the feature is not activated by anything other than the custom provider but, because of feature unification, get_default_or_install_from_crate_features might still be unsound if the wrong provider is used.

Maybe the workaround is to separate the custom provider from the workspace to prevent feature unification and then ban the aws-lc-rs and ring features in the whole workspace. EDIT: this last idea doesn't work because excluding the custom provider from the workspace doesn't prevent the features unification

[stormshield-gt]

After giving it more thought, I think even if I had the best tool, I could not make get_default_or_install_from_crate_features sound in my case.

If we imagine we have a tool that will be able to report that only my custom provider is activating the aws-lc-rs feature, that won't be sufficient.

This tool could be something like this

cargo metadata | jq '.packages[] | select(.dependencies[] | .name == "rustls" and (.uses_default_features == true or (.features[] | . == "ring" or . == "aws-lc-rs" or . == "aws_lc_rs"))) | .name'

This specific command won't work perfectly because it will include weak dependencies as the result of rust-lang/cargo#10801 . I guess I could use the krates crate, that is used by cargo deny under the hood, that seems to correct the problem (EmbarkStudios/krates#41)

Even with this tool, it will still be possible to use the vanilla aws_lc_rs provider because of feature unification.

In rcgen, we had a similar problem (rustls/rcgen#206) and we could just test to compile as standalone package (cargo build -p ) to detect this. But here, because the check is at runtime, I would have to launch all my binary in a CI until they hit get_default_or_install_from_crate_features and check that it is actually panicking and for the good reason. This is not really appealing to me.

Envisaged solutions

• if aws-lc-rs provider where a separated crate, I could just ban the use of the aws-lc-rs and ring features for every crate. Then, within my provider, I could use the aws-lc-rs provider crate, without passing by the rustls feature. I would have the guarantee that get_default_or_install_from_crate_features will panic if my provider is not used.
• Create my own crate that copy the implementation of the aws_lc_rs provider and also ban the use of the aws-lc-rs and ring features for every crate. I guess I could just create a sub-module to keep it in sync with rustls/src/crypto/aws_lc_rs folder . It's kind of a light fork but without changing the code.

[djc]

• Create my own crate that copy the implementation of the aws_lc_rs provider and also ban the use of the aws-lc-rs and ring features for every crate. I guess I could just create a sub-module to keep it in sync with rustls/src/crypto/aws_lc_rs folder . It's kind of a light fork but without changing the code.

This seems kind of reasonable?

[stormshield-gt]

I think we would like to avoid this kind of forking, because of the maintenance cost, especially because of the fact it is crypto related.

We have a very big project with a lot of standalone binary and dependencies and we would like to ensure that all of them use rustls-postquantum.
We want to guarantees that all handshakes are done in post-quantum safe way, and we want this to be valid long term.

It might not be obvious for future coworker to always install rustls-post-quantum before using rustls, and their code will work just fine because they will use directly use aws-lc-rs. It would be very reassuring if we could have a check, at least a runtime panic, to ensure this not happen.

Another solution, suggested by a coworker, might be to add an optional feature "custom-provider" to rustls and modify from_crate_features this way:

fn from_crate_features() -> Option<Self> {
        #[cfg(all(feature = "ring", not(feature = "aws_lc_rs"), not(feature = "custom-provider")))]
        {
            return Some(ring::default_provider());
        }

        #[cfg(all(feature = "aws_lc_rs", not(feature = "ring"), not(feature = "custom-provider")))]
        {
            return Some(aws_lc_rs::default_provider());
        }

        #[allow(unreachable_code)]
        None
    }

This way we can ensure that aws-lc-rs is never installed silently if someone forgot to install rustls-postquantum first.

[djc]

Hmm, sounds reasonable to me.