Separate TLS transport features from the crypto provider choice

.github/workflows/main.yml

@@ -76,6 +76,20 @@ jobs:
 
     - run: cargo test --workspace --no-default-features
 
+    # the export features must build, and be usable, without a built-in crypto provider
+    - run: cargo check --workspace --no-default-features --features logfire/export-grpc,logfire/export-http-protobuf,logfire/export-http-json
+
+    - run: cargo test -p logfire --no-default-features --features export-http-protobuf --test test_crypto_provider
+
+    - run: cargo test -p logfire-client --no-default-features --test test_crypto_provider
+
+    - name: check no crypto provider is compiled without `tls-aws-lc`
+      run: |
+        cargo tree --workspace --no-default-features \
+          --features logfire/export-grpc,logfire/export-http-protobuf,logfire/export-http-json \
+          -e normal --prefix none | sort -u > "$RUNNER_TEMP/deps.txt"
+        ! grep -E '^(aws-lc-rs|ring) ' "$RUNNER_TEMP/deps.txt"
+
     - uses: taiki-e/install-action@cargo-llvm-cov
 
     - run: cargo llvm-cov --workspace --all-features --codecov --output-path codecov.json

logfire-client/Cargo.toml

@@ -8,16 +8,21 @@ rust-version.workspace = true
 repository.workspace = true
 
 [features]
-default = []
+default = ["tls-aws-lc"]
+# Use aws-lc-rs for TLS when the application has not installed a rustls `CryptoProvider`.
+tls-aws-lc = ["reqwest/rustls"]
 
 [dependencies]
 logfire-core.workspace = true
 
 chrono = { version = "0.4", features = ["serde"] }
-reqwest = { version = "0.13", default-features = false, features = ["json", "query", "rustls"] }
+reqwest = { version = "0.13", default-features = false, features = ["json", "query", "rustls-no-provider"] }
+rustls = { version = "0.23", default-features = false, features = ["std"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 thiserror.workspace = true
 
 [dev-dependencies]
+# a provider to install in tests which run without the `tls-aws-lc` feature
+rustls = { version = "0.23", default-features = false, features = ["aws_lc_rs", "std"] }
 tokio = { version = "1", features = ["rt-multi-thread", "macros"] }

logfire-client/src/builder.rs

@@ -13,6 +13,7 @@ const LOGFIRE_BASE_URL_ENV: &str = "LOGFIRE_BASE_URL";
 
 /// Errors that can occur when building a Logfire client.
 #[derive(Debug, thiserror::Error)]
+#[non_exhaustive]
 pub enum BuilderError {
     /// Failed to construct the HTTP client.
     #[error("failed to build client")]
@@ -23,6 +24,11 @@ pub enum BuilderError {
     /// No token was configured on the builder.
     #[error("no token configured; call token() or use from_env()")]
     MissingToken,
+    /// No rustls `CryptoProvider` is installed to use for TLS.
+    #[error(
+        "no rustls `CryptoProvider` is installed: either call `CryptoProvider::install_default()` before building a client, or enable the `tls-aws-lc` feature to use aws-lc-rs"
+    )]
+    CryptoProviderRequired,
 }
 
 /// Default request timeout (30 seconds).
@@ -100,6 +106,13 @@ impl LogfireClientBuilder {
             )),
         );
 
+        // `reqwest` builds its TLS configuration when the client is built, and without the
+        // `tls-aws-lc` feature it has no provider to fall back on, so it would panic here.
+        #[cfg(not(feature = "tls-aws-lc"))]
+        if rustls::crypto::CryptoProvider::get_default().is_none() {
+            return Err(BuilderError::CryptoProviderRequired);
+        }
+
         let client = reqwest::Client::builder()
             .timeout(self.timeout)
             .default_headers(headers)
@@ -164,8 +177,16 @@ mod tests {
         ));
     }
 
