Ensure port isn't included in SNI hostname

Sources/GRPCNIOTransportCore/Client/Connection/Connection.swift

@@ -89,6 +89,9 @@ package final class Connection: Sendable {
   /// being connected to.
   private let authority: String?
 
+  /// The name of the server used for the TLS SNI extension, if applicable.
+  private let sniServerHostname: String?
+
   /// The default compression algorithm used for requests.
   private let defaultCompression: CompressionAlgorithm
 
@@ -111,6 +114,20 @@ package final class Connection: Sendable {
     self.event.stream
   }
 
+  private static func sanitizeAuthorityForSNI(_ authority: String) -> String {
+    // Strip off a trailing ":{PORT}". Look for the last non-digit byte, if it's
+    // a colon then keep everything up to that index.
+    let index = authority.utf8.lastIndex { byte in
+      return byte < UInt8(ascii: "0") || byte > UInt8(ascii: "9")
+    }
+
+    if let index = index, authority.utf8[index] == UInt8(ascii: ":") {
+      return String(authority.utf8[..<index])!
+    } else {
+      return authority
+    }
+  }
+
   package init(
     address: SocketAddress,
     authority: String?,
@@ -120,6 +137,7 @@ package final class Connection: Sendable {
   ) {
     self.address = address
     self.authority = authority
+    self.sniServerHostname = authority.map { Self.sanitizeAuthorityForSNI($0) }
     self.defaultCompression = defaultCompression
     self.enabledCompression = enabledCompression
     self.http2Connector = http2Connector
@@ -140,7 +158,7 @@ package final class Connection: Sendable {
           // The authority here is used for the SNI hostname in the TLS handshake (if applicable)
           // where a raw IP address isn't permitted, so fallback to 'address.sniHostname' rather
           // than 'address.authority'.
-          authority: self.authority ?? self.address.sniHostname
+          sniServerHostname: self.sniServerHostname ?? self.address.sniHostname
         )
       } catch let error as RPCError {
         throw error

Sources/GRPCNIOTransportCore/Client/Connection/ConnectionFactory.swift

@@ -23,10 +23,10 @@ package protocol HTTP2Connector: Sendable {
   ///
   /// - Parameters:
   ///   - address: The address to connect to.
-  ///   - authority: The authority as used for the TLS SNI extension (if applicable).
+  ///   - sniServerHostname: The name of the server used for the TLS SNI extension (if applicable).
   func establishConnection(
     to address: SocketAddress,
-    authority: String?
+    sniServerHostname: String?
   ) async throws -> HTTP2Connection
 }
 

Sources/GRPCNIOTransportHTTP2Posix/HTTP2ClientTransport+Posix.swift

@@ -165,7 +165,7 @@ extension HTTP2ClientTransport.Posix {
 
     func establishConnection(
       to address: GRPCNIOTransportCore.SocketAddress,
-      authority: String?
+      sniServerHostname: String?
     ) async throws -> HTTP2Connection {
       let (channel, multiplexer) = try await ClientBootstrap(
         group: self.eventLoopGroup
@@ -175,7 +175,7 @@ extension HTTP2ClientTransport.Posix {
             try channel.pipeline.syncOperations.addHandler(
               NIOSSLClientHandler(
                 context: sslContext,
-                serverHostname: authority
+                serverHostname: sniServerHostname
               )
             )
           }

Sources/GRPCNIOTransportHTTP2TransportServices/HTTP2ClientTransport+TransportServices.swift

@@ -149,7 +149,7 @@ extension HTTP2ClientTransport.TransportServices {
 
     func establishConnection(
       to address: GRPCNIOTransportCore.SocketAddress,
-      authority: String?
+      sniServerHostname: String?
     ) async throws -> HTTP2Connection {
       let bootstrap: NIOTSConnectionBootstrap
       let isPlainText: Bool
@@ -162,7 +162,7 @@ extension HTTP2ClientTransport.TransportServices {
       case .tls(let tlsConfig):
         isPlainText = false
         do {
-          let options = try NWProtocolTLS.Options(tlsConfig, authority: authority)
+          let options = try NWProtocolTLS.Options(tlsConfig, authority: sniServerHostname)
           bootstrap = NIOTSConnectionBootstrap(group: self.eventLoopGroup)
             .channelOption(NIOTSChannelOptions.waitForActivity, value: false)
             .tlsOptions(options)

Tests/GRPCNIOTransportCoreTests/Client/Connection/ConnectionTests.swift

@@ -21,6 +21,7 @@ import NIOCore
 import NIOHPACK
 import NIOHTTP2
 import NIOPosix
+import Synchronization
 import XCTest
 
 final class ConnectionTests: XCTestCase {
@@ -199,6 +200,44 @@ final class ConnectionTests: XCTestCase {
       XCTAssertEqual(error.code, .unavailable)
     }
   }
+
+  private func testAuthorityIsSanitized(authority: String, expected: String) async throws {
+    let recorder = SNIRecordingConnector()
+    let connection = Connection(
+      address: .ipv4(host: "ignored", port: 0),
+      authority: authority,
+      http2Connector: recorder,
+      defaultCompression: .none,
+      enabledCompression: .none
+    )
+
+    // The connect attempt will fail, but as a side effect the SNI hostname
+    // will be recorded.
+    await connection.run()
+    XCTAssertEqual(recorder.sniHostnames, [expected])
+  }
+
+  func testAuthorityIsSanitized() async throws {
+    try await self.testAuthorityIsSanitized(
+      authority: "foo.example.com",
+      expected: "foo.example.com"
+    )
+
+    try await self.testAuthorityIsSanitized(
+      authority: "foo.example.com:31415",
+      expected: "foo.example.com"
+    )
+
+    try await self.testAuthorityIsSanitized(
+      authority: "foo.example-31415",
+      expected: "foo.example-31415"
+    )
+
+    try await self.testAuthorityIsSanitized(
+      authority: "foo.example.com:abc123",
+      expected: "foo.example.com:abc123"
+    )
+  }
 }
 
 extension ClientBootstrap {
@@ -252,3 +291,25 @@ extension Metadata {
     self = metadata
   }
 }
+
+final class SNIRecordingConnector: HTTP2Connector {
+  private let _sniHostnames: Mutex<[String?]>
+
+  var sniHostnames: [String?] {
+    self._sniHostnames.withLock { $0 }
+  }
+
+  init() {
+    self._sniHostnames = Mutex([])
+  }
+
+  func establishConnection(
+    to address: GRPCNIOTransportCore.SocketAddress,
+    sniServerHostname: String?
+  ) async throws -> GRPCNIOTransportCore.HTTP2Connection {
+    self._sniHostnames.withLock {
+      $0.append(sniServerHostname)
+    }
+    throw RPCError(code: .unavailable, message: "Test is expected to throw.")
+  }
+}

Tests/GRPCNIOTransportCoreTests/Client/Connection/Utilities/HTTP2Connectors.swift

@@ -62,7 +62,7 @@ struct ThrowingConnector: HTTP2Connector {
 
   func establishConnection(
     to address: GRPCNIOTransportCore.SocketAddress,
-    authority: String?
+    sniServerHostname: String?
   ) async throws -> HTTP2Connection {
     throw self.error
   }
@@ -71,7 +71,7 @@ struct ThrowingConnector: HTTP2Connector {
 struct NeverConnector: HTTP2Connector {
   func establishConnection(
     to address: GRPCNIOTransportCore.SocketAddress,
-    authority: String?
+    sniServerHostname: String?
   ) async throws -> HTTP2Connection {
     fatalError("\(#function) called unexpectedly")
   }
@@ -103,7 +103,7 @@ struct NIOPosixConnector: HTTP2Connector {
 
   func establishConnection(
     to address: GRPCNIOTransportCore.SocketAddress,
-    authority: String?
+    sniServerHostname: String?
   ) async throws -> HTTP2Connection {
     return try await ClientBootstrap(group: self.eventLoopGroup).connect(to: address) { channel in
       channel.eventLoop.makeCompletedFuture {