From 1eb5e9f68e84c0637fec2f9af8d23e6cf1f39e5e Mon Sep 17 00:00:00 2001
From: George Barnett <gbarnett@apple.com>
Date: Wed, 4 Nov 2020 15:50:15 +0000
Subject: [PATCH] Use the server hostname override as the :authority, if
 present

Motivation:

When using a secured connection to a server, the client may present a
server hostname as part of the TLS SNI extension. However, the
':authority' header sent by the client will still just be the host the
caller specified, rather than the server hostname.

Modifications:

- User the TLS server hostname override, if present, in prefernce to the
  hostname when setting the authority.

Result:

- Resolves #1029
---
 Sources/GRPC/ClientConnection.swift   |  2 +-
 Tests/GRPCTests/ClientTLSTests.swift  | 72 +++++++++++++++++++++++++++
 Tests/GRPCTests/XCTestManifests.swift |  1 +
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/Sources/GRPC/ClientConnection.swift b/Sources/GRPC/ClientConnection.swift
index e587f977d..432817867 100644
--- a/Sources/GRPC/ClientConnection.swift
+++ b/Sources/GRPC/ClientConnection.swift
@@ -116,7 +116,7 @@ public class ClientConnection {
   public init(configuration: Configuration) {
     self.configuration = configuration
     self.scheme = configuration.tls == nil ? "http" : "https"
-    self.authority = configuration.target.host
+    self.authority = configuration.tls?.hostnameOverride ?? configuration.target.host
     self.connectionManager = ConnectionManager(
       configuration: configuration,
       logger: configuration.backgroundActivityLogger
diff --git a/Tests/GRPCTests/ClientTLSTests.swift b/Tests/GRPCTests/ClientTLSTests.swift
index b97b9137e..c853149e1 100644
--- a/Tests/GRPCTests/ClientTLSTests.swift
+++ b/Tests/GRPCTests/ClientTLSTests.swift
@@ -136,4 +136,76 @@ class ClientTLSHostnameOverrideTests: GRPCTestCase {
 
     try self.doTestUnary()
   }
+
+  func testAuthorityUsesTLSHostnameOverride() throws {
+    // This test validates that when suppled with a server hostname override, the client uses it
+    // as the ":authority" pseudo-header.
+
+    self.server = try Server.secure(
+      group: self.eventLoopGroup,
+      certificateChain: [SampleCertificate.exampleServer.certificate],
+      privateKey: SamplePrivateKey.exampleServer
+    )
+    .withTLS(trustRoots: .certificates([SampleCertificate.ca.certificate]))
+    .withServiceProviders([AuthorityCheckingEcho()])
+    .withLogger(self.serverLogger)
+    .bind(host: "localhost", port: 0)
+    .wait()
+
+    guard let port = self.server.channel.localAddress?.port else {
+      XCTFail("could not get server port")
+      return
+    }
+
+    self.connection = ClientConnection.secure(group: self.eventLoopGroup)
+      .withTLS(trustRoots: .certificates([SampleCertificate.ca.certificate]))
+      .withTLS(serverHostnameOverride: "example.com")
+      .withBackgroundActivityLogger(self.clientLogger)
+      .connect(host: "localhost", port: port)
+
+    try self.doTestUnary()
+  }
+}
+
+private class AuthorityCheckingEcho: Echo_EchoProvider {
+  func get(
+    request: Echo_EchoRequest,
+    context: StatusOnlyCallContext
+  ) -> EventLoopFuture<Echo_EchoResponse> {
+    // Since we currently go via the HTTP 2-to-1 handler, ':authority' gets normalized to 'host'.
+    // TODO: Use ':authority' when we fully switch to HTTP/2
+    guard let authority = context.headers.first(name: "host") else {
+      let status = GRPCStatus(
+        code: .failedPrecondition,
+        message: "Missing ':authority' pseudo header"
+      )
+      return context.eventLoop.makeFailedFuture(status)
+    }
+
+    XCTAssertEqual(authority, SampleCertificate.exampleServer.commonName)
+    XCTAssertNotEqual(authority, "localhost")
+
+    return context.eventLoop.makeSucceededFuture(.with {
+      $0.text = "Swift echo get: \(request.text)"
+    })
+  }
+
+  func expand(
+    request: Echo_EchoRequest,
+    context: StreamingResponseCallContext<Echo_EchoResponse>
+  ) -> EventLoopFuture<GRPCStatus> {
+    preconditionFailure("Not implemented")
+  }
+
+  func collect(
+    context: UnaryResponseCallContext<Echo_EchoResponse>
+  ) -> EventLoopFuture<(StreamEvent<Echo_EchoRequest>) -> Void> {
+    preconditionFailure("Not implemented")
+  }
+
+  func update(
+    context: StreamingResponseCallContext<Echo_EchoResponse>
+  ) -> EventLoopFuture<(StreamEvent<Echo_EchoRequest>) -> Void> {
+    preconditionFailure("Not implemented")
+  }
 }
diff --git a/Tests/GRPCTests/XCTestManifests.swift b/Tests/GRPCTests/XCTestManifests.swift
index c55a13402..d60e63f51 100644
--- a/Tests/GRPCTests/XCTestManifests.swift
+++ b/Tests/GRPCTests/XCTestManifests.swift
@@ -96,6 +96,7 @@ extension ClientTLSHostnameOverrideTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__ClientTLSHostnameOverrideTests = [
+        ("testAuthorityUsesTLSHostnameOverride", testAuthorityUsesTLSHostnameOverride),
         ("testTLSWithHostnameOverride", testTLSWithHostnameOverride),
         ("testTLSWithNoCertificateVerification", testTLSWithNoCertificateVerification),
         ("testTLSWithoutHostnameOverride", testTLSWithoutHostnameOverride),