+    /// Install a provider, for test configurations which have no built-in one.
+    fn install_crypto_provider() {
+        #[cfg(not(feature = "tls-aws-lc"))]
+        let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
+    }
+
     #[test]
     fn build_client_with_token() {
+        install_crypto_provider();
+
         let mut builder = LogfireClientBuilder::new();
         builder.token("pylf_v1_us_abc123");
         assert!(builder.build_client().is_ok());

logfire-client/tests/test_crypto_provider.rs

@@ -0,0 +1,32 @@
+//! Without the `tls-aws-lc` feature there is no provider to fall back on, so building a client
+//! requires the application to have installed one.
+//!
+//! Run with:
+//! `cargo test -p logfire-client --no-default-features --test test_crypto_provider`
+#![cfg(not(feature = "tls-aws-lc"))]
+
+use logfire_client::builder::{BuilderError, LogfireClientBuilder};
+
+#[test]
+fn build_client_requires_an_installed_crypto_provider() {
+    let mut builder = LogfireClientBuilder::new();
+    builder.token("pylf_v1_us_abc123");
+
+    let error = builder
+        .build_client()
+        .err()
+        .expect("expected an error when no crypto provider is installed");
+
+    assert!(
+        matches!(error, BuilderError::CryptoProviderRequired),
+        "unexpected error: {error}"
+    );
+
+    rustls::crypto::aws_lc_rs::default_provider()
+        .install_default()
+        .expect("failed to install crypto provider");
+
+    builder
+        .build_client()
+        .expect("client should build once a provider is installed");
+}

logfire/Cargo.toml

@@ -19,6 +19,11 @@ env_filter = "2"
 http = { version = "1.2", optional = true }
 tonic = { version = "0.14", optional = true }
 
+# TLS for OTLP exports. `reqwest` must stay on the same major version as `opentelemetry-http`'s,
+# so that the TLS features enabled here apply to the client it builds.
+reqwest = { version = "0.13", optional = true, default-features = false }
+rustls = { version = "0.23", optional = true, default-features = false, features = ["std"] }
+
 rand = "0.10"
 
 opentelemetry = { version = "0.32", default-features = false, features = [
@@ -64,6 +69,8 @@ opentelemetry_sdk = { version = "0.32", default-features = false, features = [
     "testing",
 ] }
 regex = "1.11.1"
+# a provider to install in tests which run without the `tls-aws-lc` feature
+rustls = { version = "0.23", default-features = false, features = ["aws_lc_rs", "std"] }
 tokio = { version = "1.44.1", features = [
     "test-util",
     "macros",
@@ -96,24 +103,29 @@ futures-util = "0.3"
 tempfile = "3.20.0"
 
 [features]
-default = ["data-dir", "export-http-protobuf"]
+default = ["data-dir", "export-http-protobuf", "tls-aws-lc"]
 data-dir = ["dep:serde", "dep:serde_json"]
 export-grpc = [
     "opentelemetry-otlp/grpc-tonic",
-    "opentelemetry-otlp/tls-aws-lc",
+    "tonic/tls-native-roots",
     "dep:http",
+    "dep:rustls",
     "dep:tonic",
 ]
 export-http-protobuf = [
     "opentelemetry-otlp/http-proto",
     "opentelemetry-otlp/reqwest-client",
-    "opentelemetry-otlp/reqwest-rustls",
+    "reqwest/rustls-no-provider",
+    "dep:rustls",
 ]
 export-http-json = [
     "opentelemetry-otlp/http-json",
     "opentelemetry-otlp/reqwest-client",
-    "opentelemetry-otlp/reqwest-rustls",
+    "reqwest/rustls-no-provider",
+    "dep:rustls",
 ]
+# Use aws-lc-rs for TLS when the application has not installed a rustls `CryptoProvider`.
+tls-aws-lc = ["tonic?/tls-aws-lc", "reqwest?/rustls"]
 
 [lints.rust]
 missing_docs = "warn"

logfire/src/exporters.rs

@@ -100,20 +100,15 @@ pub fn span_exporter(
             use opentelemetry_otlp::WithTonicConfig;
             opentelemetry_otlp::SpanExporter::builder()
                 .with_tonic()
-                .with_channel(
-                    tonic::transport::Channel::builder(
-                        endpoint
-                            .try_into()
-                            .map_err(|e: http::uri::InvalidUri| ConfigureError::Other(e.into()))?,
-                    )
-                    .connect_lazy(),
-                )
+                .with_channel(grpc_channel(endpoint)?)
                 .with_metadata(build_metadata_from_headers(headers)?)
                 .build()?
         }
         #[cfg(feature = "export-http-protobuf")]
         Protocol::HttpBinary => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             opentelemetry_otlp::SpanExporter::builder()
                 .with_http()
                 .with_protocol(Protocol::HttpBinary)
@@ -124,6 +119,8 @@ pub fn span_exporter(
         #[cfg(feature = "export-http-json")]
         Protocol::HttpJson => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             opentelemetry_otlp::SpanExporter::builder()
                 .with_http()
                 .with_protocol(Protocol::HttpJson)
@@ -189,20 +186,15 @@ pub fn metric_exporter(
             Ok(opentelemetry_otlp::MetricExporter::builder()
                 .with_temporality(opentelemetry_sdk::metrics::Temporality::Delta)
                 .with_tonic()
-                .with_channel(
-                    tonic::transport::Channel::builder(
-                        endpoint
-                            .try_into()
-                            .map_err(|e: http::uri::InvalidUri| ConfigureError::Other(e.into()))?,
-                    )
-                    .connect_lazy(),
-                )
+                .with_channel(grpc_channel(endpoint)?)
                 .with_metadata(build_metadata_from_headers(headers)?)
                 .build()?)
         }
         #[cfg(feature = "export-http-protobuf")]
         Protocol::HttpBinary => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             Ok(opentelemetry_otlp::MetricExporter::builder()
                 .with_temporality(opentelemetry_sdk::metrics::Temporality::Delta)
                 .with_http()
@@ -214,6 +206,8 @@ pub fn metric_exporter(
         #[cfg(feature = "export-http-json")]
         Protocol::HttpJson => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             Ok(opentelemetry_otlp::MetricExporter::builder()
                 .with_temporality(opentelemetry_sdk::metrics::Temporality::Delta)
                 .with_http()
@@ -270,20 +264,15 @@ pub fn log_exporter(
             use opentelemetry_otlp::WithTonicConfig;
             Ok(opentelemetry_otlp::LogExporter::builder()
                 .with_tonic()
-                .with_channel(
-                    tonic::transport::Channel::builder(
-                        endpoint
-                            .try_into()
-                            .map_err(|e: http::uri::InvalidUri| ConfigureError::Other(e.into()))?,
-                    )
-                    .connect_lazy(),
-                )
+                .with_channel(grpc_channel(endpoint)?)
                 .with_metadata(build_metadata_from_headers(headers)?)
                 .build()?)
         }
         #[cfg(feature = "export-http-protobuf")]
         Protocol::HttpBinary => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             Ok(opentelemetry_otlp::LogExporter::builder()
                 .with_http()
                 .with_protocol(Protocol::HttpBinary)
@@ -294,6 +283,8 @@ pub fn log_exporter(
         #[cfg(feature = "export-http-json")]
         Protocol::HttpJson => {
             use opentelemetry_otlp::{WithExportConfig, WithHttpConfig};
+
+            require_crypto_provider()?;
             Ok(opentelemetry_otlp::LogExporter::builder()
                 .with_http()
                 .with_protocol(Protocol::HttpJson)
@@ -318,6 +309,107 @@ pub fn log_exporter(
     }
 }
 
+/// Build the `tonic` endpoint used for gRPC exports.
+///
+/// TLS is configured here because `tonic` only configures it itself in `Endpoint::new`, which
+/// none of its public constructors go through. Without this, an https endpoint fails to connect
+/// with "Connecting to HTTPS without TLS enabled".
+#[cfg(feature = "export-grpc")]
+fn grpc_endpoint(endpoint: &str) -> Result<tonic::transport::Endpoint, ConfigureError> {
+    let uri: http::Uri = endpoint
+        .try_into()
+        .map_err(|e: http::uri::InvalidUri| ConfigureError::Other(e.into()))?;
+
+    let mut builder = tonic::transport::Channel::builder(uri.clone());
+
+    if uri.scheme() == Some(&http::uri::Scheme::HTTPS) {
+        require_crypto_provider()?;
+        builder = builder
+            .tls_config(tonic::transport::ClientTlsConfig::new().with_enabled_roots())
+            .map_err(|e| ConfigureError::Other(e.into()))?;
+    }
+
+    Ok(builder)
+}
+
+/// Build the `tonic` channel used for gRPC exports.
+#[cfg(feature = "export-grpc")]
+fn grpc_channel(endpoint: &str) -> Result<tonic::transport::Channel, ConfigureError> {
+    Ok(grpc_endpoint(endpoint)?.connect_lazy())
+}
+
+#[cfg(all(test, feature = "export-grpc"))]
+mod grpc_tls_tests {
+    use super::grpc_endpoint;
+
+    /// Connect to a listener which accepts connections but never replies. With TLS configured,
+    /// the client sends a handshake and waits, so the connection attempt times out. Without it,
+    /// `tonic` fails immediately with "Connecting to HTTPS without TLS enabled".
+    #[tokio::test]
+    async fn https_endpoints_have_tls_configured() {
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
+            .await
+            .expect("failed to bind listener");
+        let addr = listener.local_addr().expect("failed to read listener addr");
+
+        tokio::spawn(async move {
+            // hold the connections open, so that the client waits for a handshake it never gets
+            let mut accepted = Vec::new();
+            loop {
+                match listener.accept().await {
+                    Ok((stream, _)) => accepted.push(stream),
+                    Err(_) => break,
+                }
+            }
+        });
+
+        let endpoint = grpc_endpoint(&format!("https://{addr}")).expect("failed to build endpoint");
+
+        let result =
+            tokio::time::timeout(std::time::Duration::from_secs(2), endpoint.connect()).await;
+
+        match result {
+            // the handshake is still in flight, so TLS was configured
+            Err(_elapsed) => {}
+            Ok(Ok(_)) => panic!("unexpectedly connected to a listener which never replies"),
+            Ok(Err(error)) => {
+                let mut message = error.to_string();
+                let mut source = std::error::Error::source(&error);
+                while let Some(inner) = source {
+                    message.push_str(&format!(": {inner}"));
+                    source = inner.source();
+                }
+
+                assert!(
+                    !message.contains("without TLS enabled"),
+                    "TLS was not configured for an https endpoint: {message}"
+                );
+            }
+        }
+    }
+}
+
+/// Check that a rustls `CryptoProvider` will be available for TLS.
+///
+/// With the `tls-aws-lc` feature, `tonic` and `reqwest` fall back to aws-lc-rs when the
+/// application has not installed a provider, so there is nothing to check. Without it they have
+/// no fallback to use, and `reqwest` panics rather than returning an error, so the installed
+/// provider is required up front.
+#[cfg(any(
+    feature = "export-grpc",
+    feature = "export-http-protobuf",
+    feature = "export-http-json"
+))]
+#[cfg_attr(feature = "tls-aws-lc", allow(clippy::unnecessary_wraps))]
+fn require_crypto_provider() -> Result<(), ConfigureError> {
+    #[cfg(not(feature = "tls-aws-lc"))]
+    if rustls::crypto::CryptoProvider::get_default().is_none() {
+        return Err(ConfigureError::CryptoProviderRequired);
+    }
+
+    Ok(())
+}
+
 #[cfg(feature = "export-grpc")]
 fn build_metadata_from_headers(
     headers: Option<HashMap<String, String>>,