Changing default settings for CertGetCertificateChain

[nnmkhang]

Changing default settings for CertGetCertificateChain:

• Changing flags
• Decreasing timeout
• Remove error bit checking
• Adding new errors for Verifier

[complexspaces]

Hey there, thanks for the PR @nnmkhang! As a heads up, you'll need to rebase your PR onto main because the error mapping changed with the last rustls update.

I had a few questions (and comments) about the changes made here since they represent some larger behavior differences and start to make the Windows verifier diverge from the other platforms:

• CERT_CHAIN_CACHE_END_CERT: This is a good catch, I forgot that we were not keeping the certificate store around between certificate validations, so this wouldn't have any benefit. We can always bring it back if/when that is changed.
• CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY: I think that I'm onboard with making this change since the other platform's verifiers are currently permitting networking during verification/revocation, and allows more revocations to occur.
• CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT: Why was this one removed? The other platforms verify the revocation status of both the end-entity and any presented intermediates.

Finally, the error handling changes: These look equivalent in what they catch but could you explain if there are any cases one catches and the other wouldn't? Its not clear to me (after skimming the Win32 docs) what the difference is between checking a certificate's trust status vs getting those forwarded(?) through the policy enforcement.

[nnmkhang]

Hi @complexspaces, thanks for the heads up, I'll work on rebasing the PR. Thanks for your comments. To respond to your question on CERT_CHAIN_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT;

Most CA certs have CRLs with a validity of 1-6 months and on Windows, CRLs are cached so this validity period is too long to have any value. In the scenario where a CA is compromised, it would be added to the Windows Disallowed CTL, which would be checked on every chainbuild.

Additionally, the recommended practice for TLS servers is to support OCSP stapling for the end cert, which in that case no wire retrievals would be necessary unless the OSCP response has expired.

By only checking the leaf cert, we can lower the number of times we hit the wire.

In terms of the error handling changes, there is no difference. The certificate's trust status is what will be forwarded through the policy enforcement.

Hope this makes a little more sense. We're currently making updates to our documentation to give more background on chainbuilding. I will include a PDF that more or less sums up the documentation changes.

Background Info on Cert Chain Building.pdf

[cpu]

Thanks! This looks good to me. I've spent less time in this corner of the platform-verifier code (and with Windows generally) but the rationale for the changes and their implementation makes sense to me.

[complexspaces]

Hey again, I genuinely apologize for the half-year delay getting back to this PR. It fell off my radar during work and I had difficulties setting aside time to look through it in-depth. Today I looked through it and the provided documentation in-depth.

With a bunch more research in November-December about the behavior of revocation across all of our supported platforms (and then the change in #40), I feel much better about the changes being made here to intermediate root revocation. The information in your comment and in the documentation PDF was extraordinarily helpful, so thank you very much for the resources/transparency.

I rebased your branch onto main, formatted it, fixed the broken merge (the extra error mappings are needed), and then cleaned up a bit. Thanks again for your contribution!