Skip to content

Commit

Permalink
deps: update rustls 0.20.0 -> 0.21.0.
Browse files Browse the repository at this point in the history
Update rustls dependency from 0.20.0 to 0.21.0.

Also patches transitive deps on rustls-native-certs, webpki-roots,
reqwest, hyper-rustls and tokio-rustls to use releases that also depend
on rustls 0.21.0.

Most notable for this codebase, the
`rustls::Error::InvalidCertificateData` has been removed and replaced
by `InvalidCertificate` and a number of `CertificateError` sub-variants.

Now that the upstream `InvalidCertificate` error offers a way to specify
what went wrong with more granularity we're able to trim most of the
`error_messages` consts, choosing instead to return the more specific
upstream error types.

For the case where invalid extensions are detected we define our own
type to use for this error case.

In all other circumstances where we want to return an
`InvalidCertificate` error with some platform specific error message we
use the `InvalidCertificate(CertificateError::Other)` variant with the
error message, repurposing the `invalid_certificate` helper for
constructing an `Arc` over a `Box` with the error message.

We also have to take some special care when asserting the equality of
errors, handling the case where looking at a `CertificateError::Other`
specially, since it can box a dyn error that can't be compared directly
without downcasting the error to a concrete type.
  • Loading branch information
cpu authored and complexspaces committed May 17, 2023
1 parent 128f700 commit c18e462
Show file tree
Hide file tree
Showing 10 changed files with 429 additions and 274 deletions.
392 changes: 228 additions & 164 deletions Cargo.lock

Large diffs are not rendered by default.

11 changes: 6 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ cert-logging = ["base64"]
docsrs = ["jni", "once_cell"]

[dependencies]
rustls = { version = "0.20", features = ["dangerous_configuration", "tls12", "logging"] }
rustls = { version = "0.21", features = ["dangerous_configuration", "tls12", "logging"] }
log = { version = "0.4" }
base64 = { version = "0.13", optional = true } # Only used when the `cert-logging` feature is enabled.
jni = { version = "0.19", default-features = false, optional = true } # Only used during doc generation
Expand All @@ -46,17 +46,18 @@ once_cell = { version = "1.9", optional = true } # Only used during doc generati
[target.'cfg(target_os = "linux")'.dependencies]
rustls-native-certs = "0.6"
once_cell = "1.9"
webpki-roots = "0.22" # Augmentation to `openssl-probe` due to inconsistencies on Linux distros.
webpki-roots = "0.23" # Augmentation to `openssl-probe` due to inconsistencies on Linux distros.
webpki = { package = "rustls-webpki", version = "0.100.0", features = ["alloc", "std"] }

[target.'cfg(target_os = "android")'.dependencies]
jni = { version = "0.19", default-features = false }
webpki = "0.22"
webpki = { package = "rustls-webpki", version = "0.100.0", features = ["alloc", "std"] }
once_cell = "1.9"
android_logger = { version = "0.11", optional = true } # Only used during testing.

[target.'cfg(target_arch = "wasm32")'.dependencies]
once_cell = "1.9"
webpki-roots = "0.22"
webpki-roots = "0.23"

[target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies]
core-foundation = "0.9"
Expand All @@ -69,7 +70,7 @@ winapi = { version = "0.3", features = ["wincrypt", "winerror"] }

[dev-dependencies]
tokio = { version = "1.5", features = ["macros", "rt-multi-thread"] }
reqwest = { version = "0.11", default-features = false, features = ["rustls-tls-manual-roots"] }
reqwest = { version = "0.11.18", default-features = false, features = ["rustls-tls-manual-roots"] }

[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "docsrs"]
Expand Down
33 changes: 31 additions & 2 deletions src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
#[cfg(feature = "ffi-testing")]
pub mod ffi;

use std::error::Error as StdError;

mod verification_real_world;

mod verification_mock;

struct TestCase<'a> {
use rustls::{CertificateError, Error as TlsError, Error::InvalidCertificate};

struct TestCase<'a, E: StdError> {
/// The name of the server we're connecting to.
pub reference_id: &'a str,

Expand All @@ -15,5 +19,30 @@ struct TestCase<'a> {
/// The stapled OCSP response given to us by Rustls, if any.
pub stapled_ocsp: Option<&'a [u8]>,

pub expected_result: Result<(), rustls::Error>,
pub expected_result: Result<(), TlsError>,

/// An error that should be present inside an expected `CertificateError::Other` variant.
///
/// Set this if the error being tested uses `CertificateError::Other` and not statically known
/// variants in [TlsError]
#[allow(dead_code)]
pub other_error: Option<E>,
}

pub fn assert_cert_error_eq<E: StdError + PartialEq + 'static>(
result: &Result<(), TlsError>,
expected: &Result<(), TlsError>,
expected_err: Option<&E>,
) {
// If the expected error is an "Other" CertificateError we can't directly assert equality, we rely
// on the test caller to provide the correct value to compare.
if let Err(InvalidCertificate(CertificateError::Other(err))) = &expected {
let expected_err = expected_err.expect("error not provided for `Other` case handling");
let err: &E = err
.downcast_ref()
.expect("incorrect `Other` inner error kind");
assert_eq!(err, expected_err);
} else {
assert_eq!(result, expected);
}
}
72 changes: 53 additions & 19 deletions src/tests/verification_mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
))]

