Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new with extra roots on windows #135

Merged
merged 6 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions rustls-platform-verifier/src/tests/verification_mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ macro_rules! mock_root_test_cases {
}

paste::paste!{
#[cfg(all($target, not(windows), not(target_os = "android")))]
#[cfg(all($target, not(target_os = "android")))]
cpu marked this conversation as resolved.
Show resolved Hide resolved
#[test]
pub fn [<$name _extra>](){
super::[<$name _extra>]()
Expand All @@ -60,7 +60,7 @@ macro_rules! mock_root_test_cases {
$name,

paste::paste!{
#[cfg(all($target, not(windows), not(target_os = "android")))]
#[cfg(all($target, not(target_os = "android")))]
[<$name _extra>]
}

Expand All @@ -77,7 +77,7 @@ macro_rules! mock_root_test_cases {
}

paste::paste!{
#[cfg(all($target, not(windows), not(target_os = "android")))]
#[cfg(all($target, not(target_os = "android")))]
pub(super) fn [<$name _extra>]() {
test_with_mock_root(&$test_case, Roots::ExtraAndPlatform);
}
Expand Down Expand Up @@ -116,7 +116,8 @@ pub(super) fn verification_without_mock_root() {
.iter()
.cloned()
.collect(),
);
)
.unwrap();

#[cfg(not(target_os = "freebsd"))]
let verifier = Verifier::new();
Expand Down Expand Up @@ -336,8 +337,8 @@ fn test_with_mock_root<E: std::error::Error + PartialEq + 'static>(

let verifier = match root_src {
Roots::OnlyExtra => Verifier::new_with_fake_root(ROOT1), // TODO: time
#[cfg(all(unix, not(target_os = "android")))]
Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]),
#[cfg(not(target_os = "android"))]
Roots::ExtraAndPlatform => Verifier::new_with_extra_roots(vec![ROOT1.into()]).unwrap(),
};
let mut chain = test_case
.chain
Expand Down Expand Up @@ -379,6 +380,6 @@ enum Roots {
/// Test with loading the extra roots and the platform trust store.
///
/// Right now, not all platforms are supported.
#[cfg(all(unix, not(target_os = "android")))]
#[cfg(not(target_os = "android"))]
ExtraAndPlatform,
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ fn real_world_test<E: std::error::Error>(test_case: &TestCase<E>) {
.iter()
.cloned()
.collect(),
);
)
.unwrap();

#[cfg(not(target_os = "freebsd"))]
let verifier = Verifier::new();
Expand Down
48 changes: 26 additions & 22 deletions rustls-platform-verifier/src/verification/apple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ fn system_time_to_cfdate(time: pki_types::UnixTime) -> Result<CFDate, TlsError>
pub struct Verifier {
/// Extra trust anchors to add to the verifier above and beyond those provided by
/// the system-provided trust stores.
extra_roots: Vec<pki_types::CertificateDer<'static>>,
extra_roots: Vec<SecCertificate>,
/// Testing only: The root CA certificate to trust.
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
test_only_root_ca_override: Option<Vec<u8>>,
test_only_root_ca_override: Option<SecCertificate>,
pub(super) crypto_provider: OnceCell<Arc<CryptoProvider>>,
}

Expand All @@ -72,21 +72,30 @@ impl Verifier {
/// facilities with the addition of extra root certificates to trust.
///
/// See [Verifier::new] for the external requirements the verifier needs.
pub fn new_with_extra_roots(roots: Vec<pki_types::CertificateDer<'static>>) -> Self {
Self {
extra_roots: roots,
pub fn new_with_extra_roots(
roots: Vec<pki_types::CertificateDer<'static>>,
) -> Result<Self, TlsError> {
cpu marked this conversation as resolved.
Show resolved Hide resolved
let extra_roots = roots
.into_iter()
.map(|root| {
SecCertificate::from_der(&root)
.map_err(|_| TlsError::InvalidCertificate(CertificateError::BadEncoding))
})
.collect::<Result<Vec<_>, _>>()?;
Ok(Self {
extra_roots,
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
test_only_root_ca_override: None,
crypto_provider: OnceCell::new(),
}
})
}

/// Creates a test-only TLS certificate verifier which trusts our fake root CA cert.
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
pub(crate) fn new_with_fake_root(root: &[u8]) -> Self {
Self {
extra_roots: Vec::new(),
test_only_root_ca_override: Some(root.into()),
test_only_root_ca_override: Some(SecCertificate::from_der(root).unwrap()),
crypto_provider: OnceCell::new(),
}
}
Expand Down Expand Up @@ -141,29 +150,24 @@ impl Verifier {
.map_err(|e| invalid_certificate(e.to_string()))?;
}

let raw_extra_roots = self.extra_roots.iter();
#[cfg(not(any(test, feature = "ffi-testing", feature = "dbg")))]
let extra_roots = self.extra_roots.as_slice();

#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
let extra_root = self
.test_only_root_ca_override
.as_ref()
.map(|root| pki_types::CertificateDer::from_slice(root));

let extra_roots: Vec<_> = self
.extra_roots
.iter()
.chain(self.test_only_root_ca_override.as_ref())
.cloned()
.collect();
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
let raw_extra_roots = raw_extra_roots.chain(&extra_root).to_owned();

let extra_roots = raw_extra_roots
.map(|root| {
SecCertificate::from_der(root)
.map_err(|_| TlsError::InvalidCertificate(CertificateError::BadEncoding))
})
.collect::<Result<Vec<_>, _>>()?;
let extra_roots = extra_roots.as_slice();

// If any extra roots were provided by the user (or tests), provide them to the trust
// evaluation regardless of their system trust settings or status.
if !extra_roots.is_empty() {
trust_evaluation
.set_anchor_certificates(&extra_roots)
.set_anchor_certificates(extra_roots)
.map_err(|e| TlsError::Other(OtherError(Arc::new(e))))?;

// We want to trust both the system-installed and the extra roots. This must be set
Expand Down
8 changes: 5 additions & 3 deletions rustls-platform-verifier/src/verification/others.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ impl Verifier {
/// Creates a new verifier whose certificate validation is provided by
/// WebPKI, using root certificates provided by the platform and augmented by
/// the provided extra root certificates.
pub fn new_with_extra_roots(roots: Vec<pki_types::CertificateDer<'static>>) -> Self {
Self {
pub fn new_with_extra_roots(
roots: Vec<pki_types::CertificateDer<'static>>,
) -> Result<Self, TlsError> {
Ok(Self {
inner: OnceCell::new(),
extra_roots: roots
.into_iter()
Expand All @@ -66,7 +68,7 @@ impl Verifier {
#[cfg(any(test, feature = "ffi-testing", feature = "dbg"))]
test_only_root_ca_override: None,
crypto_provider: OnceCell::new(),
}
})
}

/// Creates a test-only TLS certificate verifier which trusts our fake root CA cert.
Expand Down
Loading