Allow custom verification callback to be configured for servers (#1595)

Motivation:

NIOSSL allows users to override its certificate verification logic by
setting a verification callback. gRPC allows clients to configure this
but not servers.

Modifications:

- Add extra API to `GRPCTLSConfiguration` to allow custom verification
  callbacks to be set for servers.

Result:

Users can override the certificate verification logic on the server.

Author: glbrntt

Sources/GRPC/GRPCTLSConfiguration.swift

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #if canImport(NIOSSL)
+import NIOCore
 import NIOSSL
 #endif
 
@@ -310,6 +311,38 @@ extension GRPCTLSConfiguration {
     trustRoots: NIOSSLTrustRoots = .default,
     certificateVerification: CertificateVerification = .none,
     requireALPN: Bool = true
+  ) -> GRPCTLSConfiguration {
+    return Self.makeServerConfigurationBackedByNIOSSL(
+      certificateChain: certificateChain,
+      privateKey: privateKey,
+      trustRoots: trustRoots,
+      certificateVerification: certificateVerification,
+      requireALPN: requireALPN,
+      customVerificationCallback: nil
+    )
+  }
+
+  /// TLS Configuration with suitable defaults for servers.
+  ///
+  /// This is a wrapper around `NIOSSL.TLSConfiguration` to restrict input to values which comply
+  /// with the gRPC protocol.
+  ///
+  /// - Parameter certificateChain: The certificate to offer during negotiation.
+  /// - Parameter privateKey: The private key associated with the leaf certificate.
+  /// - Parameter trustRoots: The trust roots to validate certificates, this defaults to using a
+  ///     root provided by the platform.
+  /// - Parameter certificateVerification: Whether to verify the remote certificate. Defaults to
+  ///     `.none`.
+  /// - Parameter requireALPN: Whether ALPN is required or not.
+  /// - Parameter customVerificationCallback: A callback to provide to override the certificate verification logic,
+  ///     defaults to `nil`.
+  public static func makeServerConfigurationBackedByNIOSSL(
+    certificateChain: [NIOSSLCertificateSource],
+    privateKey: NIOSSLPrivateKeySource,
+    trustRoots: NIOSSLTrustRoots = .default,
+    certificateVerification: CertificateVerification = .none,
+    requireALPN: Bool = true,
+    customVerificationCallback: NIOSSLCustomVerificationCallback? = nil
   ) -> GRPCTLSConfiguration {
     var configuration = TLSConfiguration.makeServerConfiguration(
       certificateChain: certificateChain,
@@ -323,7 +356,8 @@ extension GRPCTLSConfiguration {
 
     return GRPCTLSConfiguration.makeServerConfigurationBackedByNIOSSL(
       configuration: configuration,
-      requireALPN: requireALPN
+      requireALPN: requireALPN,
+      customVerificationCallback: customVerificationCallback
     )
   }
 
@@ -338,6 +372,28 @@ extension GRPCTLSConfiguration {
   public static func makeServerConfigurationBackedByNIOSSL(
     configuration: TLSConfiguration,
     requireALPN: Bool = true
+  ) -> GRPCTLSConfiguration {
+    return Self.makeServerConfigurationBackedByNIOSSL(
+      configuration: configuration,
+      requireALPN: requireALPN,
+      customVerificationCallback: nil
+    )
+  }
+
+  /// Creates a gRPC TLS Configuration suitable for servers using the given
+  /// `NIOSSL.TLSConfiguration`.
+  ///
+  /// - Note: If no ALPN tokens are set in `configuration.applicationProtocols` then "grpc-exp",
+  ///   "h2", and "http/1.1" will be used.
+  /// - Parameters:
+  ///   - configuration: The `NIOSSL.TLSConfiguration` to base this configuration on.
+  ///   - requiresALPN: Whether the server enforces ALPN. Defaults to `true`.
+  /// - Parameter customVerificationCallback: A callback to provide to override the certificate verification logic,
+  ///     defaults to `nil`.
+  public static func makeServerConfigurationBackedByNIOSSL(
+    configuration: TLSConfiguration,
+    requireALPN: Bool = true,
+    customVerificationCallback: NIOSSLCustomVerificationCallback? = nil
   ) -> GRPCTLSConfiguration {
     var configuration = configuration
 
@@ -348,7 +404,7 @@ extension GRPCTLSConfiguration {
 
     let nioConfiguration = NIOConfiguration(
       configuration: configuration,
-      customVerificationCallback: nil,
+      customVerificationCallback: customVerificationCallback,
       hostnameOverride: nil,
       requireALPN: requireALPN
     )

Sources/GRPC/Server.swift

@@ -140,7 +140,17 @@ public final class Server {
           let sync = channel.pipeline.syncOperations
           #if canImport(NIOSSL)
           if let sslContext = try sslContext?.get() {
-            try sync.addHandler(NIOSSLServerHandler(context: sslContext))
+            let sslHandler: NIOSSLServerHandler
+            if let verify = configuration.tlsConfiguration?.nioSSLCustomVerificationCallback {
+              sslHandler = NIOSSLServerHandler(
+                context: sslContext,
+                customVerificationCallback: verify
+              )
+            } else {
+              sslHandler = NIOSSLServerHandler(context: sslContext)
+            }
+
+            try sync.addHandler(sslHandler)
           }
           #endif // canImport(NIOSSL)
 

Tests/GRPCTests/AsyncAwaitSupport/AsyncClientTests.swift

@@ -33,12 +33,12 @@ final class AsyncClientCancellationTests: GRPCTestCase {
 
   override func tearDown() async throws {
     if self.pool != nil {
-      try self.pool.close().wait()
+      try await self.pool.close().get()
       self.pool = nil
     }
 
     if self.server != nil {
-      try self.server.close().wait()
+      try await self.server.close().get()
       self.server = nil
     }
 

Tests/GRPCTests/ConnectionPool/ConnectionPoolTests.swift

@@ -1245,7 +1245,6 @@ struct HTTP2FrameEncoder {
     buf.writeInteger(Int32(frame.streamID))
 
     // frame payload follows, which depends on the frame type itself
-    let payloadStart = buf.writerIndex
     let extraFrameData: IOData?
     let payloadSize: Int
 

Tests/GRPCTests/ServerTLSErrorTests.swift

@@ -124,6 +124,83 @@ class ServerTLSErrorTests: GRPCTestCase {
       XCTFail("Expected NIOSSLError.handshakeFailed(BoringSSL.sslError)")
     }
   }
+
+  func testServerCustomVerificationCallback() async throws {
+    let verificationCallbackInvoked = self.serverEventLoopGroup.next().makePromise(of: Void.self)
+    let configuration = GRPCTLSConfiguration.makeServerConfigurationBackedByNIOSSL(
+      certificateChain: [.certificate(SampleCertificate.server.certificate)],
+      privateKey: .privateKey(SamplePrivateKey.server),
+      certificateVerification: .fullVerification,
+      customVerificationCallback: { _, promise in
+        verificationCallbackInvoked.succeed()
+        promise.succeed(.failed)
+      }
+    )
+
+    let server = try await Server.usingTLS(with: configuration, on: self.serverEventLoopGroup)
+      .withServiceProviders([EchoProvider()])
+      .bind(host: "localhost", port: 0)
+      .get()
+    defer {
+      XCTAssertNoThrow(try server.close().wait())
+    }
+
+    let clientTLSConfiguration = GRPCTLSConfiguration.makeClientConfigurationBackedByNIOSSL(
+      certificateChain: [.certificate(SampleCertificate.client.certificate)],
+      privateKey: .privateKey(SamplePrivateKey.client),
+      trustRoots: .certificates([SampleCertificate.ca.certificate]),
+      certificateVerification: .noHostnameVerification,
+      hostnameOverride: SampleCertificate.server.commonName
+    )
+
+    let client = try GRPCChannelPool.with(
+      target: .hostAndPort("localhost", server.channel.localAddress!.port!),
+      transportSecurity: .tls(clientTLSConfiguration),
+      eventLoopGroup: self.clientEventLoopGroup
+    )
+    defer {
+      XCTAssertNoThrow(try client.close().wait())
+    }
+
+    let echo = Echo_EchoAsyncClient(channel: client)
+
+    enum TaskResult {
+      case rpcFailed
+      case rpcSucceeded
+      case verificationCallbackInvoked
+    }
+
+    await withTaskGroup(of: TaskResult.self, returning: Void.self) { group in
+      group.addTask {
+        // Call the service to start an RPC.
+        do {
+          _ = try await echo.get(.with { $0.text = "foo" })
+          return .rpcSucceeded
+        } catch {
+          return .rpcFailed
+        }
+      }
+
+      group.addTask {
+        // '!' is okay, the promise is only ever succeeded.
+        try! await verificationCallbackInvoked.futureResult.get()
+        return .verificationCallbackInvoked
+      }
+
+      while let next = await group.next() {
+        switch next {
+        case .verificationCallbackInvoked:
+          // Expected.
+          group.cancelAll()
+        case .rpcFailed:
+          // Expected, carry on.
+          continue
+        case .rpcSucceeded:
+          XCTFail("RPC succeeded but shouldn't have")
+        }
+      }
+    }
+  }
 }
 
 #endif // canImport(NIOSSL)