use super::TestCase;
use crate::verification::{error_messages, Verifier};
use rustls::{client::ServerCertVerifier, Error as TlsError};
use crate::tests::assert_cert_error_eq;
use crate::verification::{EkuError, Verifier};
use rustls::{client::ServerCertVerifier, CertificateError, Error as TlsError};
use std::convert::TryFrom;
use std::net::IpAddr;
use std::sync::Arc;

macro_rules! mock_root_test_cases {
{ $( $name:ident [ $target:meta ] => $test_case:expr ),+ , } => {
Expand Down Expand Up @@ -61,6 +63,12 @@ macro_rules! mock_root_test_cases {
};
}

macro_rules! no_error {
() => {
None::<std::convert::Infallible>
};
}

const ROOT1: &[u8] = include_bytes!("root1.crt");
const ROOT1_INT1: &[u8] = include_bytes!("root1-int1.crt");
const ROOT1_INT1_EXAMPLE_COM_GOOD: &[u8] = include_bytes!("root1-int1-ee_example.com-good.crt");
Expand Down Expand Up @@ -96,9 +104,9 @@ pub(super) fn verification_without_mock_root() {

assert_eq!(
result.map(|_| ()),
Err(TlsError::InvalidCertificateData(String::from(
error_messages::UNKNOWN_CERT
)))
Err(TlsError::InvalidCertificate(
CertificateError::UnknownIssuer
))
);
}

Expand All @@ -117,56 +125,65 @@ mock_root_test_cases! {
chain: &[ROOT1_INT1_EXAMPLE_COM_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
valid_no_stapling_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV4,
chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
valid_no_stapling_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV6,
chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
valid_stapled_good_dns [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
reference_id: EXAMPLE_COM,
chain: &[ROOT1_INT1_EXAMPLE_COM_GOOD, ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_example.com-good.ocsp")),
expected_result: Ok(()),
other_error: no_error!(),
},
valid_stapled_good_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV4,
chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD, ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_127.0.0.1-good.ocsp")),
expected_result: Ok(()),
other_error: no_error!(),
},
valid_stapled_good_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV6,
chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD, ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_1-good.ocsp")),
expected_result: Ok(()),
other_error: no_error!(),
},
// Uses a separate certificate from the one used in the "good" case to deal
// with operating systems with validation data caches (e.g. Windows).
stapled_revoked_dns [ any(windows, target_os = "android", target_os = "macos") ] => TestCase {
reference_id: EXAMPLE_COM,
chain: &[include_bytes!("root1-int1-ee_example.com-revoked.crt"), ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_example.com-revoked.ocsp")),
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::REVOKED))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::Revoked)),
other_error: no_error!(),
},
stapled_revoked_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV4,
chain: &[include_bytes!("root1-int1-ee_127.0.0.1-revoked.crt"), ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_127.0.0.1-revoked.ocsp")),
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::REVOKED))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::Revoked)),
other_error: no_error!(),
},
stapled_revoked_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV6,
chain: &[include_bytes!("root1-int1-ee_1-revoked.crt"), ROOT1_INT1],
stapled_ocsp: Some(include_bytes!("root1-int1-ee_1-revoked.ocsp")),
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::REVOKED))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::Revoked)),
other_error: no_error!(),
},
// Validation fails with no intermediate (that can't be fetched
// with AIA because there's no AIA issuer field in the certificate).
Expand All @@ -177,60 +194,72 @@ mock_root_test_cases! {
reference_id: EXAMPLE_COM,
chain: &[ROOT1_INT1_EXAMPLE_COM_GOOD],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::UNKNOWN_CERT))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::UnknownIssuer)),
other_error: no_error!(),
},
ee_only_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV4,
chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::UNKNOWN_CERT))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::UnknownIssuer)),
other_error: no_error!(),
},
ee_only_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV6,
chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::UNKNOWN_CERT))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::UnknownIssuer)),
other_error: no_error!(),
},
// Validation fails when the certificate isn't valid for the reference ID.
domain_mismatch_dns [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
reference_id: "example.org",
chain: &[ROOT1_INT1_EXAMPLE_COM_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::WRONG_NAME))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
other_error: no_error!(),
},
domain_mismatch_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: "198.168.0.1",
chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::WRONG_NAME))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
other_error: no_error!(),
},
domain_mismatch_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: "::ffff:c6a8:1",
chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD, ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::WRONG_NAME))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
other_error: no_error!(),
},
wrong_eku_dns [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
reference_id: EXAMPLE_COM,
chain: &[include_bytes!("root1-int1-ee_example.com-wrong_eku.crt"), ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::INVALID_EXTENSIONS))),
expected_result: Err(TlsError::InvalidCertificate(
CertificateError::Other(Arc::from(EkuError)))),
other_error: Some(EkuError),
},
wrong_eku_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV4,
chain: &[include_bytes!("root1-int1-ee_127.0.0.1-wrong_eku.crt"), ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::INVALID_EXTENSIONS))),
expected_result: Err(TlsError::InvalidCertificate(
CertificateError::Other(Arc::from(EkuError)))),
other_error: Some(EkuError),
},
wrong_eku_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
reference_id: LOCALHOST_IPV6,
chain: &[include_bytes!("root1-int1-ee_1-wrong_eku.crt"), ROOT1_INT1],
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::INVALID_EXTENSIONS))),
expected_result: Err(TlsError::InvalidCertificate(
CertificateError::Other(Arc::from(EkuError)))),
other_error: Some(EkuError),
},
}

