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

WithSslVerificationDisabled doesn't disable ssl verification #387

Open
mefellows opened this issue Feb 13, 2024 · 2 comments
Open

WithSslVerificationDisabled doesn't disable ssl verification #387

mefellows opened this issue Feb 13, 2024 · 2 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@mefellows
Copy link
Member

mefellows commented Feb 13, 2024

See pact-foundation/pact-net#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?

@mefellows mefellows added the bug Indicates an unexpected problem or unintended behavior label Feb 13, 2024
@rholshausen
Copy link
Contributor

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
Copy link
Member Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants