Skip to content

Commit 0c7d82f

Browse files
authored
chore(trust): improve unreachable branch in certificate loading (#1167)
* chore(trust): improve unreachable branch in certificate loading * fmt
1 parent 659b3ed commit 0c7d82f

3 files changed

Lines changed: 60 additions & 71 deletions

File tree

src/client.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ struct Config {
226226
tls_sni: bool,
227227
tls_verify_hostname: bool,
228228
tls_identity: Option<Identity>,
229-
tls_cert_store: CertStore,
229+
tls_cert_store: Option<CertStore>,
230230
tls_cert_verification: bool,
231231
tls_min_version: Option<TlsVersion>,
232232
tls_max_version: Option<TlsVersion>,
@@ -312,7 +312,7 @@ impl Client {
312312
tls_sni: true,
313313
tls_verify_hostname: true,
314314
tls_identity: None,
315-
tls_cert_store: CertStore::default(),
315+
tls_cert_store: None,
316316
tls_cert_verification: true,
317317
tls_min_version: None,
318318
tls_max_version: None,
@@ -1386,7 +1386,7 @@ impl ClientBuilder {
13861386
/// for TLS connections. By default, the system's verify certificate store is used.
13871387
#[inline]
13881388
pub fn tls_cert_store(mut self, store: CertStore) -> ClientBuilder {
1389-
self.config.tls_cert_store = store;
1389+
self.config.tls_cert_store = Some(store);
13901390
self
13911391
}
13921392

src/tls/conn/ext.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,24 @@ impl SslConnectorBuilderExt for SslConnectorBuilder {
4646
if let Some(store) = store {
4747
self.set_cert_store_ref(&store.0)
4848
} else {
49+
#[cfg(feature = "webpki-roots")]
50+
{
51+
static LOAD_CERTS: std::sync::OnceLock<crate::Result<CertStore>> =
52+
std::sync::OnceLock::new();
53+
54+
if let Ok(store) = LOAD_CERTS.get_or_init(|| {
55+
CertStore::from_der_certs(webpki_root_certs::TLS_SERVER_ROOT_CERTS).map_err(
56+
|err| {
57+
warn!("Failed to load webpki root certificates: {:?}", err);
58+
err
59+
},
60+
)
61+
}) {
62+
self.set_cert_store_ref(&store.0);
63+
return Ok(self);
64+
}
65+
}
66+
4967
self.set_default_verify_paths().map_err(Error::tls)?;
5068
}
5169

src/tls/trust/store.rs

Lines changed: 39 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,44 @@ use super::{
88
};
99
use crate::{Error, Result};
1010

11-
/// A builder for constructing a `CertStore`.
11+
/// A builder for constructing a [`CertStore`].
1212
pub struct CertStoreBuilder {
1313
builder: Result<X509StoreBuilder>,
1414
}
1515

16-
// ====== impl CertStoreBuilder ======
17-
1816
impl CertStoreBuilder {
17+
fn parse_cert<'c, C, P>(mut self, cert: C, parser: P) -> Self
18+
where
19+
C: Into<CertificateInput<'c>>,
20+
P: Fn(&'c [u8]) -> Result<Certificate>,
21+
{
22+
if let Ok(ref mut builder) = self.builder {
23+
let input = cert.into();
24+
let result = input
25+
.with_parser(parser)
26+
.and_then(|cert| builder.add_cert(cert.0).map_err(Error::tls));
27+
28+
if let Err(err) = result {
29+
self.builder = Err(err);
30+
}
31+
}
32+
self
33+
}
34+
35+
fn parse_certs<'c, I>(mut self, certs: I, parser: fn(&'c [u8]) -> Result<Certificate>) -> Self
36+
where
37+
I: IntoIterator,
38+
I::Item: Into<CertificateInput<'c>>,
39+
{
40+
if let Ok(ref mut builder) = self.builder {
41+
let certs = filter_map_certs(certs, parser);
42+
if let Err(err) = process_certs(certs, builder) {
43+
self.builder = Err(err);
44+
}
45+
}
46+
self
47+
}
48+
1949
/// Adds a DER-encoded certificate to the certificate store.
2050
#[inline]
2151
pub fn add_der_cert<'c, C>(self, cert: C) -> Self
@@ -84,9 +114,9 @@ impl CertStoreBuilder {
84114
self
85115
}
86116

87-
/// Constructs the `CertStore`.
117+
/// Constructs the [`CertStore`].
88118
///
89-
/// This method finalizes the builder and constructs the `CertStore`
119+
/// This method finalizes the builder and constructs the [`CertStore`]
90120
/// containing all the added certificates.
91121
#[inline]
92122
pub fn build(self) -> Result<CertStore> {
@@ -97,40 +127,6 @@ impl CertStoreBuilder {
97127
}
98128
}
99129

100-
impl CertStoreBuilder {
101-
fn parse_cert<'c, C, P>(mut self, cert: C, parser: P) -> Self
102-
where
103-
C: Into<CertificateInput<'c>>,
104-
P: Fn(&'c [u8]) -> Result<Certificate>,
105-
{
106-
if let Ok(ref mut builder) = self.builder {
107-
let input = cert.into();
108-
let result = input
109-
.with_parser(parser)
110-
.and_then(|cert| builder.add_cert(cert.0).map_err(Error::tls));
111-
112-
if let Err(err) = result {
113-
self.builder = Err(err);
114-
}
115-
}
116-
self
117-
}
118-
119-
fn parse_certs<'c, I>(mut self, certs: I, parser: fn(&'c [u8]) -> Result<Certificate>) -> Self
120-
where
121-
I: IntoIterator,
122-
I::Item: Into<CertificateInput<'c>>,
123-
{
124-
if let Ok(ref mut builder) = self.builder {
125-
let certs = filter_map_certs(certs, parser);
126-
if let Err(err) = process_certs(certs, builder) {
127-
self.builder = Err(err);
128-
}
129-
}
130-
self
131-
}
132-
}
133-
134130
/// A thread-safe certificate store for TLS connections.
135131
///
136132
/// [`CertStore`] manages a collection of trusted certificates used for verifying peer identities.
@@ -147,18 +143,16 @@ impl CertStoreBuilder {
147143
#[derive(Clone)]
148144
pub struct CertStore(pub(in crate::tls) Arc<X509Store>);
149145

150-
// ====== impl CertStore ======
151-
152146
impl CertStore {
153-
/// Creates a new `CertStoreBuilder`.
147+
/// Creates a new [`CertStoreBuilder`].
154148
#[inline]
155149
pub fn builder() -> CertStoreBuilder {
156150
CertStoreBuilder {
157151
builder: X509StoreBuilder::new().map_err(Error::builder),
158152
}
159153
}
160154

161-
/// Creates a new `CertStore` from a collection of DER-encoded certificates.
155+
/// Creates a new [`CertStore`] from a collection of DER-encoded certificates.
162156
#[inline]
163157
pub fn from_der_certs<'c, C>(certs: C) -> Result<CertStore>
164158
where
@@ -170,7 +164,7 @@ impl CertStore {
170164
.map(CertStore)
171165
}
172166

173-
/// Creates a new `CertStore` from a collection of PEM-encoded certificates.
167+
/// Creates a new [`CertStore`] from a collection of PEM-encoded certificates.
174168
#[inline]
175169
pub fn from_pem_certs<'c, C>(certs: C) -> Result<CertStore>
176170
where
@@ -182,7 +176,7 @@ impl CertStore {
182176
.map(CertStore)
183177
}
184178

185-
/// Creates a new `CertStore` from a PEM-encoded certificate stack.
179+
/// Creates a new [`CertStore`] from a PEM-encoded certificate stack.
186180
#[inline]
187181
pub fn from_pem_stack<C>(certs: C) -> Result<CertStore>
188182
where
@@ -193,26 +187,3 @@ impl CertStore {
193187
.map(CertStore)
194188
}
195189
}
196-
197-
impl Default for CertStore {
198-
fn default() -> Self {
199-
#[cfg(feature = "webpki-roots")]
200-
static LOAD_CERTS: std::sync::LazyLock<CertStore> = std::sync::LazyLock::new(|| {
201-
CertStore::builder()
202-
.add_der_certs(webpki_root_certs::TLS_SERVER_ROOT_CERTS)
203-
.build()
204-
.expect("failed to load default cert store")
205-
});
206-
207-
#[cfg(not(feature = "webpki-roots"))]
208-
{
209-
CertStore::builder()
210-
.set_default_paths()
211-
.build()
212-
.expect("failed to load default cert store")
213-
}
214-
215-
#[cfg(feature = "webpki-roots")]
216-
LOAD_CERTS.clone()
217-
}
218-
}

0 commit comments

Comments
 (0)