Skip to content

Pluggable Crypto / Update reqwest 0.13#707

Open
goffrie wants to merge 10 commits into
apache:mainfrom
goffrie:pluggable-crypto
Open

Pluggable Crypto / Update reqwest 0.13#707
goffrie wants to merge 10 commits into
apache:mainfrom
goffrie:pluggable-crypto

Conversation

@goffrie
Copy link
Copy Markdown

@goffrie goffrie commented Apr 28, 2026

NOTE: this is a rebased version of #585

Which issue does this PR close?

Closes #413.

Rationale for this change

What changes are included in this PR?

Adds the ability to compile object_store without a dependency on ring, and instead use aws-lc-rs or a user-provided crypto implementation. Also changes the default to aws-lc-rs, which the ecosystem seems to be moving toward as a default.

Also upgrades reqwest to 0.13, and makes it possible to configure reqwest with any TLS backend by selecting a -no-crypto feature.

Are there any user-facing changes?

@djc
Copy link
Copy Markdown

djc commented May 12, 2026

Ping? Would be nice to avoid having duplicate reqwest versions in downstream projects.

@goffrie
Copy link
Copy Markdown
Author

goffrie commented May 13, 2026

I've given this another rebase, I think it's more blocked on review/maintainer capacity though?

@goffrie
Copy link
Copy Markdown
Author

goffrie commented May 13, 2026

(cc @Creperum since you were asking for a rebase in #585)

@goffrie goffrie force-pushed the pluggable-crypto branch from 9363266 to 51969a8 Compare May 26, 2026 21:06
@djc
Copy link
Copy Markdown

djc commented Jun 1, 2026

Maintainers, could someone take a look? Would be nice to avoid duplicate dependencies in downstream applications that are up to date with the rest of the ecosystem (and it's been 5 months since reqwest 0.13 was released).

Copy link
Copy Markdown

@Tpt Tpt left a comment

Choose a reason for hiding this comment

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

Not a maintainer but I would love to see this MR get merged.

I allowed myself 3 suggestions.

Comment thread .github/workflows/ci.yml
- 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

Choose a reason for hiding this comment

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

I guess this test should also add aws-lc-rs?

Suggested change
| grep -qE '\b(ring|openssl)\b' && { echo "❌ disallowed crate found"; exit 1; } || echo "✅ no disallowed crates"
| grep -qE '\b(ring|openssl|aws-lc-rs)\b' && { echo "❌ disallowed crate found"; exit 1; } || echo "✅ no disallowed crates"

Comment thread src/client/mod.rs
"user_agent" => Ok(Self::UserAgent),
_ => Err(super::Error::UnknownConfigurationKey {
store: "HTTP",
store: "http-no-crypto",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

find/replace error?

Suggested change
store: "http-no-crypto",
store: "HTTP",

Comment thread src/gcp/credential.rs
Ok(_) => Err(Error::MissingKey),
Err(source) => Err(Error::ReadPem { source }),
}
#[cfg(feature = "aws-lc-rs")]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hyper-nit (please ignore): moving the cfg switch inside of the function might help ensuring the signature does not diverge between aws-lc and rustls (same for the other functions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change crypto provider from ring to aws-rust-lc

5 participants