Openssl hostname verification

[Lorak-mmk]

This PR adds a bugfix / missing functionality (it is unclear to me if this classifies as bug) for hostname verification when using openssl.

Why we had hostname verification for rustls, but not openssl?
To create encrypted async sockets we use tokio-rustls and tokio-openssl crates respectively.

rustls

In rustls we first create tokio_rustls::TlsConnector from rustls::ClientConfig, then we call its connect method, which also takes
a stream and a rustls_pki_types::server_name::ServerName (which is either IP or domain name). This domain name will be used
for hostname verification. That means there is no (easy) way to skip hostname verification when using rustls (and tokio_rustls to wrap the socket).
I think user would need to provide custom certificate verifier in ClientConfig to bypass that.
We always provide node IP as ServerName, so this approach assumes that node always presents a certificate with its IP in SAN field.

openssl

openssl / tokio-openssl API is not so simple.
tokio-openssl has tokio_openssl::SslStream struct, which is created from openssl::ssl::Ssl and a TCP stream.
connect method on this SslStream does not take any hostname. The conclusion is clear: openssl::ssl::Ssl struct that we pass
is responsible for hostname verification, including having a hostname somehow configured.

How does Ssl gets created, and how can hostname verification be configured on it?

Configuring Ssl

I found exactly one way to configure hostname verification on already existing Ssl instance.
To do that, we need to call param_mut() on it, getting &mut X509VerifyParamRef. This has set_ip and set_hostname method.
This is what this PR does. I added a call to set_ip with node ip.
I think you'll agree that this is quite non-obvious approach for something as important as hostname verification, so I thought it must not be the only way.

Creating properly configured Ssl

Ssl can be created either from SslContextRef (this is what the driver uses, because we accept SslContext from the user) or from ConnectConfiguration).

SslContext

