diff --git a/rustls-platform-verifier/src/verification/windows.rs b/rustls-platform-verifier/src/verification/windows.rs index 23952bfc..af35bd02 100644 --- a/rustls-platform-verifier/src/verification/windows.rs +++ b/rustls-platform-verifier/src/verification/windows.rs @@ -25,22 +25,25 @@ use crate::windows::{ use rustls::{client::ServerCertVerifier, CertificateError, Error as TlsError}; use winapi::{ shared::{ - minwindef::{DWORD, FILETIME, TRUE}, + minwindef::{FILETIME, TRUE}, ntdef::{LPSTR, VOID}, - winerror::{CERT_E_CN_NO_MATCH, CERT_E_INVALID_NAME, CRYPT_E_REVOKED}, + winerror::{ + CERT_E_CN_NO_MATCH, CERT_E_EXPIRED, CERT_E_INVALID_NAME, CERT_E_UNTRUSTEDROOT, + CERT_E_WRONG_USAGE, CRYPT_E_REVOKED, + }, }, um::wincrypt::{ - self, CertAddEncodedCertificateToStore, CertCloseStore, CertFreeCertificateChain, + CertAddEncodedCertificateToStore, CertCloseStore, CertFreeCertificateChain, CertFreeCertificateChainEngine, CertFreeCertificateContext, CertGetCertificateChain, CertOpenStore, CertSetCertificateContextProperty, CertVerifyCertificateChainPolicy, AUTHTYPE_SERVER, CERT_CHAIN_CACHE_END_CERT, CERT_CHAIN_CONTEXT, CERT_CHAIN_PARA, CERT_CHAIN_POLICY_IGNORE_ALL_REV_UNKNOWN_FLAGS, CERT_CHAIN_POLICY_PARA, - CERT_CHAIN_POLICY_SSL, CERT_CHAIN_POLICY_STATUS, CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY, - CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT, CERT_CONTEXT, CERT_OCSP_RESPONSE_PROP_ID, - CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, CERT_STORE_ADD_ALWAYS, - CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, CERT_STORE_PROV_MEMORY, CERT_USAGE_MATCH, - CRYPT_DATA_BLOB, CTL_USAGE, SSL_EXTRA_CERT_CHAIN_POLICY_PARA, USAGE_MATCH_TYPE_AND, - X509_ASN_ENCODING, + CERT_CHAIN_POLICY_SSL, CERT_CHAIN_POLICY_STATUS, + CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT, CERT_CHAIN_REVOCATION_CHECK_END_CERT, + CERT_CONTEXT, CERT_OCSP_RESPONSE_PROP_ID, CERT_SET_PROPERTY_IGNORE_PERSIST_ERROR_FLAG, + CERT_STORE_ADD_ALWAYS, CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, CERT_STORE_PROV_MEMORY, + CERT_USAGE_MATCH, CRYPT_DATA_BLOB, CTL_USAGE, SSL_EXTRA_CERT_CHAIN_POLICY_PARA, + USAGE_MATCH_TYPE_AND, X509_ASN_ENCODING, }, }; @@ -364,13 +367,17 @@ impl CertificateStore { } }; - // `CERT_CHAIN_CACHE_END_CERT` is somewhat cargo-culted from Chromium. - // `CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY` prevents fetching of OCSP - // and CRLs. `CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT` enables - // revocation checking of the entire chain. - const FLAGS: u32 = CERT_CHAIN_CACHE_END_CERT - | CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY - | CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT; + // `CERT_CHAIN_REVOCATION_CHECK_END_CERT` only checks revocation for end cert. See the crate's revocation documentation + // for more details. + // `CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT` accumulates network retrievals timeouts + // to limit network time and improve performance. + // `CERT_CHAIN_CACHE_END_CERT` speeds up the common case of multiple connections to same server. + const FLAGS: u32 = CERT_CHAIN_REVOCATION_CHECK_END_CERT + | CERT_CHAIN_REVOCATION_ACCUMULATIVE_TIMEOUT + | CERT_CHAIN_CACHE_END_CERT; + + // Lowering URL retrieval timeout from default 15s to 10s to account for higher internet speeds + parameters.dwUrlRetrievalTimeout = 10 * 1000; // milliseconds // SAFETY: `cert` points to a valid certificate context, parameters is valid for reads, `cert_chain` is valid // for writes, and the certificate store is valid and initialized. @@ -387,18 +394,12 @@ impl CertificateStore { ) }; - let cert_chain = call_with_last_error(|| match nonnull_from_const_ptr(cert_chain) { - Some(c) if res == TRUE => Some(c), + // XXX: Windows will internally map the chain's `TrustStatus.dwErrorStatus` to a `dwError` when + // a chain policy is verified, so we only check for errors there. + call_with_last_error(|| match nonnull_from_const_ptr(cert_chain) { + Some(c) if res == TRUE => Some(CertChain { inner: c }), _ => None, - })?; - - { - // SAFETY: `cert_chain` was just initialized with a valid pointer. - let cert_chain: &_ = unsafe { cert_chain.as_ref() }; - map_trust_error_status(cert_chain.TrustStatus.dwErrorStatus)?; - } - - Ok(CertChain { inner: cert_chain }) + }) } } @@ -412,81 +413,6 @@ fn call_with_last_error Option>(mut call: F) -> Result Result<(), TlsError> { - // Returns true if the only bitflags set in |status| are in |flags| - // and at least one of the bitflags in |flags| is set in |status|. - #[must_use] - fn only_flags_set(status: DWORD, flags: DWORD) -> bool { - ((status & !flags) == 0) && ((status & flags) != 0) - } - - // Ignore errors related to missing revocation info, so that a network - // partition between the client and the CA's OCSP/CRL servers does not - // cause the connection to fail. - // - // Unlike Chromium, we don't differentiate between "no revocation mechanism" - // and "unable to check revocation." - const UNABLE_TO_CHECK_REVOCATION: DWORD = - wincrypt::CERT_TRUST_REVOCATION_STATUS_UNKNOWN | wincrypt::CERT_TRUST_IS_OFFLINE_REVOCATION; - let status = unfiltered_status & !UNABLE_TO_CHECK_REVOCATION; - - // If there are no errors, then we're done. - if status == wincrypt::CERT_TRUST_NO_ERROR { - return Ok(()); - } - - // Windows may return multiple errors (webpki only returns one). Rustls - // only allows a single error to be returned. Group the errors into - // classes roughly similar to the ones used by Chromium, and then - // choose what to do based on the class. - - // If the certificate is revoked, then return that, ignoring other errors, - // as we consider revocation to be super critical. - if (status & wincrypt::CERT_TRUST_IS_REVOKED) != 0 { - return Err(InvalidCertificate(CertificateError::Revoked)); - } - - if only_flags_set( - status, - wincrypt::CERT_TRUST_IS_NOT_VALID_FOR_USAGE - | wincrypt::CERT_TRUST_CTL_IS_NOT_VALID_FOR_USAGE, - ) { - return Err(InvalidCertificate(CertificateError::Other( - std::sync::Arc::new(super::EkuError), - ))); - } - - // Otherwise, if there is only one class of error, map that class to - // a well-known error string (for testing and debugging). - if only_flags_set( - status, - wincrypt::CERT_TRUST_IS_NOT_TIME_VALID | wincrypt::CERT_TRUST_CTL_IS_NOT_TIME_VALID, - ) { - return Err(InvalidCertificate(CertificateError::Expired)); - } - - // XXX: winapi doesn't expose this. - const CERT_TRUST_IS_EXPLICIT_DISTRUST: DWORD = 0x04000000; - if only_flags_set( - status, - wincrypt::CERT_TRUST_IS_UNTRUSTED_ROOT - | CERT_TRUST_IS_EXPLICIT_DISTRUST - | wincrypt::CERT_TRUST_IS_PARTIAL_CHAIN, - ) { - return Err(InvalidCertificate(CertificateError::UnknownIssuer)); - } - - // Return an error that contains exactly what Windows told us. - Err(invalid_certificate(format!( - "Bad certificate chain with Windows error status {}", - unfiltered_status - ))) -} - /// A TLS certificate verifier that utilizes the Windows certificate facilities. #[derive(Default)] pub struct Verifier { @@ -579,6 +505,9 @@ impl Verifier { InvalidCertificate(CertificateError::NotValidForName) } CRYPT_E_REVOKED => InvalidCertificate(CertificateError::Revoked), + CERT_E_EXPIRED => InvalidCertificate(CertificateError::Expired), + CERT_E_UNTRUSTEDROOT => InvalidCertificate(CertificateError::UnknownIssuer), + CERT_E_WRONG_USAGE => InvalidCertificate(CertificateError::InvalidPurpose), error_num => { let err = std::io::Error::from_raw_os_error(error_num); // The included error message has both the description and raw OS error code.