tls: verify server_name on OpenSSL, opt-in

include/seastar/net/tls.hh

@@ -445,13 +445,23 @@ namespace tls {
     public:
         /// \brief whether to wait for EOF from server on session termination
         deprecated_wait_for_eof_on_shutdown wait_for_eof_on_shutdown;
-        /// \brief server name to be used for the SNI TLS extension
+        /// \brief the server being connected to: a DNS name, or an IP literal
+        /// (brackets allowed) which is not sent as SNI (RFC 6066 §3). Also the
+        /// name verification checks, see verify_server_name.
         sstring server_name = {};
 
         /// \brief whether server certificate should be verified. May be set to false
         /// in test environments.
         bool verify_certificate = true;
 
+        /// \brief whether the peer certificate must be issued for server_name
+        /// (a DNS SAN for names, an IP SAN for literals) as well as chaining to
+        /// a trusted CA. GnuTLS always checks this when server_name is set;
+        /// OpenSSL only when this is true. Off by default, so clients
+        /// connecting by address to certificates without an IP SAN keep
+        /// working until they opt in.
+        bool verify_server_name = false;
+
         /// \brief Optional session resume data. Must be retrieved via
         /// get_session_resume_data below.
         session_data session_resume_data;

src/net/tls_gnutls.cc

@@ -749,8 +749,9 @@ class session : public enable_shared_from_this<session>, public tls::session_imp
         }
 
         unsigned int status;
-        auto res = gnutls_certificate_verify_peers3(*this, _type != type::CLIENT || _options.server_name.empty()
-                        ? nullptr : _options.server_name.c_str(), &status);
+        auto name = verification_name(_options.server_name);
+        auto res = gnutls_certificate_verify_peers3(*this, _type != type::CLIENT || name.empty()
+                        ? nullptr : name.c_str(), &status);
         if (res == GNUTLS_E_NO_CERTIFICATE_FOUND && _type != type::CLIENT && _creds->get_client_auth() != client_auth::REQUIRE) {
             return;
         }

src/net/tls_openssl.cc

@@ -845,6 +845,9 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
                 SSL_set_tlsext_host_name(
                   _ssl.get(), _options.server_name.c_str());
             }
+            if (_options.verify_server_name && !_options.server_name.empty()) {
+                expect_peer_name(_options.server_name);
+            }
             SSL_set_connect_state(_ssl.get());
         }
 
@@ -867,6 +870,23 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
             tls_options options = {})
             : openssl_session(t, std::move(creds), net::get_impl::get(std::move(sock)), options) {}
 
+    // OpenSSL then reports a mismatch through SSL_get_verify_result, so
+    // verify() sees it like any other chain error.
+    void expect_peer_name(std::string_view name) {
+        auto host = verification_name(name);
+        auto* param = SSL_get0_param(_ssl.get());
+        if (is_ip_literal(host)) {
+            if (1 != X509_VERIFY_PARAM_set1_ip_asc(param, host.c_str())) {
+                throw make_openssl_error("Failed to set the expected peer address");
+            }
+            return;
+        }
+        X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+        if (1 != SSL_set1_host(_ssl.get(), host.c_str())) {
+            throw make_openssl_error("Failed to set the expected peer name");
+        }
+    }
+
     ~openssl_session() {
         SEASTAR_ASSERT(_output_pending.available());
     }

tests/unit/CMakeLists.txt

@@ -632,6 +632,9 @@ function(seastar_add_certgen name)
   if (NOT CERT_ALT_IP_1)
     set(CERT_ALT_IP_1 127.0.0.1)
   endif()
+  if (NOT CERT_ALT_IP_2)
+    set(CERT_ALT_IP_2 ::1)
+  endif()
   if (NOT CERT_ALT_DNS)
     set(CERT_ALT_DNS ${CERT_COMMON})
   endif()

tests/unit/cert.cfg.in

@@ -23,4 +23,4 @@ basicConstraints = CA:FALSE
 keyUsage = nonRepudiation, digitalSignature, keyEncipherment
 
 [req_ext]
-subjectAltName=email:@CERT_ALT_EMAIL_1@,email:@CERT_ALT_EMAIL_2@,IP:@CERT_ALT_IP_1@,DNS:@CERT_ALT_DNS@
+subjectAltName=email:@CERT_ALT_EMAIL_1@,email:@CERT_ALT_EMAIL_2@,IP:@CERT_ALT_IP_1@,IP:@CERT_ALT_IP_2@,DNS:@CERT_ALT_DNS@

tests/unit/tls_test.cc

@@ -49,6 +49,7 @@
 
 #include <boost/dll.hpp>
 
+#include "ipv6_support.hh"
 #include "loopback_socket.hh"
 #include "tmpdir.hh"
 
