pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
91 stars 46 forks source link

`WithSslVerificationDisabled` doesn't disable ssl verification #387

Open mefellows opened 4 months ago

mefellows commented 4 months ago

See https://github.com/pact-foundation/pact-net/issues/488.

I'm wondering if this ever worked. Given that we use rustls-tls-native-roots, the reqwest library seems to only support for other types:

    /// Controls the use of certificate validation.
    ///
    /// Defaults to `false`.
    ///
    /// # Warning
    ///
    /// You should think very carefully before using this method. If
    /// invalid certificates are trusted, *any* certificate for *any* site
    /// will be trusted for use. This includes expired certificates. This
    /// introduces significant vulnerabilities, and should only be used
    /// as a last resort.
    ///
    /// # Optional
    ///
    /// This requires the optional `default-tls`, `native-tls`, or `rustls-tls(-...)`
    /// feature to be enabled.
    #[cfg(feature = "__tls")]
    #[cfg_attr(
        docsrs,
        doc(cfg(any(
            feature = "default-tls",
            feature = "native-tls",
            feature = "rustls-tls"
        )))
    )]
    pub fn danger_accept_invalid_certs(mut self, accept_invalid_certs: bool) -> ClientBuilder {
        self.config.certs_verification = !accept_invalid_certs;
        self
    }

Am I reading this code correctly?

rholshausen commented 4 months ago

This just disables the TLS certificate verification (hostname and expired status checks). But you still need a non-garbage cert to do TLS. The downstream problem seems to be with dodgy certs in the cert store.

mefellows commented 4 months ago

That might be true, but the question is - does the code take effect if there was a non-garbage but invalid/self-signed cert?

The upstream bug might be due to a garbage cert (it's hard to tell without the cert)