fn test_with_mock_root(test_case: &TestCase) {
fn test_with_mock_root<E: std::error::Error + PartialEq + 'static>(test_case: &TestCase<E>) {
log::info!("verifying {:?}", test_case.expected_result);

let verifier = Verifier::new_with_fake_root(ROOT1); // TODO: time
Expand Down Expand Up @@ -264,6 +293,11 @@ fn test_with_mock_root(test_case: &TestCase) {
test_case.stapled_ocsp.unwrap_or(&[]),
std::time::SystemTime::now(),
);
assert_eq!(result.map(|_| ()), test_case.expected_result);

assert_cert_error_eq(
&result.map(|_| ()),
&test_case.expected_result,
test_case.other_error.as_ref(),
);
// TODO: get into specifics of errors returned when it fails.
}
39 changes: 22 additions & 17 deletions src/tests/verification_real_world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
//! Thus we don't expect these tests to be flaky w.r.t. that, except for
//! potentially poor performance.
use super::TestCase;
use crate::verification::error_messages;
use rustls::Error as TlsError;
use crate::tests::assert_cert_error_eq;
use rustls::{CertificateError, Error as TlsError};
use std::convert::TryFrom;

const SHARED_CHAIN: &[&[u8]] = &[
Expand Down Expand Up @@ -113,7 +113,13 @@ macro_rules! real_world_test_cases {
}
}

fn real_world_test(test_case: &TestCase) {
macro_rules! no_error {
() => {
None::<std::convert::Infallible>
};
}

fn real_world_test<E: std::error::Error>(test_case: &TestCase<E>) {
log::info!("verifying {:?}", test_case.expected_result);

let verifier = crate::verifier_for_testing();
Expand Down Expand Up @@ -141,18 +147,11 @@ fn real_world_test(test_case: &TestCase) {
)
.map(|_| ());

// Note: Linux is special-cased becuse it uses the defaukt `webpki` verifier, meaning
// that we have no control over the error strings used.
if test_case.expected_result.is_err() && cfg!(target_os = "linux") {
assert_eq!(
result,
Err(TlsError::InvalidCertificateData(String::from(
"invalid peer certificate: CertNotValidForName"
)))
)
} else {
assert_eq!(result.map(|_| ()), test_case.expected_result);
}
assert_cert_error_eq(
&result.map(|_| ()),
&test_case.expected_result,
None::<&std::convert::Infallible>,
);
// TODO: get into specifics of errors returned when it fails.
}

Expand All @@ -165,27 +164,31 @@ real_world_test_cases! {
chain: VALID_1PASSWORD_COM_CHAIN,
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
// Same as above but without stapled OCSP.
my_1password_com_valid_no_stapled => TestCase {
reference_id: MY_1PASSWORD_COM,
chain: VALID_1PASSWORD_COM_CHAIN,
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
// Valid also for 1password.com (no subdomain).
_1password_com_valid => TestCase {
reference_id: "1password.com",
chain: VALID_1PASSWORD_COM_CHAIN,
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
// The certificate isn't valid for an unrelated subdomain.
unrelated_domain_invalid => TestCase {
reference_id: VALID_UNRELATED_DOMAIN,
chain: VALID_1PASSWORD_COM_CHAIN,
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::WRONG_NAME))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
other_error: no_error!(),
},
// The certificate chain for the unrelated domain is valid for that
// unrelated domain.
Expand All @@ -194,14 +197,16 @@ real_world_test_cases! {
chain: VALID_UNRELATED_CHAIN,
stapled_ocsp: None,
expected_result: Ok(()),
other_error: no_error!(),
},
// The certificate chain for the unrelated domain is not valid for
// my.1password.com.
unrelated_chain_not_valid_for_my_1password_com => TestCase {
reference_id: MY_1PASSWORD_COM,
chain: VALID_UNRELATED_CHAIN,
stapled_ocsp: None,
expected_result: Err(TlsError::InvalidCertificateData(String::from(error_messages::WRONG_NAME))),
expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
other_error: no_error!(),
},

// OCSP stapling works.
Expand Down
Loading

0 comments on commit c18e462

Please sign in to comment.