tls: do not send IP literals in the SNI extension

src/net/tls-impl.hh

@@ -34,6 +34,7 @@
 #include <seastar/core/seastar.hh>
 #include <seastar/net/tls.hh>
 #include <seastar/net/stack.hh>
+#include <seastar/net/inet_address.hh>
 
 namespace seastar {
 
@@ -117,6 +118,28 @@ void visit_blobs(Blobs& blobs, Visitor&& visitor) {
 
 namespace seastar::tls {
 
+// The host part of an authority: "[::1]" -> "::1", anything else unchanged.
+inline std::string_view unbracket(std::string_view name) {
+    if (name.size() > 2 && name.front() == '[' && name.back() == ']') {
+        name.remove_prefix(1);
+        name.remove_suffix(1);
+    }
+    return name;
+}
+
+// Callers connecting by address pass the address as server_name, and RFC 6066
+// §3 forbids an IP literal in the SNI extension (verification still uses it).
+inline bool is_ip_literal(std::string_view name) {
+    return net::inet_address::parse_numerical(sstring(unbracket(name))).has_value();
+}
+
+// The identity to check the certificate against: server_name without brackets
+// or a zone, neither of which appears in a certificate.
+inline sstring verification_name(std::string_view server_name) {
+    auto name = unbracket(server_name);
+    return sstring(name.substr(0, name.find('%')));
+}
+
 /// Abstract interface for DH parameters.
 ///
 /// GnuTLS has a full implementation; OpenSSL's is a stub.

src/net/tls_gnutls.cc

@@ -589,8 +589,11 @@ class session : public enable_shared_from_this<session>, public tls::session_imp
     }
 
     future<> do_handshake_invoke(int (*func)(gnutls_session_t)) {
-        if (_type == type::CLIENT && !_options.server_name.empty()) {
-            gnutls_server_name_set(*this, GNUTLS_NAME_DNS, _options.server_name.data(), _options.server_name.size());
+        if (_type == type::CLIENT && !_options.server_name.empty() && !is_ip_literal(_options.server_name)) {
+            auto res = gnutls_server_name_set(*this, GNUTLS_NAME_DNS, _options.server_name.data(), _options.server_name.size());
+            if (res < 0) {
+                return make_exception_future<>(std::system_error(res, local_error_category()));
+            }
         }
         try {
             auto res = func(*this);

src/net/tls_openssl.cc

@@ -841,7 +841,7 @@ class openssl_session : public enable_shared_from_this<openssl_session>, public
         if (_type == session_type::SERVER) {
             SSL_set_accept_state(_ssl.get());
         } else {
-            if (!_options.server_name.empty()) {
+            if (!_options.server_name.empty() && !is_ip_literal(_options.server_name)) {
                 SSL_set_tlsext_host_name(
                   _ssl.get(), _options.server_name.c_str());
             }

tests/unit/tls_test.cc

@@ -445,6 +445,79 @@ SEASTAR_TEST_CASE(test_failed_connect) {
     return connect_to_ssl_addr(b.build_certificate_credentials(), ipv4_addr()).handle_exception([](auto) {});
 }
 
+// The server_name extension of the ClientHello sent on `s`, read off the
+// wire by a server that never answers.
+static future<std::optional<sstring>> read_client_hello_sni(connected_socket s) {
+    auto in = s.input();
+    auto record = co_await in.read_exactly(5);
+    BOOST_REQUIRE_EQUAL(record.size(), 5u);
+    BOOST_REQUIRE_EQUAL(uint8_t(record[0]), 0x16u); // handshake record
+    auto body = co_await in.read_exactly((uint8_t(record[3]) << 8) | uint8_t(record[4]));
+    co_await in.close();
+
+    auto* p = reinterpret_cast<const uint8_t*>(body.get());
+    auto* end = p + body.size();
+    auto take = [&](size_t n) { BOOST_REQUIRE_GE(size_t(end - p), n); auto* r = p; p += n; return r; };
+    auto u8 = [&] { return size_t(*take(1)); };
+    auto u16 = [&] { auto* q = take(2); return size_t((q[0] << 8) | q[1]); };
+    BOOST_REQUIRE_EQUAL(u8(), 1u);   // ClientHello
+    take(3);                         // length
+    take(2);                         // legacy_version
+    take(32);                        // random
+    take(u8());                      // session id
+    take(u16());                     // cipher suites
+    take(u8());                      // compression methods
+    if (p == end) {
+        co_return std::nullopt;
+    }
+    auto* extensions_end = p + u16();
+    while (p < extensions_end) {
+        auto type = u16();
+        auto length = u16();
+        auto* ext = take(length);
+        if (type == 0) {             // server_name
+            BOOST_REQUIRE_GE(length, 5u);
+            BOOST_REQUIRE_EQUAL(ext[2], 0);  // host_name
+            size_t name_length = (ext[3] << 8) | ext[4];
+            co_return sstring(reinterpret_cast<const char*>(ext + 5), name_length);
+        }
+    }
+    co_return std::nullopt;
+}
+
+// RFC 6066 §3: an IP literal is not a valid SNI host_name, so a client that
+// connects by address must not send one (RFC 6066 §3). A DNS name still goes out.
+SEASTAR_THREAD_TEST_CASE(test_sni_omits_ip_literals) {
+    ::listen_options opts;
+    opts.reuse_address = true;
+    opts.set_fixed_cpu(this_shard_id());
+    auto server = seastar::listen(socket_address(ipv4_addr("127.0.0.1", 0)), opts);
+    auto addr = server.local_address();
+    auto creds = tls::credentials_builder().build_certificate_credentials();
+
+    struct { sstring server_name; std::optional<sstring> sni; } cases[] = {
+        {"localhost", "localhost"},
+        {"test.scylladb.org", "test.scylladb.org"},
+        {"127.0.0.1", std::nullopt},
+        {"::1", std::nullopt},
+        {"[::1]", std::nullopt},
+        {"2001:db8::1", std::nullopt},
+    };
+    for (auto& c : cases) {
+        auto accepted = server.accept();
+        auto client = tls::connect(creds, addr, tls::tls_options{.server_name = c.server_name}).get();
+        // the handshake, and with it the ClientHello, starts on first use
+        auto out = client.output();
+        auto sent = out.write("x").then([&out] { return out.flush(); });
+        auto sni = read_client_hello_sni(std::move(accepted.get().connection)).get();
+        BOOST_REQUIRE_MESSAGE(sni == c.sni, fmt::format("server_name {:?}: SNI sent = {:?}", c.server_name, sni.value_or("<none>")));
+        // the server hung up mid-handshake; that failure is not under test
+        std::move(sent).then_wrapped([](future<> f) { f.ignore_ready_future(); }).get();
+        out.close().then_wrapped([](future<> f) { f.ignore_ready_future(); }).get();
+    }
+    server.abort_accept();
+}
+
 SEASTAR_TEST_CASE(test_non_tls) {
     ::listen_options opts;
     opts.reuse_address = true;