Log errors from the server channel. (#791)

Motivation:

When the child channel initializer fails, or if any part of child
channel setup fails, the error that results is not thrown into that
child channel but is instead propagated into the server channel. As
nothing in grpc was listening for errors there, those errors got lost.
This can make some problems particularly tricky to debug, as they lead
to silent service failures.

Modifications:

- Added a channel handler to be put in all server channels.

Results:

Server channel creation errors now get propagated into the error
delegate.

Sources/GRPC/Server.swift

@@ -143,6 +143,12 @@ public final class Server {
     // Maintain a strong reference to ensure it lives as long as the server.
     self.errorDelegate = errorDelegate
 
+    // If we have an error delegate, add a server channel error handler as well. We don't need to wait for the handler to
+    // be added.
+    if let errorDelegate = errorDelegate {
+      _ = channel.pipeline.addHandler(ServerChannelErrorHandler(errorDelegate: errorDelegate))
+    }
+
     // nil out errorDelegate to avoid retain cycles.
     onClose.whenComplete { _ in
       self.errorDelegate = nil

Sources/GRPC/ServerChannelErrorHandler.swift

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2020, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import NIO
+
+/// A handler that passes errors thrown into the server channel to the server error delegate.
+///
+/// A NIO server bootstrap produces two kinds of channels. The first and most common is the "child" channel:
+/// each of these corresponds to one connection, and has the connection state stored on it. The other kind is
+/// the "server" channel. Each bootstrap produces only one of these, and it is the channel that owns the listening
+/// socket.
+///
+/// This channel handler is inserted into the server channel, and is responsible for passing any errors in that pipeline
+/// to the server error delegate. If there is no error delegate, this handler is not inserted into the pipeline.
+final class ServerChannelErrorHandler {
+  private let errorDelegate: ServerErrorDelegate
+
+  init(errorDelegate: ServerErrorDelegate) {
+    self.errorDelegate = errorDelegate
+  }
+}
+
+extension ServerChannelErrorHandler: ChannelInboundHandler {
+  typealias InboundIn = Any
+  typealias InboundOut = Any
+
+  func errorCaught(context: ChannelHandlerContext, error: Error) {
+    // This handler does not treat errors as fatal to the listening socket, as it's possible they were transiently
+    // occurring in a single connection setup attempt.
+    errorDelegate.observeLibraryError(error)
+    context.fireErrorCaught(error)
+  }
+}

Sources/GRPCSampleData/GRPCSwiftCertificate.swift

@@ -46,6 +46,12 @@ public struct SampleCertificate {
     commonName: "localhost",
     // 22/07/2024 16:32:23
     notAfter: Date(timeIntervalSince1970: 1721662343.0))
+
+  public static let exampleServerWithExplicitCurve = SampleCertificate(
+    certificate: try! NIOSSLCertificate(bytes: .init(serverExplicitCurveCert.utf8), format :.pem),
+    commonName: "localhost",
+    // 13/05/2021 12:32:03
+    notAfter: Date(timeIntervalSince1970: 1620909123.0))
 }
 
 extension SampleCertificate {
@@ -62,6 +68,7 @@ public struct SamplePrivateKey {
   public static let server = try! NIOSSLPrivateKey(bytes: .init(serverKey.utf8), format: .pem)
   public static let exampleServer = try! NIOSSLPrivateKey(bytes: .init(exampleServerKey.utf8), format: .pem)
   public static let client = try! NIOSSLPrivateKey(bytes: .init(clientKey.utf8), format: .pem)
+  public static let exampleServerWithExplicitCurve = try! NIOSSLPrivateKey(bytes: .init(serverExplicitCurveKey.utf8), format: .pem)
 }
 
 // MARK: - Certificates and private keys
@@ -234,3 +241,33 @@ private let clientKey = """
     eEv8+Pr/GzzyAHdlESmPYdKjasD734+DL+c0imj7lmlt4d8kQs/oaQ==
     -----END RSA PRIVATE KEY-----
     """
+
+private let serverExplicitCurveCert = """
+    -----BEGIN CERTIFICATE-----
+    MIICEDCCAbYCCQDOr0V8CUAs8TAKBggqhkjOPQQDAjAWMRQwEgYDVQQDDAtleGFt
+    cGxlLmNvbTAeFw0yMDA1MTMxMjMyMDNaFw0yMTA1MTMxMjMyMDNaMBYxFDASBgNV
+    BAMMC2V4YW1wbGUuY29tMIIBSzCCAQMGByqGSM49AgEwgfcCAQEwLAYHKoZIzj0B
+    AQIhAP////8AAAABAAAAAAAAAAAAAAAA////////////////MFsEIP////8AAAAB
+    AAAAAAAAAAAAAAAA///////////////8BCBaxjXYqjqT57PrvVV2mIa8ZR0GsMxT
+    sPY7zjw+J9JgSwMVAMSdNgiG5wSTamZ44ROdJreBn36QBEEEaxfR8uEsQkf4vObl
+    Y6RA8ncDfYEt6zOg9KE5RdiYwpZP40Li/hp/m47n60p8D54WK84zV2sxXs7LtkBo
+    N79R9QIhAP////8AAAAA//////////+85vqtpxeehPO5ysL8YyVRAgEBA0IABChr
+    XwTLM3T1C0aA+8pJMVJOyVDP0Scd38OdqBISYvHLaNPRuIaMFA2KTE25pMqsqNe9
+    YNfgimABp6HUG7xKTMwwCgYIKoZIzj0EAwIDSAAwRQIhAM6ihMqgQ3Rr/w7oBhG6
+    uuA2+wn2KhZgSqgqTTtyo/ImAiBLrG/b76/7eaZ4t6xWHtKWH4y2e1zrxLDDpcjD
+    0zglag==
+    -----END CERTIFICATE-----
+    """
+
+private let serverExplicitCurveKey = """
+    -----BEGIN EC PRIVATE KEY-----
+    MIIBaAIBAQQgZeJYnJVaOdltFsUs6KatYy9XFmX6ujfUSkOR69RoyRWggfowgfcC
+    AQEwLAYHKoZIzj0BAQIhAP////8AAAABAAAAAAAAAAAAAAAA////////////////
+    MFsEIP////8AAAABAAAAAAAAAAAAAAAA///////////////8BCBaxjXYqjqT57Pr
+    vVV2mIa8ZR0GsMxTsPY7zjw+J9JgSwMVAMSdNgiG5wSTamZ44ROdJreBn36QBEEE
+    axfR8uEsQkf4vOblY6RA8ncDfYEt6zOg9KE5RdiYwpZP40Li/hp/m47n60p8D54W
+    K84zV2sxXs7LtkBoN79R9QIhAP////8AAAAA//////////+85vqtpxeehPO5ysL8
+    YyVRAgEBoUQDQgAEKGtfBMszdPULRoD7ykkxUk7JUM/RJx3fw52oEhJi8cto09G4
+    howUDYpMTbmkyqyo171g1+CKYAGnodQbvEpMzA==
+    -----END EC PRIVATE KEY-----
+    """

Tests/GRPCTests/ServerTLSErrorTests.swift

@@ -0,0 +1,117 @@
+/*
+ * Copyright 2020, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+@testable import GRPC
+import GRPCSampleData
+import EchoImplementation
+import Logging
+import NIO
+import NIOSSL
+import XCTest
+
+class ServerErrorRecordingDelegate: ServerErrorDelegate {
+  var errors: [Error] = []
+  var expectation: XCTestExpectation
+
+  init(expectation: XCTestExpectation) {
+    self.expectation = expectation
+  }
+
+  func observeLibraryError(_ error: Error) {
+    self.errors.append(error)
+    self.expectation.fulfill()
+  }
+}
+
+class ServerTLSErrorTests: GRPCTestCase {
+  let defaultClientTLSConfiguration = ClientConnection.Configuration.TLS(
+    certificateChain: [.certificate(SampleCertificate.client.certificate)],
+    privateKey: .privateKey(SamplePrivateKey.client),
+    trustRoots: .certificates([SampleCertificate.ca.certificate]),
+    hostnameOverride: SampleCertificate.server.commonName)
+
+  var defaultTestTimeout: TimeInterval = 1.0
+
+  var clientEventLoopGroup: EventLoopGroup!
+  var serverEventLoopGroup: EventLoopGroup!
+
+  func makeClientConfiguration(
+    tls: ClientConnection.Configuration.TLS,
+    port: Int
+  ) -> ClientConnection.Configuration {
+    return .init(
+      target: .hostAndPort("localhost", port),
+      eventLoopGroup: self.clientEventLoopGroup,
+      tls: tls,
+      // No need to retry connecting.
+      connectionBackoff: nil
+    )
+  }
+
+  func makeClientConnectionExpectation() -> XCTestExpectation {
+    return self.expectation(description: "EventLoopFuture<ClientConnection> resolved")
+  }
+
+  override func setUp() {
+    self.serverEventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+    self.clientEventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+  }
+
+  override func tearDown() {
+    XCTAssertNoThrow(try self.clientEventLoopGroup.syncShutdownGracefully())
+    self.clientEventLoopGroup = nil
+
+    XCTAssertNoThrow(try self.serverEventLoopGroup.syncShutdownGracefully())
+    self.serverEventLoopGroup = nil
+  }
+
+  func testErrorIsLoggedWhenSSLContextErrors() throws {
+    let clientShutdownExpectation = self.expectation(description: "client shutdown")
+    let errorExpectation = self.expectation(description: "error")
+    let errorDelegate = ServerErrorRecordingDelegate(expectation: errorExpectation)
+
+    let server = try! Server.secure(
+      group: self.serverEventLoopGroup,
+      certificateChain: [SampleCertificate.exampleServerWithExplicitCurve.certificate],
+      privateKey: SamplePrivateKey.exampleServerWithExplicitCurve
+    ).withServiceProviders([EchoProvider()])
+      .withErrorDelegate(errorDelegate)
+      .bind(host: "localhost", port: 0)
+      .wait()
+    defer {
+      XCTAssertNoThrow(try server.close().wait())
+    }
+
+    let port = server.channel.localAddress!.port!
+
+    var tls = self.defaultClientTLSConfiguration
+    tls.trustRoots = .certificates([SampleCertificate.exampleServerWithExplicitCurve.certificate])
+    var configuration = self.makeClientConfiguration(tls: tls, port: port)
+
+    let stateChangeDelegate = ConnectivityStateCollectionDelegate(shutdown: clientShutdownExpectation)
+    configuration.connectivityStateDelegate = stateChangeDelegate
+
+    _ = ClientConnection(configuration: configuration)
+
+    self.wait(for: [clientShutdownExpectation, errorExpectation], timeout: self.defaultTestTimeout)
+
+    if let nioSSLError = errorDelegate.errors.first as? NIOSSLError,
+      case .failedToLoadCertificate = nioSSLError {
+      // Expected case.
+    } else {
+      XCTFail("Expected NIOSSLError.handshakeFailed(BoringSSL.sslError)")
+    }
+  }
+}

Tests/GRPCTests/XCTestManifests.swift

@@ -606,6 +606,15 @@ extension ServerErrorTransformingTests {
     ]
 }
 
+extension ServerTLSErrorTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__ServerTLSErrorTests = [
+        ("testErrorIsLoggedWhenSSLContextErrors", testErrorIsLoggedWhenSSLContextErrors),
+    ]
+}
+
 extension ServerThrowingTests {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -716,6 +725,7 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(ReadStateTests.__allTests__ReadStateTests),
         testCase(ServerDelayedThrowingTests.__allTests__ServerDelayedThrowingTests),
         testCase(ServerErrorTransformingTests.__allTests__ServerErrorTransformingTests),
+        testCase(ServerTLSErrorTests.__allTests__ServerTLSErrorTests),
         testCase(ServerThrowingTests.__allTests__ServerThrowingTests),
         testCase(ServerWebTests.__allTests__ServerWebTests),
         testCase(StopwatchTests.__allTests__StopwatchTests),

scripts/makecert

@@ -31,3 +31,7 @@ openssl x509 -req -days 365 -in client.csr -CA ca.crt -CAkey ca.key -set_serial
 # http://netty.io/wiki/sslcontextbuilder-and-private-key.html
 openssl pkcs8 -topk8 -nocrypt -in client.key -out client.pem
 openssl pkcs8 -topk8 -nocrypt -in server.key -out server.pem
+
+# Server cert with explicit EC parameters (not supported)
+openssl ecparam -name prime256v1 -genkey -param_enc explicit -out server-explicit.key
+openssl req -new -x509 -days 365 -key server-explicit.key -out server-explicit.crt -subj "/CN=${CN_SERVER}"