Ignore SSL unclean shutdown errors (#650)

Motivation:

gRPC is self-terminated and is not prone to truncation attacks.

Modifications:

Ignore unclean shutdown errors in the delegating error handler.

Result:

Less log spam from peers which do not send close notify.

Sources/GRPC/DelegatingErrorHandler.swift

@@ -15,6 +15,7 @@
  */
 import Foundation
 import NIO
+import NIOSSL
 import Logging
 
 /// A channel handler which allows caught errors to be passed to a `ClientErrorDelegate`. This
@@ -31,12 +32,20 @@ public class DelegatingErrorHandler: ChannelInboundHandler {
   }
 
   public func errorCaught(context: ChannelHandlerContext, error: Error) {
+    // We can ignore unclean shutdown since gRPC is self-terminated and therefore not prone to
+    // truncation attacks.
+    //
+    // Without this we would unnecessarily log when we're communicating with peers which don't
+    // send `close_notify`.
+    if let sslError = error as? NIOSSLError, case .uncleanShutdown = sslError {
+      return
+    }
+
     if let delegate = self.delegate {
       let grpcError = (error as? GRPCError) ?? .unknown(error, origin: .client)
       delegate.didCatchError(grpcError.wrappedError, file: grpcError.file, line: grpcError.line)
     }
     self.logger.error("caught error in client channel", metadata: [MetadataKey.error: "\(error)"])
-    self.logger.info("closing client channel")
     context.close(promise: nil)
   }
 }

Tests/GRPCTests/DelegatingErrorHandlerTests.swift

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2019, 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 Foundation
+import GRPC
+import NIO
+import NIOSSL
+import XCTest
+
+class DelegatingErrorHandlerTests: GRPCTestCase {
+  class ErrorRecorder: ClientErrorDelegate {
+    var errors: [Error] = []
+
+    init() { }
+
+    func didCatchError(_ error: Error, file: StaticString, line: Int) {
+      self.errors.append(error)
+    }
+  }
+
+  func testUncleanShutdownIsIgnored() throws {
+    let delegate = ErrorRecorder()
+    let channel = EmbeddedChannel(handler: DelegatingErrorHandler(delegate: delegate))
+    channel.pipeline.fireErrorCaught(NIOSSLError.uncleanShutdown)
+    channel.pipeline.fireErrorCaught(NIOSSLError.writeDuringTLSShutdown)
+
+    XCTAssertEqual(delegate.errors.count, 1)
+    XCTAssertEqual(delegate.errors[0] as? NIOSSLError, .writeDuringTLSShutdown)
+  }
+}

Tests/GRPCTests/XCTestManifests.swift

@@ -120,6 +120,15 @@ extension ConnectivityStateMonitorTests {
     ]
 }
 
+extension DelegatingErrorHandlerTests {
+    // DO NOT MODIFY: This is autogenerated, use:
+    //   `swift test --generate-linuxmain`
+    // to regenerate.
+    static let __allTests__DelegatingErrorHandlerTests = [
+        ("testUncleanShutdownIsIgnored", testUncleanShutdownIsIgnored),
+    ]
+}
+
 extension FunctionalTestsAnonymousClient {
     // DO NOT MODIFY: This is autogenerated, use:
     //   `swift test --generate-linuxmain`
@@ -585,6 +594,7 @@ public func __allTests() -> [XCTestCaseEntry] {
         testCase(ClientTimeoutTests.__allTests__ClientTimeoutTests),
         testCase(ConnectionBackoffTests.__allTests__ConnectionBackoffTests),
         testCase(ConnectivityStateMonitorTests.__allTests__ConnectivityStateMonitorTests),
+        testCase(DelegatingErrorHandlerTests.__allTests__DelegatingErrorHandlerTests),
         testCase(FunctionalTestsAnonymousClient.__allTests__FunctionalTestsAnonymousClient),
         testCase(FunctionalTestsAnonymousClientNIOTS.__allTests__FunctionalTestsAnonymousClientNIOTS),
         testCase(FunctionalTestsInsecureTransport.__allTests__FunctionalTestsInsecureTransport),