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

Improve sslmode support #11

Open
nyurik opened this issue Feb 12, 2023 · 1 comment
Open

Improve sslmode support #11

nyurik opened this issue Feb 12, 2023 · 1 comment

Comments

@nyurik
Copy link

nyurik commented Feb 12, 2023

It seems practically every user of the postgres lib is struggling with support sslmode=require|verify-ca|verifu-full per pg ssl docs. I believe this requires these new features, and this lib is probably the best place to add them to because they are mostly postgres specific, and because rustls doesn't want them :(

  • improve connection string parsing to detect sslmode parameter.
  • provide a way to skip certificate validation
  • provide a way to skip host validation

The overall expected behavior for sslmode:

let (verify_ca, verify_hostname) = match ssl_mode {
    SslMode::Disable | SslMode::Prefer => (false, false),
    SslMode::Require => match pg_certs.ssl_root_cert {
        // If a root CA file exists, the behavior of sslmode=require will be the same as
        // that of verify-ca, meaning the server certificate is validated against the CA.
        // For more details, check out the note about backwards compatibility in
        // https://postgresql.org/docs/current/libpq-ssl.html#LIBQ-SSL-CERTIFICATES
        // See also notes in
        // https://github.com/sfu-db/connector-x/blob/b26f3b73714259dc55010f2233e663b64d24f1b1/connectorx/src/sources/postgres/connection.rs#L25
        Some(_) => (true, false),
        None => (false, false),  // no root CA file, so no verification
    },
    SslMode::VerifyCa => (true, false),
    SslMode::VerifyFull => (true, true),
};

If verify_ca is false, then we need to provide a way to skip certificate validation. If verify_hostname is false, then we need to provide a way to skip hostname validation.

sslmode parsing

This code works around a current limitation in the postgres SSL parsing, which really should be fixed in sfackler/rust-postgres#988 -- basically the rust-postgres lib should support the additional verify-ca and verify-full values, but without that patch, we can use regex to replace them with required in the connection string. See example in my code

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SslModeOverride {
    Unmodified(SslMode),
    VerifyCa,
    VerifyFull,
}

/// Special treatment for sslmode=verify-ca & sslmode=verify-full - if found, replace them with sslmode=require
pub fn parse_conn_str(conn_str: &str) -> Result<(Config, SslModeOverride)> {
    let mut mode = SslModeOverride::Unmodified(SslMode::Disable);

    let exp = r"(?P<before>(^|\?|&| )sslmode=)(?P<mode>verify-(ca|full))(?P<after>$|&| )";
    let re = Regex::new(exp).unwrap();
    let pg_cfg = if let Some(captures) = re.captures(conn_str) {
        let captured_value = &captures["mode"];
        mode = match captured_value {
            "verify-ca" => SslModeOverride::VerifyCa,
            "verify-full" => SslModeOverride::VerifyFull,
            _ => unreachable!(),
        };
        let conn_str = re.replace(conn_str, "${before}require${after}");
        Config::from_str(conn_str.as_ref())
    } else {
        Config::from_str(conn_str)
    };
    let pg_cfg = pg_cfg.map_err(|e| BadConnectionString(e, conn_str.to_string()))?;
    if let SslModeOverride::Unmodified(_) = mode {
        mode = SslModeOverride::Unmodified(pg_cfg.get_ssl_mode());
    }
    Ok((pg_cfg, mode))
}

enable ssl but ignore cert verification

In OpenSSL, we can use builder.set_verify(SslVerifyMode::NONE), but unfortunately rustls doesn't want to allow this, hence this workaround. See example usage from maplibre/martin#474 pull request.

struct NoCertificateVerification {}

impl rustls::client::ServerCertVerifier for NoCertificateVerification {
    fn verify_server_cert(
        &self,
        _end_entity: &Certificate,
        _intermediates: &[Certificate],
        _server_name: &rustls::ServerName,
        _scts: &mut dyn Iterator<Item = &[u8]>,
        _ocsp: &[u8],
        _now: std::time::SystemTime,
    ) -> std::result::Result<rustls::client::ServerCertVerified, rustls::Error> {
        Ok(rustls::client::ServerCertVerified::assertion())
    }
}

....

    if !verify_ca {
        builder
            .dangerous()
            .set_certificate_verifier(std::sync::Arc::new(NoCertificateVerification {}));
    }

allow connections without hostname validation

OpenSSL supports this code:

    if !verify_hostname {
        connector.set_callback(|cfg, _domain| {
            cfg.set_verify_hostname(false);
            Ok(())
        });
    }

but there is no similar API for rustls. There are some examples in the rustls/rustls#578 issue, but rustls seems not to want to implement/maintain it, so we need a workaround.

@jbg
Copy link
Owner

jbg commented Mar 21, 2023

It would indeed be an improvement if you could just set sslmode in the connection string rather than needing to set up the ClientConfig manually with an appropriate verifier for the desired behaviour. The problem is that the tokio-postgres API doesn't provide any information about the sslmode to the MakeTlsConnect / TlsConnect impls, so the TLS connector has no knowledge of the sslmode defined in the connection string.

If tokio-postgres was to extend the API with this information somehow, it would be possible to have automatic handling of the sslmodes in this crate. (ref sfackler/rust-postgres#988 (comment))

We could also possibly add a convenience method alongside new(config: ClientConfig), which takes an SSL mode and sets up the ClientConfig automatically for the desired mode. This however has the problem of potential confusion if a different SSL mode is passed to tokio-postgres-rustls here than is provided in the connection string.

If tokio-postgres plans to eventually pass the SSL mode through, I would be inclined to wait for that, and in the meantime just add examples to the documentation regarding how to set up ClientConfig for each mode.

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