Skip to content

Commit 661577e

Browse files
committed
Avoid implicitly relying on default CryptoProvider
1 parent b43a06a commit 661577e

File tree

10 files changed

+97
-145
lines changed

10 files changed

+97
-145
lines changed

rustls-platform-verifier/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
use std::sync::Arc;
66

7+
#[cfg(feature = "dbg")]
8+
use rustls::crypto::CryptoProvider;
79
#[cfg(feature = "dbg")]
810
use rustls::pki_types::CertificateDer;
911
use rustls::{client::WantsClientCert, ClientConfig, ConfigBuilder, WantsVerifier};
@@ -89,8 +91,9 @@ pub fn tls_config_with_provider(
8991
#[cfg(feature = "dbg")]
9092
pub fn verifier_for_dbg(
9193
root: CertificateDer<'static>,
94+
crypto_provider: Arc<CryptoProvider>,
9295
) -> Arc<dyn rustls::client::danger::ServerCertVerifier> {
93-
Arc::new(Verifier::new_with_fake_root(root))
96+
Arc::new(Verifier::new_with_fake_root(root, crypto_provider))
9497
}
9598

9699
/// Extension trait to help configure [`ClientConfig`]s with the platform verifier.
@@ -111,7 +114,7 @@ impl BuilderVerifierExt for ConfigBuilder<ClientConfig, WantsVerifier> {
111114
fn with_platform_verifier(self) -> ConfigBuilder<ClientConfig, WantsClientCert> {
112115
let provider = self.crypto_provider().clone();
113116
self.dangerous()
114-
.with_custom_certificate_verifier(Arc::new(Verifier::new().with_provider(provider)))
117+
.with_custom_certificate_verifier(Arc::new(Verifier::new(provider)))
115118
}
116119
}
117120

rustls-platform-verifier/src/tests/ffi.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ mod android {
4747
.with_filter(log_filter),
4848
);
4949
crate::android::init_with_env(env, cx).unwrap();
50-
crate::tests::ensure_global_state();
5150
std::panic::set_hook(Box::new(|info| {
5251
let msg = if let Some(msg) = info.payload().downcast_ref::<&'static str>() {
5352
msg

rustls-platform-verifier/src/tests/mod.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
#[cfg(feature = "ffi-testing")]
22
pub mod ffi;
33

4-
use std::error::Error as StdError;
54
use std::time::Duration;
5+
use std::{error::Error as StdError, sync::Arc};
66

77
mod verification_real_world;
88

99
mod verification_mock;
1010

11-
use rustls::{pki_types, CertificateError, Error as TlsError, Error::InvalidCertificate};
11+
use rustls::{
12+
crypto::CryptoProvider,
13+
pki_types, CertificateError,
14+
Error::{self as TlsError, InvalidCertificate},
15+
};
1216

1317
struct TestCase<'a, E: StdError> {
1418
/// The name of the server we're connecting to.
@@ -62,6 +66,6 @@ pub(crate) fn verification_time() -> pki_types::UnixTime {
6266
pki_types::UnixTime::since_unix_epoch(Duration::from_secs(1_748_633_220))
6367
}
6468

65-
fn ensure_global_state() {
66-
let _ = rustls::crypto::ring::default_provider().install_default();
69+
fn test_provider() -> Arc<CryptoProvider> {
70+
Arc::new(rustls::crypto::ring::default_provider())
6771
}

rustls-platform-verifier/src/tests/verification_mock/mod.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use rustls::pki_types::{DnsName, ServerName};
3434
use rustls::{CertificateError, Error as TlsError, OtherError};
3535

3636
use super::TestCase;
37-
use crate::tests::{assert_cert_error_eq, ensure_global_state, verification_time};
37+
use crate::tests::{assert_cert_error_eq, test_provider, verification_time};
3838
use crate::verification::{EkuError, Verifier};
3939

4040
macro_rules! mock_root_test_cases {
@@ -94,18 +94,21 @@ const LOCALHOST_IPV6: &str = "::1";
9494
#[cfg(any(test, feature = "ffi-testing"))]
9595
#[cfg_attr(feature = "ffi-testing", allow(dead_code))]
9696
pub(super) fn verification_without_mock_root() {
97-
ensure_global_state();
97+
let crypto_provider = test_provider();
98+
9899
// Since Rustls 0.22 constructing a webpki verifier (like the one backing Verifier on unix
99100
// systems) without any roots produces `OtherError(NoRootAnchors)` - since our FreeBSD CI
100101
// runner fails to find any roots with openssl-probe we need to provide webpki-root-certs here
101102
// or the test will fail with the `OtherError` instead of the expected `CertificateError`.
102103
#[cfg(target_os = "freebsd")]
103-
let verifier =
104-
Verifier::new_with_extra_roots(webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned())
105-
.unwrap();
104+
let verifier = Verifier::new_with_extra_roots(
105+
webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned(),
106+
crypto_provider,
107+
)
108+
.unwrap();
106109

107110
#[cfg(not(target_os = "freebsd"))]
108-
let verifier = Verifier::new();
111+
let verifier = Verifier::new(crypto_provider);
109112

110113
let server_name = pki_types::ServerName::try_from(EXAMPLE_COM).unwrap();
111114
let end_entity = pki_types::CertificateDer::from(ROOT1_INT1_EXAMPLE_COM_GOOD);
@@ -335,13 +338,13 @@ fn test_with_mock_root<E: std::error::Error + PartialEq + 'static>(
335338
test_case: &TestCase<E>,
336339
root_src: Roots,
337340
) {
338-
ensure_global_state();
339341
log::info!("verifying {:?}", test_case.expected_result);
340342

343+
let provider = test_provider();
341344
let verifier = match root_src {
342-
Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1), // TODO: time
345+
Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1, provider), // TODO: time
343346
#[cfg(not(target_os = "android"))]
344-
Roots::ExtraAndPlatform => Verifier::new_with_extra_roots([ROOT1]).unwrap(),
347+
Roots::ExtraAndPlatform => Verifier::new_with_extra_roots([ROOT1], provider).unwrap(),
345348
};
346349
let mut chain = test_case
347350
.chain

rustls-platform-verifier/src/tests/verification_real_world/mod.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use rustls::pki_types::{DnsName, ServerName};
4242
use rustls::{CertificateError, Error as TlsError};
4343

4444
use super::TestCase;
45-
use crate::tests::{assert_cert_error_eq, ensure_global_state, verification_time};
45+
use crate::tests::{assert_cert_error_eq, test_provider, verification_time};
4646
use crate::Verifier;
4747

4848
// This is the certificate chain presented by one server for
@@ -120,22 +120,25 @@ macro_rules! no_error {
120120
}
121121

122122
fn real_world_test<E: std::error::Error>(test_case: &TestCase<E>) {
123-
ensure_global_state();
124123
log::info!(
125124
"verifying ref ID {:?} expected {:?}",
126125
test_case.reference_id,
127126
test_case.expected_result
128127
);
129128

129+
let crypto_provider = test_provider();
130+
130131
// On BSD systems openssl-probe fails to find the system CA bundle,
131132
// so we must provide extra roots from webpki-root-cert.
132133
#[cfg(target_os = "freebsd")]
133-
let verifier =
134-
Verifier::new_with_extra_roots(webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned())
135-
.unwrap();
134+
let verifier = Verifier::new_with_extra_roots(
135+
webpki_root_certs::TLS_SERVER_ROOT_CERTS.iter().cloned(),
136+
crypto_provider,
137+
)
138+
.unwrap();
136139

137140
#[cfg(not(target_os = "freebsd"))]
138-
let verifier = Verifier::new();
141+
let verifier = Verifier::new(crypto_provider);
139142

140143
let mut chain = test_case
141144
.chain

rustls-platform-verifier/src/verification/android.rs

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use jni::{
55
strings::JavaStr,
66
JNIEnv,
77
};
8-
use once_cell::sync::OnceCell;
98
use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerifier};
109
use rustls::crypto::{verify_tls12_signature, verify_tls13_signature, CryptoProvider};
1110
use rustls::pki_types;
@@ -47,13 +46,7 @@ pub struct Verifier {
4746
/// Testing only: The root CA certificate to trust.
4847
#[cfg(any(test, feature = "ffi-testing"))]
4948
test_only_root_ca_override: Option<pki_types::CertificateDer<'static>>,
50-
pub(super) crypto_provider: OnceCell<Arc<CryptoProvider>>,
51-
}
52-
53-
impl Default for Verifier {
54-
fn default() -> Self {
55-
Self::new()
56-
}
49+
crypto_provider: Arc<CryptoProvider>,
5750
}
5851

5952
#[cfg(any(test, feature = "ffi-testing"))]
@@ -71,24 +64,23 @@ impl Drop for Verifier {
7164
impl Verifier {
7265
/// Creates a new instance of a TLS certificate verifier that utilizes the
7366
/// Android certificate facilities.
74-
///
75-
/// A [`CryptoProvider`] must be set with
76-
/// [`set_provider`][Verifier::set_provider]/[`with_provider`][Verifier::with_provider] or
77-
/// [`CryptoProvider::install_default`] before the verifier can be used.
78-
pub fn new() -> Self {
67+
pub fn new(crypto_provider: Arc<CryptoProvider>) -> Self {
7968
Self {
8069
#[cfg(any(test, feature = "ffi-testing"))]
8170
test_only_root_ca_override: None,
82-
crypto_provider: OnceCell::new(),
71+
crypto_provider,
8372
}
8473
}
8574

8675
/// Creates a test-only TLS certificate verifier which trusts our fake root CA cert.
8776
#[cfg(any(test, feature = "ffi-testing"))]
88-
pub(crate) fn new_with_fake_root(root: pki_types::CertificateDer<'static>) -> Self {
77+
pub(crate) fn new_with_fake_root(
78+
root: pki_types::CertificateDer<'static>,
79+
crypto_provider: Arc<CryptoProvider>,
80+
) -> Self {
8981
Self {
9082
test_only_root_ca_override: Some(root),
91-
crypto_provider: OnceCell::new(),
83+
crypto_provider,
9284
}
9385
}
9486

@@ -314,7 +306,7 @@ impl ServerCertVerifier for Verifier {
314306
message,
315307
cert,
316308
dss,
317-
&self.get_provider().signature_verification_algorithms,
309+
&self.crypto_provider.signature_verification_algorithms,
318310
)
319311
}
320312

@@ -328,12 +320,12 @@ impl ServerCertVerifier for Verifier {
328320
message,
329321
cert,
330322
dss,
331-
&self.get_provider().signature_verification_algorithms,
323+
&self.crypto_provider.signature_verification_algorithms,
332324
)
333325
}
334326

335327
fn supported_verify_schemes(&self) -> Vec<SignatureScheme> {
336-
self.get_provider()
328+
self.crypto_provider
337329
.signature_verification_algorithms
338330
.supported_schemes()
339331
}

rustls-platform-verifier/src/verification/apple.rs

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::sync::Arc;
22

33
use core_foundation::date::CFDate;
44
use core_foundation_sys::date::kCFAbsoluteTimeIntervalSince1970;
5-
use once_cell::sync::OnceCell;
65
use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerifier};
76
use rustls::crypto::{verify_tls12_signature, verify_tls13_signature, CryptoProvider};
87
use rustls::pki_types;
@@ -50,22 +49,18 @@ pub struct Verifier {
5049
/// Testing only: The root CA certificate to trust.
5150
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
5251
test_only_root_ca_override: Option<SecCertificate>,
53-
pub(super) crypto_provider: OnceCell<Arc<CryptoProvider>>,
52+
crypto_provider: Arc<CryptoProvider>,
5453
}
5554

5655
impl Verifier {
5756
/// Creates a new instance of a TLS certificate verifier that utilizes the Apple certificate
5857
/// facilities.
59-
///
60-
/// A [`CryptoProvider`] must be set with
61-
/// [`set_provider`][Verifier::set_provider]/[`with_provider`][Verifier::with_provider] or
62-
/// [`CryptoProvider::install_default`] before the verifier can be used.
63-
pub fn new() -> Self {
58+
pub fn new(crypto_provider: Arc<CryptoProvider>) -> Self {
6459
Self {
6560
extra_roots: Vec::new(),
6661
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
6762
test_only_root_ca_override: None,
68-
crypto_provider: OnceCell::new(),
63+
crypto_provider,
6964
}
7065
}
7166

@@ -75,6 +70,7 @@ impl Verifier {
7570
/// See [Verifier::new] for the external requirements the verifier needs.
7671
pub fn new_with_extra_roots(
7772
roots: impl IntoIterator<Item = pki_types::CertificateDer<'static>>,
73+
crypto_provider: Arc<CryptoProvider>,
7874
) -> Result<Self, TlsError> {
7975
let extra_roots = roots
8076
.into_iter()
@@ -87,17 +83,20 @@ impl Verifier {
8783
extra_roots,
8884
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
8985
test_only_root_ca_override: None,
90-
crypto_provider: OnceCell::new(),
86+
crypto_provider,
9187
})
9288
}
9389

9490
/// Creates a test-only TLS certificate verifier which trusts our fake root CA cert.
9591
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
96-
pub(crate) fn new_with_fake_root(root: pki_types::CertificateDer<'static>) -> Self {
92+
pub(crate) fn new_with_fake_root(
93+
root: pki_types::CertificateDer<'static>,
94+
crypto_provider: Arc<CryptoProvider>,
95+
) -> Self {
9796
Self {
9897
extra_roots: Vec::new(),
9998
test_only_root_ca_override: Some(SecCertificate::from_der(root.as_ref()).unwrap()),
100-
crypto_provider: OnceCell::new(),
99+
crypto_provider,
101100
}
102101
}
103102

@@ -283,7 +282,7 @@ impl ServerCertVerifier for Verifier {
283282
message,
284283
cert,
285284
dss,
286-
&self.get_provider().signature_verification_algorithms,
285+
&self.crypto_provider.signature_verification_algorithms,
287286
)
288287
}
289288

@@ -297,19 +296,13 @@ impl ServerCertVerifier for Verifier {
297296
message,
298297
cert,
299298
dss,
300-
&self.get_provider().signature_verification_algorithms,
299+
&self.crypto_provider.signature_verification_algorithms,
301300
)
302301
}
303302

304303
fn supported_verify_schemes(&self) -> Vec<SignatureScheme> {
305-
self.get_provider()
304+
self.crypto_provider
306305
.signature_verification_algorithms
307306
.supported_schemes()
308307
}
309308
}
310-
311-
impl Default for Verifier {
312-
fn default() -> Self {
313-
Self::new()
314-
}
315-
}

rustls-platform-verifier/src/verification/mod.rs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use rustls::crypto::CryptoProvider;
1+
#[cfg(any(windows, target_vendor = "apple"))]
22
use std::sync::Arc;
33

44
#[cfg(all(
@@ -84,30 +84,3 @@ const ALLOWED_EKUS: &[windows_sys::core::PCSTR] =
8484
&[windows_sys::Win32::Security::Cryptography::szOID_PKIX_KP_SERVER_AUTH];
8585
#[cfg(target_os = "android")]
8686
pub const ALLOWED_EKUS: &[&str] = &["1.3.6.1.5.5.7.3.1"];
87-
88-
impl Verifier {
89-
/// Chainable setter to configure the [`CryptoProvider`] for this `Verifier`.
90-
///
91-
/// This will be used instead of the rustls process-default `CryptoProvider`, even if one has
92-
/// been installed.
93-
pub fn with_provider(mut self, crypto_provider: Arc<CryptoProvider>) -> Self {
94-
self.set_provider(crypto_provider);
95-
self
96-
}
97-
98-
/// Configures the [`CryptoProvider`] for this `Verifier`.
99-
///
100-
/// This will be used instead of the rustls process-default `CryptoProvider`, even if one has
101-
/// been installed.
102-
pub fn set_provider(&mut self, crypto_provider: Arc<CryptoProvider>) {
103-
self.crypto_provider = crypto_provider.into();
104-
}
105-
106-
fn get_provider(&self) -> &Arc<CryptoProvider> {
107-
self.crypto_provider.get_or_init(|| {
108-
CryptoProvider::get_default()
109-
.expect("rustls default CryptoProvider not set")
110-
.clone()
111-
})
112-
}
113-
}

0 commit comments

Comments
 (0)