verification: enable ip subj. validation on android.

While the Android verifier defers to the platform verifier for most
certificate validation options, it relies on `webpki` for ensuring that
a certificate is valid for a given subject name. Since Webpki v0.100.x
it's been possible to use `webpki` to validate IP address subject names,
and so we want this capability to be enabled for the Android verifier.

This commit updates the Android `verifier` to accomplish this.
Additionally it enables the mock IP address subject test cases in the
mock test suite to ensure things work as expected.

In order to support this change the function signature of the inner
`Verifier.verify_certificate` fn has to change from accepting a `&str`
server name, to also accepting the `&rustls::ServerName` that the trait
based `ServerCertVerifier.verify_server_cert` was already accepting.
There are two main reasons for this:

1. If we try to pull out a `String` to pass forward from the
   `rustls::ServerName::IpAddress(&IpAddr)` using `IpAddr.to_string()`
   we'll get back a "compressed" address for IPv6 addresses. This is
   problematic when later trying to convert to a `webpki::IpAddrRef` for
   the validation call using `IpAddrRef::try_from_ascii_str`, because it
   doesn't support the compressed form.
2. By passing through the `rustls::ServerName` directly we can defer the
   actual process of interacting with `webpki` to the newly exposed
   `rustls::client::verify_server_name` fn offered with the
   `dangerous_configuration` feature. This will ensure the logic for
   name validation is applied consistently between Rustls and the
   platform verifier.

Author: cpu

src/tests/verification_mock/mod.rs

@@ -72,15 +72,11 @@ macro_rules! no_error {
 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");
-#[cfg(any(windows, target_os = "macos", target_os = "linux"))]
 const ROOT1_INT1_LOCALHOST_IPV4_GOOD: &[u8] = include_bytes!("root1-int1-ee_127.0.0.1-good.crt");
-#[cfg(any(windows, target_os = "macos", target_os = "linux"))]
 const ROOT1_INT1_LOCALHOST_IPV6_GOOD: &[u8] = include_bytes!("root1-int1-ee_1-good.crt");
 
 const EXAMPLE_COM: &str = "example.com";
-#[cfg(any(windows, target_os = "macos", target_os = "linux"))]
 const LOCALHOST_IPV4: &str = "127.0.0.1";
-#[cfg(any(windows, target_os = "macos", target_os = "linux"))]
 const LOCALHOST_IPV6: &str = "::1";
 
 #[cfg(any(test, feature = "ffi-testing"))]
@@ -127,35 +123,35 @@ mock_root_test_cases! {
         expected_result: Ok(()),
         other_error: no_error!(),
     },
-    valid_no_stapling_ipv4 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    valid_no_stapling_ipv4 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => 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", target_os = "linux") ] => TestCase {
+    valid_no_stapling_ipv6 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => 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 {
+    valid_stapled_good_dns [ any(windows, target_os = "android", 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", target_os = "linux") ] => TestCase {
+    valid_stapled_good_ipv4 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => 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", target_os = "linux") ] => TestCase {
+    valid_stapled_good_ipv6 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: LOCALHOST_IPV6,
         chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD, ROOT1_INT1],
         stapled_ocsp: Some(include_bytes!("root1-int1-ee_1-good.ocsp")),
@@ -173,14 +169,14 @@ mock_root_test_cases! {
         expected_result: Err(TlsError::InvalidCertificate(CertificateError::Revoked)),
         other_error: no_error!(),
     },
-    stapled_revoked_ipv4 [ any(windows, target_os = "macos") ] => TestCase {
+    stapled_revoked_ipv4 [ any(windows, target_os = "android", 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::InvalidCertificate(CertificateError::Revoked)),
         other_error: no_error!(),
     },
-    stapled_revoked_ipv6 [ any(windows, target_os = "macos") ] => TestCase {
+    stapled_revoked_ipv6 [ any(windows, target_os = "android", 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")),
@@ -199,14 +195,14 @@ mock_root_test_cases! {
         expected_result: Err(TlsError::InvalidCertificate(CertificateError::UnknownIssuer)),
         other_error: no_error!(),
     },
-    ee_only_ipv4 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    ee_only_ipv4 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: LOCALHOST_IPV4,
         chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD],
         stapled_ocsp: None,
         expected_result: Err(TlsError::InvalidCertificate(CertificateError::UnknownIssuer)),
         other_error: no_error!(),
     },
-    ee_only_ipv6 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    ee_only_ipv6 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: LOCALHOST_IPV6,
         chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD],
         stapled_ocsp: None,
@@ -221,14 +217,14 @@ mock_root_test_cases! {
         expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
         other_error: no_error!(),
     },
-    domain_mismatch_ipv4 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    domain_mismatch_ipv4 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: "198.168.0.1",
         chain: &[ROOT1_INT1_LOCALHOST_IPV4_GOOD, ROOT1_INT1],
         stapled_ocsp: None,
         expected_result: Err(TlsError::InvalidCertificate(CertificateError::NotValidForName)),
         other_error: no_error!(),
     },
-    domain_mismatch_ipv6 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    domain_mismatch_ipv6 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: "::ffff:c6a8:1",
         chain: &[ROOT1_INT1_LOCALHOST_IPV6_GOOD, ROOT1_INT1],
         stapled_ocsp: None,
@@ -243,15 +239,15 @@ mock_root_test_cases! {
             CertificateError::Other(Arc::from(EkuError)))),
         other_error: Some(EkuError),
     },
-    wrong_eku_ipv4 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    wrong_eku_ipv4 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => 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::InvalidCertificate(
             CertificateError::Other(Arc::from(EkuError)))),
         other_error: Some(EkuError),
     },