The first way (that we use) looks like a simple conversion ( https://docs.rs/openssl/0.10.73/openssl/ssl/struct.Ssl.html#method.new ) so for hostname verification to
be enabled there it would already have to be enabled on SslContext. There is no way to enable it, or provide hostname, on SslContext. It can be done with SslContextBuilder using the similar approach param_mut approach I described above (the method is called verify_param_mut here),
but it is not feasible for us (no way to create builder from context). Dead end.

ConnectConfiguration

As I mentioned, the alternative is to use ConnectConfiguration. Its into_ssl method takes a hostname (which can also be an ip address).
It also has methods to enable disable verification (and sni), advising against disabling it. Looks great!

What does it internally do to set the hostname?

    if #[cfg(any(ossl102, libressl261))] {
        fn setup_verify(ctx: &mut SslContextBuilder) {
            ctx.set_verify(SslVerifyMode::PEER);
        }

        fn setup_verify_hostname(ssl: &mut SslRef, domain: &str) -> Result<(), ErrorStack> {
            use crate::x509::verify::X509CheckFlags;

            let param = ssl.param_mut();
            param.set_hostflags(X509CheckFlags::NO_PARTIAL_WILDCARDS);
            match domain.parse() {
                Ok(ip) => param.set_ip(ip),
                Err(_) => param.set_host(domain),
            }
        }
    } else {
        fn setup_verify(ctx: &mut SslContextBuilder) {
            ctx.set_verify_callback(SslVerifyMode::PEER, verify::verify_callback);
        }

        fn setup_verify_hostname(ssl: &mut Ssl, domain: &str) -> Result<(), ErrorStack> {
            let domain = domain.to_string();
            let hostname_idx = verify::try_get_hostname_idx()?;
            ssl.set_ex_data(*hostname_idx, domain);
            Ok(())
        }

It looks like it depends on the system library, and either uses the param_mut approach, or set_ex_data which Sets the extra data at the specified index. (that looks like extremely low-level approach).
Good news is that our param_mut approach looks correct.

But in general it seems that ConnectConfiguration is a more friendly API, which makes me think that we should use it. Can we?
It can be created in exactly one way: using configure() method on SslConnector. What is SslConnector?!
Its description:

A type which wraps client-side streams in a TLS session.

OpenSSL’s default configuration is highly insecure. This connector manages the OpenSSL structures, configuring cipher suites, session options, hostname verification, and more.

OpenSSL’s built-in hostname verification is used when linking against OpenSSL 1.0.2 or 1.1.0, and a custom implementation is used when linking against OpenSSL 1.0.1.

Huh, so maybe we should be using that? Can we?
What is the flow of creating SslConnector?
You call SslConnector::builder to create a new builder (SslConnectorBuilder). It implements Deref<Target=SslContextBuilder>, so it looks like the difference between them comes down to defaults.
The good news is that our users can already use this, because SslConnector has into_context method returning SslContext which they can use with the driver.
The bad news is that reverse is not true - you can't create SslConnector from SslContext (which makes sense if connector is supposed to provide safer defaults).
That means we can't utilize its safer methods that guarantee hostname verification.

Conclusion and what is to be done

This PR

I think this PR is a step in the right direction: hostname verification is an important mechanism and we should use it.
The current approach to it assumes that nodes have their IP in SAN field. I'm not sure this assumption is 100% correct in all deployments,
and I'm not sure who to ask (@elcallio )? From a brief look, I think other drivers also use this assumption.
The only other option I see, since there is no hostname-per-node mechanism, is to hardcode one hostname that all nodes must have in their SAN.
Is that approach used in practice?

I think this is already possible with openssl (but I did not write test for it because I don't know if we need this mechanism), but not rustls.

Given that SslConnector seems to be a better API, I added a recommendation for users to use it (with into_context method) instead of SslContext.

Possible further improvements

We could consider introducing variant of TlsContext with SslConnector. Unfortunately, we can't make the current Openssl010 variant an enum itself because of backwards compatibility.

If it turns out that "hardcoded domain name" approach is used in practice, we need to

• Write tests for it
• Introduce another TlsContext variant for rustls that will also store hostname.

The above makes me think it may have been a mistake to implement TlsContex the way we did.
Maybe the body of its variants should be opaque structs for which we could provide builders? That way we could change the internals of each variant.
We could also consider replacing the enum with trait.

Digression: older tokio-openssl versions

Current tokio-openssl version is 0.6.x, and its behavior is described above.
Version 0.5 had this description:

Async TLS streams backed by OpenSSL

This library is an implementation of TLS streams using OpenSSL for negotiating the connection. Each TLS stream implements the Read and Write traits to interact and interoperate with the rest of the futures I/O ecosystem. Client connections initiated from this crate verify hostnames automatically and by default.

This crate primarily exports this ability through two extension traits, SslConnectorExt and SslAcceptorExt. These traits augment the functionality provided by the openssl crate, on which this crate is built. Configuration of TLS parameters is still primarily done through the openssl crate.

The interesting part is the one about hostname verification being enabled by default. Back then, this crate had connect function accepting ConnectConfiguration and hostname.
I'm not sure why they changed that, and I couldn't find this info. The only reason I can think of is that SslContext is more elastic / universal.
I opened an issue with a question about recommended way to enable hostname verification: tokio-rs/tokio-openssl#52

Fixes #1116

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce.
• I have adjusted the documentation in ./docs/source/.
• I added appropriate Fixes: annotations to PR description.

[github-actions]

cargo semver-checks found no API-breaking changes in this PR.
Checked commit: 05cdbb5

[Copilot]

Pull request overview

This PR adds hostname verification for OpenSSL TLS connections, fixing a security gap where OpenSSL connections were not verifying that the server certificate's subject alternative name (SAN) matched the node's IP address (while rustls already performed this verification).

Key changes:

• Implemented hostname verification for OpenSSL by calling ssl.param_mut().set_ip(node_address) before establishing the TLS connection
• Added ssl.set_connect_state() call during SSL object initialization to properly configure the connection for client mode
• Updated test to verify that connections with mismatched certificate SANs are properly rejected

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
scylla/src/network/tls.rs	Added set_connect_state() call to configure OpenSSL in client mode during SSL object creation
scylla/src/network/connection.rs	Implemented hostname verification by setting the node IP address in SSL verification parameters before establishing connection
scylla/tests/integration/ccm/tls.rs	Updated test to verify hostname verification now properly rejects connections when certificate SAN doesn't match node IP
docs/source/connecting/tls.md	Added documentation section explaining hostname verification approach for both OpenSSL and rustls implementations

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Lorak-mmk]

I confirmed with the cloud (@charconstpointer ) that in Cloud certs have node IP in SAN, so it looks like our approach is correct.