@@ -745,9 +746,10 @@ static future<> echo_client_session(::shared_ptr<sstring> msg,
                                     socket_address addr,
                                     const sstring& name,
                                     int loops,
-                                    bool do_read)
+                                    bool do_read,
+                                    bool verify_server_name = false)
 {
-    auto s = co_await tls::connect(certs, addr, tls::tls_options{.server_name = name});
+    auto s = co_await tls::connect(certs, addr, tls::tls_options{.server_name = name, .verify_server_name = verify_server_name});
     auto strms = ::make_lw_shared<streams>(std::move(s));
 
     auto echo = [strms, msg, loops]() -> future<> {
@@ -779,15 +781,17 @@ static future<> run_echo_test(sstring message,
                 sstring client_key = {},
                 bool do_read = true,
                 bool use_dh_params = true,
-                tls::dn_callback distinguished_name_callback = {}
+                tls::dn_callback distinguished_name_callback = {},
+                bool verify_server_name = false,
+                std::optional<socket_address> listen_addr = {}
 )
 {
     static const auto port = 4711;
 
     auto msg = ::make_shared<sstring>(std::move(message));
     auto certs = ::make_shared<tls::certificate_credentials>();
     auto server = ::make_shared<seastar::sharded<echoserver>>();
-    auto addr = ::make_ipv4_address( {0x7f000001, port});
+    auto addr = listen_addr.value_or(::make_ipv4_address( {0x7f000001, port}));
 
     SEASTAR_ASSERT(do_read || loops == 1);
 
@@ -806,7 +810,7 @@ static future<> run_echo_test(sstring message,
             server_trust = trust;
         }
         co_await server->invoke_on_all(&echoserver::listen, addr, crt, key, ca, server_trust);
-        co_await echo_client_session(msg, certs, addr, name, loops, do_read);
+        co_await echo_client_session(msg, certs, addr, name, loops, do_read, verify_server_name);
     }().finally([server] {
         return server->stop();
     });
@@ -878,6 +882,57 @@ SEASTAR_TEST_CASE(test_x509_client_server_cert_validation_fail_name) {
     });
 }
 
+// run_echo_test against the local server with verify_server_name set;
+// test.crt is issued for test.scylladb.org with IP SANs 127.0.0.1 and ::1.
+static future<> run_verified_echo_test(sstring name, std::optional<socket_address> addr = {}) {
+    return run_echo_test(message, 1, certfile("catest.pem"), std::move(name), certfile("test.crt"), certfile("test.key"),
+                         tls::client_auth::NONE, {}, {}, true, true, {}, true, std::move(addr));
+}
+
+static future<> expect_verification_error(future<> f) {
+    return f.then([] {
+        BOOST_FAIL("Should have gotten validation error");
+    }).handle_exception([](auto ep) {
+        try {
+            std::rethrow_exception(ep);
+        } catch (tls::verification_error&) {
+            // ok.
+        } catch (...) {
+            BOOST_FAIL(fmt::format("Unexpected exception: {}", std::current_exception()));
+        }
+    });
+}
+
+SEASTAR_TEST_CASE(test_verify_server_name_dns_match) {
+    return run_verified_echo_test("test.scylladb.org");
+}
+
+SEASTAR_TEST_CASE(test_verify_server_name_dns_mismatch) {
+    // trusted CA, wrong name: the only thing standing between the client and
+    // this server is the name check
+    return expect_verification_error(run_verified_echo_test("nils.holgersson.gov"));
+}
+
+SEASTAR_TEST_CASE(test_verify_server_name_ip_san_ipv4) {
+    return run_verified_echo_test("127.0.0.1");
+}
+
+SEASTAR_TEST_CASE(test_verify_server_name_ip_san_mismatch) {
+    return expect_verification_error(run_verified_echo_test("127.0.0.2"));
+}
+
+SEASTAR_TEST_CASE(test_verify_server_name_ip_san_ipv6) {
+    if (!seastar::testing::ipv6_available_or_skip()) {
+        return make_ready_future<>();
+    }
+    auto addr = socket_address(ipv6_addr("::1", 4711));
+    return run_verified_echo_test("::1", addr).then([addr] {
+        return run_verified_echo_test("[::1]", addr);
+    }).then([addr] {
+        return expect_verification_error(run_verified_echo_test("::2", addr));
+    });
+}
+
 SEASTAR_TEST_CASE(test_large_message_x509_client_server) {
     // Make sure we load our own auth trust pem file, otherwise our certs
     // will not validate
@@ -1621,9 +1676,20 @@ SEASTAR_THREAD_TEST_CASE(test_alt_names) {
             BOOST_FAIL("Missing " + std::to_string(min_count) + " alt name attributes of type " + std::to_string(int(type)));
         };
 
-        ensure_alt_name(tls::subject_alt_name_type::ipaddress, 1);
+        ensure_alt_name(tls::subject_alt_name_type::ipaddress, 2);
         ensure_alt_name(tls::subject_alt_name_type::rfc822name, 2);
         ensure_alt_name(tls::subject_alt_name_type::dnsname, 1);
+
+        // and the IP SANs come back as addresses of the right family
+        std::vector<net::inet_address> ips;
+        for (auto& v : alt_names) {
+            if (v.type == tls::subject_alt_name_type::ipaddress) {
+                ips.push_back(std::get<net::inet_address>(v.value));
+            }
+        }
+        BOOST_REQUIRE_EQUAL(ips.size(), 2u);
+        BOOST_REQUIRE(std::ranges::find(ips, net::inet_address("127.0.0.1")) != ips.end());
+        BOOST_REQUIRE(std::ranges::find(ips, net::inet_address("::1")) != ips.end());
     }
 
 }