-    wrong_eku_ipv6 [ any(windows, target_os = "macos", target_os = "linux") ] => TestCase {
+    wrong_eku_ipv6 [ any(windows, target_os = "android", target_os = "macos", target_os = "linux") ] => TestCase {
         reference_id: LOCALHOST_IPV6,
         chain: &[include_bytes!("root1-int1-ee_1-wrong_eku.crt"), ROOT1_INT1],
         stapled_ocsp: None,

src/verification/android.rs

@@ -4,12 +4,11 @@ use jni::{
     JNIEnv,
 };
 use rustls::Error::InvalidCertificate;
-use rustls::{client::ServerCertVerifier, CertificateError, Error as TlsError};
+use rustls::{client::ServerCertVerifier, CertificateError, Error as TlsError, ServerName};
 use std::time::SystemTime;
 
 use super::{log_server_cert, unsupported_server_name, ALLOWED_EKUS};
 use crate::android::{with_context, CachedClass};
-use crate::verification::invalid_certificate;
 
 static CERT_VERIFIER_CLASS: CachedClass =
     CachedClass::new("org/rustls/platformverifier/CertificateVerifier");
@@ -69,11 +68,12 @@ impl Verifier {
         }
     }
 
-    pub fn verify_certificate(
+    fn verify_certificate(
         &self,
         end_entity: &rustls::Certificate,
         intermediates: &[rustls::Certificate],
-        server_name: &str,
+        server_name: &rustls::ServerName,
+        server_name_str: &str,
         ocsp_response: Option<&[u8]>,
         now: SystemTime,
     ) -> Result<(), TlsError> {
@@ -167,7 +167,7 @@ impl Verifier {
                     VERIFIER_CALL,
                     &[
                         JValue::from(*cx.application_context()),
-                        JValue::from(env.new_string(server_name)?),
+                        JValue::from(env.new_string(server_name_str)?),
                         JValue::from(env.new_string(AUTH_TYPE)?),
                         JValue::from(JObject::from(allowed_ekus)),
                         JValue::from(ocsp_response),
@@ -189,17 +189,10 @@ impl Verifier {
                 match status {
                     VerifierStatus::Ok => {
                         // If everything else was OK, check the hostname.
-                        let cert = webpki::EndEntityCert::try_from(end_entity.as_ref())
-                            .map_err(pki_name_error)?;
-                        let name = webpki::SubjectNameRef::DnsName(
-                            // This unwrap can't fail since it was already validated before.
-                            webpki::DnsNameRef::try_from_ascii_str(server_name).unwrap(),
-                        );
-                        if cert.verify_is_valid_for_subject_name(name).is_ok() {
-                            Ok(())
-                        } else {
-                            Err(InvalidCertificate(CertificateError::NotValidForName))
-                        }
+                        rustls::client::verify_server_name(
+                            &rustls::server::ParsedCertificate::try_from(end_entity)?,
+                            server_name,
+                        )
                     }
                     VerifierStatus::Unavailable => Err(TlsError::General(String::from(
                         "No system trust stores available",
@@ -257,14 +250,6 @@ fn extract_result_info(env: &JNIEnv<'_>, result: JObject<'_>) -> (VerifierStatus
     (status, msg.map(String::from))
 }
 
-fn pki_name_error(error: webpki::Error) -> TlsError {
-    use webpki::Error::*;
-    match error {
-        BadDer | BadDerTime => InvalidCertificate(CertificateError::BadEncoding),
-        e => invalid_certificate(format!("invalid peer certificate: {}", e)),
-    }
-}
-
 impl ServerCertVerifier for Verifier {
     fn verify_server_cert(
         &self,
@@ -280,8 +265,15 @@ impl ServerCertVerifier for Verifier {
     ) -> Result<rustls::client::ServerCertVerified, TlsError> {
         log_server_cert(end_entity);
 
-        let server = match server_name {
-            rustls::ServerName::DnsName(name) => name.as_ref(),
+        // Verify the server name is one that we support and extract a string to use
+        // for the platform verifier call.
+        let ip_name;
+        let server_name_str = match server_name {
+            ServerName::DnsName(dns_name) => dns_name.as_ref(),
+            ServerName::IpAddress(ip_addr) => {
+                ip_name = ip_addr.to_string();
+                &ip_name
+            }
             _ => return Err(unsupported_server_name()),
         };
 
@@ -291,7 +283,14 @@ impl ServerCertVerifier for Verifier {
             None
         };
 
-        match self.verify_certificate(end_entity, intermediates, server, ocsp_data, now) {
+        match self.verify_certificate(
+            end_entity,
+            intermediates,
+            server_name,
+            server_name_str,
+            ocsp_data,
+            now,
+        ) {
             Ok(()) => Ok(rustls::client::ServerCertVerified::assertion()),
             Err(e) => {
                 // This error only tells us what the system errored with, so it doesn't leak anything

src/verification/mod.rs

@@ -57,7 +57,7 @@ fn unsupported_server_name() -> rustls::Error {
 
 // Unknown certificate error shorthand. Used when we need to construct an "Other" certificate
 // error with a platform specific error message.
-#[cfg(any(windows, target_os = "android", target_os = "macos", target_os = "ios"))]
+#[cfg(any(windows, target_os = "macos", target_os = "ios"))]
 fn invalid_certificate(reason: impl Into<String>) -> rustls::Error {
     rustls::Error::InvalidCertificate(rustls::CertificateError::Other(std::sync::Arc::from(
         Box::from(reason.into()),