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

Enable stapled OCSP verification test, investigate Windows verifier OCSP staple handling #51

Open
cpu opened this issue Jan 3, 2024 · 2 comments

Comments

@cpu
Copy link
Member

cpu commented Jan 3, 2024

After #50 lands we should be able to enable the stapled OCSP test in the real world verification test suite:

// OCSP stapling works.
//
// XXX: This test is commented-out because it is a time-bomb due to the
// short lifetime of the OCSP responses for the certificate.
//
// TODO: If/when we can validate a certificate for a specific point in time
// during a test, re-enable this and have it test the certificate validity
// at a point in time where the OCSP response is valid.
//
// revoked_badssl_com_stapled => TestCase {
// reference_id: "revoked.badssl.com",
// chain: &[
// include_bytes!("revoked_badssl_com_1.crt"),
// include_bytes!("revoked_badssl_com_2.crt"),
// ],
// stapled_ocsp: Some(include_bytes!("revoked_badssl_com_1.ocsp")),
// // XXX: We only do OCSP stapling on Windows.
// valid: !cfg!(windows),
// },

As described in this comment (which should also be fixed up) this was commented out when it wasn't possible to specify a time to use for verification to avoid flakes from the very short OCSP response validity period.

We know that Webpki doesn't support revocation checking via stapled OCSP (see rustls/webpki#217) so we will need to cfg gate the expected result to only assert a revocation error result for non-Linux/WASM platforms - something like:

revoked_badssl_com_stapled => TestCase {
        reference_id: "revoked.badssl.com",
        chain: &[
            include_bytes!("revoked_badssl_com_1.crt"),
            include_bytes!("revoked_badssl_com_2.crt"),
        ],
        stapled_ocsp: Some(include_bytes!("revoked_badssl_com_1.ocsp")),
        // Note: the vendored revoked badssl cert and OCSP response expired ~Dec 9 2021,
        //    so we use a verification time fixed to Dec 4 02:09:01 2021 UTC
        verification_time: SystemTime::UNIX_EPOCH + Duration::from_secs(1_638_583_741),
        #[cfg(not(any(target_os = "linux", target_arch = "wasm32")))]
        expected_result: Err(TlsError::InvalidCertificate(CertificateError::Revoked)),
        #[cfg(any(target_os = "linux", target_arch = "wasm32"))]
        expected_result: Ok(()), // https://github.com/rustls/webpki/issues/217
        other_error: no_error!(),
    },

However, it appears the Windows verifier is returning Ok(()) where Err(TlsError::InvalidCertificate(CertificateError::Revoked)) is expected. Further investigation is required.

@cpu
Copy link
Member Author

cpu commented Jan 3, 2024

As a quick test I also tried going back to before #17 landed and enabling the test with a fixed verification time and got the same result so whatever the root cause I don't think it's a regression from those changes.

@complexspaces
Copy link
Collaborator

I took a look through this one today and here's what I've found so far:

  • Apple uses our evaluation datetime when checking revocation through any method (CRL or OSCP). Running the above test passes on my M1 and a dive into their OSS Security code shows them passing the verifyDate into revocation handling. I can't quite nail down exactly where its used though.
  • Windows: I'm not sure if this is doable yet. The docs state that the pTime we pass in "...does not affect trust list, revocation, or root store checking." I haven't been able to find any flags to overwrite that either.
  • Android should support this, and assuming the expired root doesn't get in the way (see Android verifier doesn't consider supplied date for revocation or intermediate roots #59), you get the error you would expect: certificate was revoked: java.security.cert.CertPathValidatorException: timestamp check failed. I haven't run it through a debugger yet though because it does that regardless of it I give the OSCP checker the right timestamp or not.

Tl;dr the platforms are a mess again, what a surprise right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants