rustls-platform-verifier icon indicating copy to clipboard operation
rustls-platform-verifier copied to clipboard

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

Open cpu opened this issue 1 year ago • 2 comments
trafficstars

After https://github.com/rustls/rustls-platform-verifier/pull/50 lands we should be able to enable the stapled OCSP test in the real world verification test suite: https://github.com/rustls/rustls-platform-verifier/blob/65b2a97aff062585d91c97ae3b7b1d17fbcd7b62/rustls-platform-verifier/src/tests/verification_real_world/mod.rs#L221-L239

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 https://github.com/rustls/webpki/issues/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 avatar Jan 03 '24 20:01 cpu

As a quick test I also tried going back to before https://github.com/rustls/rustls-platform-verifier/pull/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.

cpu avatar Jan 03 '24 22:01 cpu

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 #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?

complexspaces avatar Jan 05 '24 03:01 complexspaces