Repository: grpc/grpc-swift (swift)

Issue:
Certificate pinning with NIO?
### Question Checklist

- [x] Updated grpc-swift to the latest version
- [x] I read the [Contribution Guidelines](https://github.com/grpc/grpc-swift/blob/master/CONTRIBUTING.md)
- [x] I read [README](https://github.com/grpc/grpc-swift/blob/master/README.md)
- [x] I searched for [existing GitHub issues](https://github.com/grpc/grpc-swift/issues) (especially, there are many [resolved issues related to SSL](https://github.com/grpc/grpc-swift/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aclosed+ssl))

### Question Subject
<!-- Is this a question about documentation? -->
<!-- Is this a question about a third party plugin? (If so, please go to the plugin repository first) -->

Does the NIO implementation of Swift GRPC support certificate pinning? If so, how do I use it?

#### Question Description
<!-- Please include expected behavior and any relevant code samples with your question if possible -->
<!-- Please wrap log statements and code in triple backticks (```) so they get printed correctly on GitHub. -->

I should preface this by saying I only have a basic grasp on certificate pinning and TSL in general. My understanding of it is as follows: instead of trusting any certificate signed by any of the default trust roots provided by my OS, certificate pinning means that I specify which specific certificate I want to trust, and any connection signed by any other certificate should fail to authenticate.

I have tried the following in order to use this:

**1. Got the certificate from our server**

I ran
```
openssl s_client -servername <servername> -host <hostname> -port <port> -showcerts < /dev/null
```

which yielded 2 certificates.

**2. Instantiated the certificates in code**

```
let certificateString1 = """
        -----BEGIN CERTIFICATE-----
        ...
        -----END CERTIFICATE-----
        """

let certificateString2 = """
        -----BEGIN CERTIFICATE-----
        ...
        -----END CERTIFICATE-----
        """

func getCertificate(for certificateString: String) -> NIOSSLCertificate? {
        do {
            let bytes: [Byte] = certificateString.utf8.map { $0 }
            let certificate = try NIOSSLCertificate(bytes: bytes, format: .pem)
            return certificate
        } catch let error {
            print(error)
            return nil
        }
}

let certificate1 = getCertificate(for: certificateString1)!
let certificate2 = getCertificate(for: certificateString2)!
let certificates = [certificate1, certificate2]

let certificateSource1 = NIOSSLCertificateSource.certificate(certificate1)
let certificateSource2 = NIOSSLCertificateSource.certificate(certificate2)
let certificateSources = [certificateSource1, certificateSource2]
```

**3. Created some different TLS configurations**

Configuration A:
- My certificates given as the chain
- Empty trust roots

```
let tlsA = ClientConnection.Configuration.TLS(
        certificateChain: certificateSources,
        trustRoots: .certificates([]))
```

Configuration B:
- Empty certificate chain
- My certificates given as the trust roots

```
let tlsB = ClientConnection.Configuration.TLS(
        certificateChain: [],
        trustRoots: .certificates(self.certificates))
```

Configuration C:
- My certificates given as the chain
- My certificates given as the trust roots

```
let tlsC = ClientConnection.Configuration.TLS(
        certificateChain: certificateSources,
        trustRoots: .certificates(self.certificates))
```

Configuration D:
- Using the default certificate chain
- My certificates given as the trust roots

```
let tlsD = ClientConnection.Configuration.TLS(
        trustRoots: .certificates(self.certificates))
```

Configuration E:
- Using my certificates as the chain
- Using the default trust roots

```
let tlsE = ClientConnection.Configuration.TLS(
        certificateChain: certificateSources)
```

**4. Created and used a client connection with each of these TLS configurations.**

I won't post the code here, but I will note that this part works fine if I do not attempt to set up certificate pinning; when I just pass `tls: .init()` to my client connection configuration, my endpoint calls authenticate correctly and everything. Things only start breaking once I use one of the configurations above.

**5. Results**

With configurations A-D, I get errors like these:

Errors:
```
2020-02-14T09:07:46+0200 error: error=handshakeFailed(NIOSSL.BoringSSLError.sslError([Error: 268435581 error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED])) error caught before TLS was verified
2020-02-14T09:07:46+0200 error: error=handshakeFailed(NIOSSL.BoringSSLError.sslError([Error: 268435581 error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED])) grpc client error
2020-02-14T09:07:46+0200 notice: connection_id=21C06D0C-1C94-4903-BCDF-8BE8B6DD86D2 error=handshakeFailed(NIOSSL.BoringSSLError.sslError([Error: 268435581 error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED])) connection attempt failed
```

When I use configuration E, the call succeeds; however, when I change the certificate (by substituting some of the caracters with `0`s), the call *still* succeeds, which indicates that my certificates are not being used for authentication at all. This makes sense; the certificates are signed by trusted CAs, so using the default trust roots would cause my connection to succeed.

**Overview**

My own inexperience with TLS makes it difficult for to pinpoint whether my problems originate from my own code, the Swift GRPC / NIO SSL libraries, or the server. So it would be extremely helpful if someone could tell me whether Swift GRPC supports certificate pinning, and if so, how to use it.

Pull request:
Hi,

This PR adds the ability for client connections to override the certificate verification logic implemented in nio-ssl.

The main goal is to allow client apps to perform their own certificate verification logic, such as SSL public key pinning.
Nio-ssl provide this feature as described here: [nio-ssl-issue-25](https://github.com/apple/swift-nio-ssl/issues/25) 
But I wasn't able to use it through grpc-swift because the `NIOSSLCustomVerificationCallback` is not accessible when initializing a secure `ClientConnection`.

I exposed a `NIOSSLCustomVerificationCallback` property in `ClientConnection.Configuration` struct and provided a `ClientConnection.Builder` func as well.

## Usage example for client public key pinning

```swift
let myPublicKey = Array(Data(base64Encoded: """
.... (Your base64 encoded public key)
""", options: .ignoreUnknownCharacters)!)

ClientConnection.secure(group: PlatformSupport.makeEventLoopGroup(loopCount: 1))
            .withTLSCustomVerificationCallback({ cert, eventLoopVerification in
                // Extract the leaf certificate's public key,
                // then compare it to the one you have.
                if let leaf = cert.first,
                   let publicKey = try? leaf.extractPublicKey() {

                    if let certSPKI = try? publicKey.toSPKIBytes(),
                       myPublicKey == certSPKI {
                        eventLoopVerification.succeed(.certificateVerified)
                    } else {
                        eventLoopVerification.fail(NIOSSLError.unableToValidateCertificate)
                    }
                } else {
                    eventLoopVerification.fail(NIOSSLError.noCertificateToValidate)
                }
            })
            .connect(host: host, port: port)
```

This PR + the example above should resolve #723 and resolve #1104

Patch:
diff --git a/Sources/GRPC/ClientConnection.swift b/Sources/GRPC/ClientConnection.swift
index ba8480270..4488c1b52 100644
--- a/Sources/GRPC/ClientConnection.swift
+++ b/Sources/GRPC/ClientConnection.swift
@@ -411,7 +411,8 @@ extension Channel {
     connectionIdleTimeout: TimeAmount,
     errorDelegate: ClientErrorDelegate?,
     requiresZeroLengthWriteWorkaround: Bool,
-    logger: Logger
+    logger: Logger,
+    customVerificationCallback: NIOSSLCustomVerificationCallback?
   ) -> EventLoopFuture<Void> {
     // We add at most 8 handlers to the pipeline.
     var handlers: [ChannelHandler] = []
@@ -427,11 +428,20 @@ extension Channel {
 
     if let tlsConfiguration = tlsConfiguration {
       do {
-        let sslClientHandler = try NIOSSLClientHandler(
-          context: try NIOSSLContext(configuration: tlsConfiguration),
-          serverHostname: tlsServerHostname
-        )
-        handlers.append(sslClientHandler)
+        if let customVerificationCallback = customVerificationCallback {
+          let sslClientHandler = try NIOSSLClientHandler(
+            context: try NIOSSLContext(configuration: tlsConfiguration),
+            serverHostname: tlsServerHostname,
+            customVerificationCallback: customVerificationCallback
+          )
+          handlers.append(sslClientHandler)
+        } else {
+          let sslClientHandler = try NIOSSLClientHandler(
+            context: try NIOSSLContext(configuration: tlsConfiguration),
+            serverHostname: tlsServerHostname
+          )
+          handlers.append(sslClientHandler)
+        }
         handlers.append(TLSVerificationHandler(logger: logger))
       } catch {
         return self.eventLoop.makeFailedFuture(error)
diff --git a/Sources/GRPC/ConnectionManager.swift b/Sources/GRPC/ConnectionManager.swift
index 84b50483c..14404d195 100644
--- a/Sources/GRPC/ConnectionManager.swift
+++ b/Sources/GRPC/ConnectionManager.swift
@@ -853,7 +853,8 @@ extension ConnectionManager {
             group: self.eventLoop,
             hasTLS: self.configuration.tls != nil
           ),
-          logger: self.logger
+          logger: self.logger,
+          customVerificationCallback: self.configuration.tls?.customVerificationCallback
         )
 
         // Run the debug initializer, if there is one.
diff --git a/Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift b/Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift
index aeab44830..80fd65da7 100644
--- a/Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift
+++ b/Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift
@@ -235,6 +235,15 @@ extension ClientConnection.Builder.Secure {
     self.tls.certificateVerification = certificateVerification
     return self
   }
+
+  /// A custom verification callback that allows completely overriding the certificate verification logic.
+  @discardableResult
+  public func withTLSCustomVerificationCallback(
+    _ callback: @escaping NIOSSLCustomVerificationCallback
+  ) -> Self {
+    self.tls.customVerificationCallback = callback
+    return self
+  }
 }
 
 extension ClientConnection.Builder {
diff --git a/Sources/GRPC/TLSConfiguration.swift b/Sources/GRPC/TLSConfiguration.swift
index 746ffede1..46e28f260 100644
--- a/Sources/GRPC/TLSConfiguration.swift
+++ b/Sources/GRPC/TLSConfiguration.swift
@@ -68,6 +68,9 @@ extension ClientConnection.Configuration {
       }
     }
 
+    /// A custom verification callback that allows completely overriding the certificate verification logic for this connection.
+    public var customVerificationCallback: NIOSSLCustomVerificationCallback?
+
     /// TLS Configuration with suitable defaults for clients.
     ///
     /// This is a wrapper around `NIOSSL.TLSConfiguration` to restrict input to values which comply
@@ -83,12 +86,15 @@ extension ClientConnection.Configuration {
     ///     `.fullVerification`.
     /// - Parameter hostnameOverride: Value to use for TLS SNI extension; this must not be an IP
     ///     address, defaults to `nil`.
+    /// - Parameter customVerificationCallback: A callback to provide to override the certificate verification logic,
+    ///     defaults to `nil`.
     public init(
       certificateChain: [NIOSSLCertificateSource] = [],
       privateKey: NIOSSLPrivateKeySource? = nil,
       trustRoots: NIOSSLTrustRoots = .default,
       certificateVerification: CertificateVerification = .fullVerification,
-      hostnameOverride: String? = nil
+      hostnameOverride: String? = nil,
+      customVerificationCallback: NIOSSLCustomVerificationCallback? = nil
     ) {
       self.configuration = .forClient(
         minimumTLSVersion: .tlsv12,
@@ -99,6 +105,7 @@ extension ClientConnection.Configuration {
         applicationProtocols: GRPCApplicationProtocolIdentifier.client
       )
       self.hostnameOverride = hostnameOverride
+      self.customVerificationCallback = customVerificationCallback
     }
 
     /// Creates a TLS Configuration using the given `NIOSSL.TLSConfiguration`.


Test patch:
diff --git a/Tests/GRPCTests/ClientTLSFailureTests.swift b/Tests/GRPCTests/ClientTLSFailureTests.swift
index 5c6adcb74..22e886cae 100644
--- a/Tests/GRPCTests/ClientTLSFailureTests.swift
+++ b/Tests/GRPCTests/ClientTLSFailureTests.swift
@@ -180,4 +180,49 @@ class ClientTLSFailureTests: GRPCTestCase {
       XCTFail("Expected NIOSSLExtraError.failedToValidateHostname")
     }
   }
+
+  func testClientConnectionFailsWhenCertificateValidationDenied() throws {
+    let errorExpectation = self.expectation(description: "error")
+    // 2 errors: one for the failed handshake, and another for failing the ready-channel promise
+    // (because the handshake failed).
+    errorExpectation.expectedFulfillmentCount = 2
+
+    let tlsConfiguration = ClientConnection.Configuration.TLS(
+      certificateChain: [.certificate(SampleCertificate.client.certificate)],
+      privateKey: .privateKey(SamplePrivateKey.client),
+      trustRoots: .certificates([SampleCertificate.ca.certificate]),
+      hostnameOverride: SampleCertificate.server.commonName,
+      customVerificationCallback: { _, promise in
+        // The certificate validation is forced to fail
+        promise.fail(NIOSSLError.unableToValidateCertificate)
+      }
+    )
+
+    var configuration = self.makeClientConfiguration(tls: tlsConfiguration)
+    let errorRecorder = ErrorRecordingDelegate(expectation: errorExpectation)
+    configuration.errorDelegate = errorRecorder
+
+    let stateChangeDelegate = RecordingConnectivityDelegate()
+    stateChangeDelegate.expectChanges(2) { changes in
+      XCTAssertEqual(changes, [
+        Change(from: .idle, to: .connecting),
+        Change(from: .connecting, to: .shutdown),
+      ])
+    }
+    configuration.connectivityStateDelegate = stateChangeDelegate
+
+    // Start an RPC to trigger creating a channel.
+    let echo = Echo_EchoClient(channel: ClientConnection(configuration: configuration))
+    _ = echo.get(.with { $0.text = "foo" })
+
+    self.wait(for: [errorExpectation], timeout: self.defaultTestTimeout)
+    stateChangeDelegate.waitForExpectedChanges(timeout: .seconds(5))
+
+    if let nioSSLError = errorRecorder.errors.first as? NIOSSLError,
+      case .handshakeFailed(.sslError) = nioSSLError {
+      // Expected case.
+    } else {
+      XCTFail("Expected NIOSSLError.handshakeFailed(BoringSSL.sslError)")
+    }
+  }
 }