Skip to content

Commit

Permalink
Remove undocumented panic in with_native_roots
Browse files Browse the repository at this point in the history
  • Loading branch information
kayabaNerve authored and djc committed Nov 15, 2023
1 parent a2b282b commit 77154da
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 9 deletions.
2 changes: 1 addition & 1 deletion examples/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async fn run_client() -> io::Result<()> {
// Default TLS client config with native roots
None => rustls::ClientConfig::builder()
.with_safe_defaults()
.with_native_roots()
.with_native_roots()?
.with_no_client_auth(),
};
// Prepare the HTTPS connector
Expand Down
21 changes: 17 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ use rustls::{ClientConfig, ConfigBuilder, WantsVerifier};
pub trait ConfigBuilderExt {
/// This configures the platform's trusted certs, as implemented by
/// rustls-native-certs
///
/// This will return an error if no valid certs were found. In that case,
/// it's recommended to use `with_webpki_roots`.
#[cfg(feature = "rustls-native-certs")]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>;
fn with_native_roots(
self,
) -> std::io::Result<ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>>;

/// This configures the webpki roots, which are Mozilla's set of
/// trusted roots as packaged by webpki-roots.
Expand All @@ -23,7 +28,9 @@ impl ConfigBuilderExt for ConfigBuilder<ClientConfig, WantsVerifier> {
#[cfg(feature = "rustls-native-certs")]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert> {
fn with_native_roots(
self,
) -> std::io::Result<ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>> {
let mut roots = rustls::RootCertStore::empty();
let mut valid_count = 0;
let mut invalid_count = 0;
Expand All @@ -45,9 +52,15 @@ impl ConfigBuilderExt for ConfigBuilder<ClientConfig, WantsVerifier> {
valid_count,
invalid_count
);
assert!(!roots.is_empty(), "no CA certificates found");
if roots.is_empty() {
crate::log::debug!("no valid native root CA certificates found");
Err(std::io::Error::new(
std::io::ErrorKind::NotFound,
format!("no valid native root CA certificates found ({invalid_count} invalid)"),
))?
}

self.with_root_certificates(roots)
Ok(self.with_root_certificates(roots))
}

#[cfg(feature = "webpki-roots")]
Expand Down
8 changes: 4 additions & 4 deletions src/connector/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ impl ConnectorBuilder<WantsTlsConfig> {
/// [with_safe_defaults]: rustls::ConfigBuilder::with_safe_defaults
#[cfg(feature = "rustls-native-certs")]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
pub fn with_native_roots(self) -> ConnectorBuilder<WantsSchemes> {
self.with_tls_config(
pub fn with_native_roots(self) -> std::io::Result<ConnectorBuilder<WantsSchemes>> {
Ok(self.with_tls_config(
ClientConfig::builder()
.with_safe_defaults()
.with_native_roots()
.with_native_roots()?
.with_no_client_auth(),
)
))
}

/// Shorthand for using rustls' [safe defaults][with_safe_defaults]
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! let url = ("https://hyper.rs").parse().unwrap();
//! let https = hyper_rustls::HttpsConnectorBuilder::new()
//! .with_native_roots()
//! .expect("no native root CA certificates found")
//! .https_only()
//! .enable_http1()
//! .build();
Expand Down Expand Up @@ -59,6 +60,7 @@
//! let key = rustls::PrivateKey(keys[0].clone());
//! let https = hyper_rustls::HttpsConnectorBuilder::new()
//! .with_native_roots()
//! .expect("no native root CA certificates found")
//! .https_only()
//! .enable_http1()
//! .build();
Expand Down

0 comments on commit 77154da

Please sign in to comment.