Skip to content

Pluggable Crypto / Update reqwest 0.13 #585

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.

Sign up for GitHub

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

Open
tustvold wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Pluggable Crypto / Update reqwest 0.13 #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6533616
Pluggable Crypto
tustvold Dec 20, 2025
2269013
Add CI
tustvold Jan 14, 2026
7d2d4e8
Upgrade reqwest to 0.13
AdamGS Jan 15, 2026
1c226f0
Certificates
tustvold Jan 17, 2026
5c873c1
Switch to aws-lc-rs
tustvold Jan 17, 2026
55cc3ca
Fix CI
tustvold Jan 17, 2026
c57ea50
Fix doc
tustvold Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ jobs:
- uses: actions/checkout@v6
- name: Setup Clippy
run: rustup component add clippy
- name: Check no crypto crates
run:
cargo tree --features gcp-no-crypto,aws-no-crypto,azure-no-crypto,http-no-crypto \
| grep -qE '\b(ring|openssl)\b' && { echo "❌ disallowed crate found"; exit 1; } || echo "✅ no disallowed crates"
Copy link
Copy Markdown
Contributor

@crepererum crepererum Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that cargo-deny should be able to do that, but I DON'T think it can ban dependencies based on features:

https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html

# Run different tests for the library on its own as well as
# all targets to ensure that it still works in the absence of
# features that might be enabled by dev-dependencies of other
Expand All @@ -49,12 +53,20 @@ jobs:
run: cargo clippy --no-default-features -- -D warnings
- name: Run clippy with fs features
run: cargo clippy --no-default-features --features fs -- -D warnings
- name: Run clippy with aws-no-crypto feature
run: cargo clippy --features aws-no-crypto -- -D warnings
- name: Run clippy with aws feature
run: cargo clippy --features aws -- -D warnings
- name: Run clippy with gcp-no-crypto feature
run: cargo clippy --features gcp-no-crypto -- -D warnings
- name: Run clippy with gcp feature
run: cargo clippy --features gcp -- -D warnings
- name: Run clippy with azure-no-crypto feature
run: cargo clippy --features azure-no-crypto -- -D warnings
- name: Run clippy with azure feature
run: cargo clippy --features azure -- -D warnings
- name: Run clippy with http-no-crypto feature
run: cargo clippy --features http-no-crypto -- -D warnings
- name: Run clippy with http feature
run: cargo clippy --features http -- -D warnings
- name: Run clippy with integration feature
Expand Down Expand Up @@ -193,14 +205,14 @@ jobs:
- name: Install wasm32-wasip1
run: rustup target add wasm32-wasip1
- name: Build wasm32-wasip1
run: cargo build --all-features --target wasm32-wasip1
run: cargo build --features aws-no-crypto,gcp-no-crypto,azure-no-crypto,http-no-crypto,ring --target wasm32-wasip1
- name: Install wasm-pack
run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh
- uses: actions/setup-node@v6
with:
node-version: 20
- name: Run wasm32-unknown-unknown tests (via Node)
run: wasm-pack test --node --features http --no-default-features
run: wasm-pack test --node --features http-no-crypto,ring --no-default-features

windows:
name: cargo test LocalFileSystem (win64)
Expand Down
33 changes: 24 additions & 9 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ hyper = { version = "1.2", default-features = false, optional = true }
md-5 = { version = "0.10.6", default-features = false, optional = true }
quick-xml = { version = "0.38.0", features = ["serialize", "overlapped-lists"], optional = true }
rand = { version = "0.9", default-features = false, features = ["std", "std_rng", "thread_rng"], optional = true }
reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "http2"], optional = true }
reqwest = { version = "0.13", default-features = false, features = ["http2", "rustls-no-provider"], optional = true }
Copy link
Copy Markdown

@Tpt Tpt Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having rustls-no-provider as an explicit feature requires users to setup explicitly their crypto backend. What about just:

Suggested change
reqwest = { version = "0.13", default-features = false, features = ["http2", "rustls-no-provider"], optional = true }
reqwest = { version = "0.13", default-features = false, features = ["http2"], optional = true }

and letting users that want the reqwest auto-setup depend on reqwest with the default features enabled?

Copy link
Copy Markdown

@abonander abonander Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since features are supposed to be purely additive, enabling as a few features as necessary is the best approach for a library.

Copy link
Copy Markdown

@Tpt Tpt Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Hence my suggestion to not add rustls-no-provider feature here if it's possible

Copy link
Copy Markdown
Contributor Author

@tustvold tustvold Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this brings in aws-lc-rs which people may be wanting to avoid

Copy link
Copy Markdown

@Tpt Tpt Apr 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tustvold Indeed, it's what happens if the default-tls default feature is enabled. But if I read https://docs.rs/reqwest/latest/reqwest/tls/index.html#default-tls correctly (please correct me if I am wrong), this does not happen if default-features = false is used, and it's what is done here.

(please ignore this message if this discussion is not useful, thank you for your work on this crate)

Copy link
Copy Markdown

