-
Notifications
You must be signed in to change notification settings - Fork 49
Description
Problem
We’re migrating payjoin/rust-payjoin#1288 from reqwest to bitreq to reduce our dependency footprint.
A current blocker is payjoin’s _manual-tls feature, which requires adding custom root certificates at runtime. This is necessary for development and testing scenarios that rely on self-signed certificates.
Current reqwest implementation
In payjoin (payjoin/src/core/io.rs):
#[cfg(feature = "_manual-tls")]
pub async fn fetch_ohttp_keys_with_cert(
ohttp_relay: impl IntoUrl,
payjoin_directory: impl IntoUrl,
cert_der: Vec<u8>,
) -> Result<OhttpKeys, Error> {
let client = Client::builder()
.use_rustls_tls()
.add_root_certificate(
reqwest::tls::Certificate::from_der(&cert_der)?
)
.proxy(proxy)
.http1_only()
.build()?;
// ...
}In payjoin-cli (src/app/mod.rs):
#[cfg(feature = "_manual-tls")]
fn http_agent_builder(
root_cert_path: Option<&std::path::PathBuf>,
) -> Result<reqwest::ClientBuilder> {
let mut builder = reqwest::ClientBuilder::new()
.use_rustls_tls()
.http1_only();
if let Some(root_cert_path) = root_cert_path {
let cert_der = std::fs::read(root_cert_path)?;
builder = builder.add_root_certificate(
reqwest::tls::Certificate::from_der(cert_der.as_slice())?
);
}
Ok(builder)
}Desired Solution
Add certificate configuration to Client (or a new ClientBuilder), matching reqwest's architecture where TLS settings belong at the client/connection level rather than per-request
// Proposed API - Option A: ClientBuilder pattern (matches reqwest)
let cert_der = std::fs::read("path/to/cert.der")?;
let client = bitreq::Client::builder()
.with_root_certificate(cert_der)
.build()?;
let response = client.get("https://localhost:8443")
.send_async()
.await?; // Option B: Constructor with certs
let client = bitreq::Client::with_capacity_and_certs(10, vec![cert_der]); This is architecturally preferrable imo because:
- TLS handshake happens at the connection level, before any HTTP request
- Client already maintains a connection pool so TLS config naturally belongs there
- Multiple requests can reuse connections with the same TLS settings
- Matches the familiar reqwest pattern
Implementation suggestion (from claudey boy)
Looking at src/connection/rustls_stream.rs, the current build_client_config() creates a static cached ClientConfig. For custom certificates on Client:
- Add custom_root_certs: Vec<Vec> field to the Client struct
- Add ClientBuilder with .with_root_certificate(cert_der: Vec) method (or extend Client::new())
- When Client has custom certs, create a dedicated ClientConfig for that client instance (skip the global static cache)
- Insert custom certificates into the RootCertStore before constructing the config
- Store the custom Arc in the Client and use it for all connections from that client
Alternatives Considered
System trust store
Users can install certs into the system trust store (e.g., via https-rustls-probe).
This works, but is less ergonomic for development and fragile in CI/testing environments. Need our app to get sudo perms. Ick. We decided to avoid this alternative early on.
Keep reqwest for this path
Use bitreq for standard HTTPS and fall back to reqwest when custom certs are needed.
This undermines the goal of reducing dependencies and makes our tests test different behavior.
Additional Context
This is a blocker for migrating the payjoin ecosystem to bitreq. The _manual-tls feature is used for:
- Local development with self-signed certificates
- Integration tests with temporary certificates
- Testing against non-production environments