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

Changing default settings for CertGetCertificateChain #17

Merged
merged 2 commits into from
Jan 2, 2024
Merged
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
133 changes: 31 additions & 102 deletions rustls-platform-verifier/src/verification/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down Expand Up @@ -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.
Expand All @@ -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 })
})
}
}

Expand All @@ -412,81 +413,6 @@ fn call_with_last_error<T, F: FnMut() -> Option<T>>(mut call: F) -> Result<T, Tl
}
}

// Maps a valid `cert_chain.TrustStatus.dwErrorStatus` to a `TLSError`.
//
// See Chromium's `MapCertChainErrorStatusToCertStatus` in
// https://chromium.googlesource.com/chromium/src/net/+/refs/heads/main/cert/cert_verify_proc_win.cc.
fn map_trust_error_status(unfiltered_status: DWORD) -> 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 {
Expand Down Expand Up @@ -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.
Expand Down
Loading