@abonander abonander Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested change avoids enabling any TLS backend by default. rustls-no-provider would still bring in RusTLS itself.

ring = { version = "0.17", default-features = false, features = ["std"], optional = true }
aws-lc-rs = { version = "1.15", default-features = false, optional = true }
rustls-pki-types = { version = "1.9", default-features = false, features = ["std"], optional = true }
serde = { version = "1.0", default-features = false, features = ["derive"], optional = true }
serde_json = { version = "1.0", default-features = false, features = ["std"], optional = true }
Expand All @@ -71,13 +72,26 @@ wasm-bindgen-futures = "0.4.18"

[features]
default = ["fs"]
cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest", "reqwest/stream", "chrono/serde", "base64", "rand", "ring", "http-body-util", "form_urlencoded", "serde_urlencoded"]
azure = ["cloud", "httparse"]
cloud-no-crypto = ["serde", "serde_json", "quick-xml", "hyper", "reqwest", "reqwest/stream", "chrono/serde", "base64", "rand","http-body-util", "form_urlencoded", "serde_urlencoded"]
Copy link
Copy Markdown
Contributor Author

@tustvold tustvold Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a massive fan of the feature explosion, but this was the only way to avoid needing to cut a breaking release.

cloud = ["aws-lc-rs", "cloud-no-crypto"]

aws-lc-rs = ["dep:aws-lc-rs", "reqwest/rustls", "rustls-pki-types"]
ring = ["dep:ring", "rustls-pki-types"]

azure-no-crypto = ["cloud-no-crypto", "httparse"]
azure = ["cloud", "azure-no-crypto"]

fs = ["walkdir"]
gcp = ["cloud", "rustls-pki-types"]
aws = ["cloud", "md-5"]
http = ["cloud"]
tls-webpki-roots = ["reqwest?/rustls-tls-webpki-roots"]
Copy link
Copy Markdown
Contributor Author

@tustvold tustvold Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this change is now spicy enough that I don't think it should go in a non-breaking release, I opted to take the opportunity to remove this feature flag. People can register the certificates manually if they wish to.


gcp-no-crypto = ["cloud-no-crypto"]
gcp = ["cloud", "gcp-no-crypto"]

aws-no-crypto = ["cloud-no-crypto", "md-5"]
aws = ["cloud", "aws-no-crypto"]

http-no-crypto = ["cloud-no-crypto"]
http = ["cloud", "http-no-crypto"]

integration = ["rand"]

[dev-dependencies] # In alphabetical order
Expand All @@ -86,8 +100,9 @@ hyper-util = "0.1"
rand = "0.9"
tempfile = "3.1.0"
regex = "1.11.1"
webpki-root-certs = "1"
# The "gzip" feature for reqwest is enabled for an integration test.
reqwest = { version = "0.12", default-features = false, features = ["gzip"] }
reqwest = { version = "0.13", default-features = false, features = ["gzip"] }

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dev-dependencies]
wasm-bindgen-test = "0.3.50"
Expand All @@ -105,4 +120,4 @@ features = ["js"]
[[test]]
name = "get_range_file"
path = "tests/get_range_file.rs"
required-features = ["fs"]
required-features = ["fs"]
12 changes: 11 additions & 1 deletion src/aws/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::aws::{
AmazonS3, AwsCredential, AwsCredentialProvider, Checksum, S3ConditionalPut, S3CopyIfNotExists,
STORE,
};
use crate::client::{HttpConnector, TokenCredentialProvider, http_connector};
use crate::client::{CryptoProvider, HttpConnector, TokenCredentialProvider, http_connector};
use crate::config::ConfigValue;
use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider};
use base64::Engine;
Expand Down Expand Up @@ -171,6 +171,8 @@ pub struct AmazonS3Builder {
client_options: ClientOptions,
/// Credentials
credentials: Option<AwsCredentialProvider>,
/// The [`CryptoProvider`] to use
crypto: Option<Arc<dyn CryptoProvider>>,
/// Skip signing requests
skip_signature: ConfigValue<bool>,
/// Copy if not exists
Expand Down Expand Up @@ -843,6 +845,12 @@ impl AmazonS3Builder {
self
}

/// The [`CryptoProvider`] to use
pub fn with_crypto_provider(mut self, provider: Arc<dyn CryptoProvider>) -> Self {
self.crypto = Some(provider);
self
}

/// Sets what protocol is allowed. If `allow_http` is :
/// * false (default): Only HTTPS are allowed
/// * true: HTTP and HTTPS are allowed
Expand Down Expand Up @@ -1150,6 +1158,7 @@ impl AmazonS3Builder {
endpoint: endpoint.clone(),
region: region.clone(),
credentials: Arc::clone(&credentials),
crypto: self.crypto.clone(),
},
http.connect(&self.client_options)?,
self.retry_config.clone(),
Expand Down Expand Up @@ -1190,6 +1199,7 @@ impl AmazonS3Builder {
bucket,
bucket_endpoint,
credentials,
crypto: self.crypto,
session_provider,
retry_config: self.retry_config,
client_options: self.client_options,
Expand Down
Loading