rustls / rustls-platform-verifier

A certificate verification library for rustls that uses the operating system's verifier
Apache License 2.0
54 stars 18 forks source link

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

Open cpu opened 6 months ago

cpu commented 6 months ago

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 commented 6 months ago

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.

complexspaces commented 6 months ago

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

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