Skip to content

Commit bd9a48b

Browse files
josephnoirjosephnoir
committed
Address review
1 parent 17b6b37 commit bd9a48b

File tree

4 files changed

+46
-23
lines changed

4 files changed

+46
-23
lines changed

‎Package.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ let dependencies: [Package.Dependency] = [
7171

7272
// This adds some build settings which allow us to map "@available(gRPCSwiftNIOTransport 2.x, *)" to
7373
// the appropriate OS platforms.
74-
let nextMinorVersion = 1
74+
let nextMinorVersion = 2
7575
let availabilitySettings: [SwiftSetting] = (0 ... nextMinorVersion).map { minor in
7676
let name = "gRPCSwiftNIOTransport"
7777
let version = "2.\(minor)"

‎Sources/GRPCNIOTransportHTTP2Posix/Config+TLS.swift‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
private import GRPCCore
18-
public import NIO
18+
public import NIOCore
1919
public import NIOCertificateReloading
2020
public import NIOSSL
2121

@@ -170,10 +170,13 @@ extension HTTP2ServerTransport.Posix.TransportSecurity {
170170

171171
/// Override the certificate verification with a custom callback that must return the verified certificate chain on success.
172172
/// Note: The callback is only used when `clientCertificateVerification` is *not* set to `noVerification`!
173+
@available(gRPCSwiftNIOTransport 2.2, *)
173174
public var customVerificationCallback:
174175
(
175-
@Sendable ([NIOSSLCertificate], EventLoopPromise<NIOSSLVerificationResultWithMetadata>) ->
176-
Void
176+
@Sendable (
177+
_ certificates: [NIOSSLCertificate],
178+
_ promise: EventLoopPromise<NIOSSLVerificationResultWithMetadata>
179+
) -> Void
177180
)?
178181

179182
/// Create a new HTTP2 NIO Posix server transport TLS config.

‎Sources/GRPCNIOTransportHTTP2Posix/HTTP2ServerTransport+Posix.swift‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,16 @@ extension HTTP2ServerTransport {
6767
serverQuiescingHelper: ServerQuiescingHelper
6868
) async throws -> NIOAsyncChannel<AcceptedChannel, Never> {
6969
let sslContext: NIOSSLContext?
70-
let tlsConfiguration: HTTP2ServerTransport.Posix.TransportSecurity.TLS?
70+
let customVerificationCallback: (
71+
@Sendable (
72+
[NIOSSLCertificate], EventLoopPromise<NIOSSLVerificationResultWithMetadata>
73+
) -> Void
74+
)?
7175

7276
switch self.transportSecurity.wrapped {
7377
case .plaintext:
7478
sslContext = nil
75-
tlsConfiguration = nil
79+
customVerificationCallback = nil
7680
case .tls(let tlsConfig):
7781
do {
7882
sslContext = try NIOSSLContext(configuration: TLSConfiguration(tlsConfig))
@@ -83,7 +87,7 @@ extension HTTP2ServerTransport {
8387
cause: error
8488
)
8589
}
86-
tlsConfiguration = tlsConfig
90+
customVerificationCallback = tlsConfig.customVerificationCallback
8791
}
8892

8993
let serverChannel = try await ServerBootstrap(group: eventLoopGroup)
@@ -102,7 +106,7 @@ extension HTTP2ServerTransport {
102106
.bind(to: address) { channel in
103107
channel.eventLoop.makeCompletedFuture {
104108
if let sslContext {
105-
if let callback = tlsConfiguration?.customVerificationCallback {
109+
if let callback = customVerificationCallback {
106110
try channel.pipeline.syncOperations.addHandler(
107111
NIOSSLServerHandler(
108112
context: sslContext,
@@ -197,11 +201,10 @@ extension HTTP2ServerTransport {
197201
}
198202

199203
public func listen(
200-
streamHandler:
201-
@escaping @Sendable (
202-
_ stream: RPCStream<Inbound, Outbound>,
203-
_ context: ServerContext
204-
) async -> Void
204+
streamHandler: @escaping @Sendable (
205+
_ stream: RPCStream<Inbound, Outbound>,
206+
_ context: ServerContext
207+
) async -> Void
205208
) async throws {
206209
try await self.underlyingTransport.listen(streamHandler: streamHandler)
207210
}

‎Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTLSEnabledTests.swift‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Foundation
1818
import GRPCCore
1919
import GRPCNIOTransportHTTP2Posix
2020
import GRPCNIOTransportHTTP2TransportServices
21-
import NIO
21+
import NIOCore
2222
import NIOSSL
2323
import SwiftASN1
2424
import Testing
@@ -192,15 +192,18 @@ struct HTTP2TransportTLSEnabledTests {
192192
func testRPC_mTLS_customVerificationCallback_OK() async throws {
193193
// Create a new certificate chain that has 4 certificate/key pairs: root, intermediate, client, server
194194
let certificateChain = try CertificateChain()
195+
let certificatesExpectedInCallback = [certificateChain.client.certificate]
195196
let filePaths = try certificateChain.writeToTemp()
197+
196198
let clientConfig = self.makeMTLSClientConfig(
197199
certificatePath: filePaths.clientCert,
198200
keyPath: filePaths.clientKey,
199201
trustRootsPath: filePaths.trustRoots,
200202
serverHostname: CertificateChain.serverName
201203
)
202-
let expectedCertificates = [certificateChain.client.certificate]
203-
try await confirmation { confirmation in
204+
205+
// The confirmation lets us check that the callback is used.
206+
try await confirmation(expectedCount: 1) { confirmation in
204207
let serverConfig = self.makeMTLSServerConfigWithCallback(
205208
certificatePath: filePaths.serverCert,
206209
keyPath: filePaths.serverKey,
@@ -209,12 +212,15 @@ struct HTTP2TransportTLSEnabledTests {
209212
let presentedCertificates = certificates.map {
210213
try! Certificate(derEncoded: $0.toDERBytes())
211214
}
212-
#expect(expectedCertificates == presentedCertificates)
215+
#expect(certificatesExpectedInCallback == presentedCertificates)
216+
// "Verify" the chain and set the certificate.
213217
promise.succeed(
214218
.certificateVerified(VerificationMetadata(ValidatedCertificateChain(certificates)))
215219
)
220+
// This should be called once.
216221
confirmation.confirm()
217222
}
223+
218224
// Run the test
219225
try await self.withClientAndServer(
220226
clientConfig: clientConfig,
@@ -234,14 +240,17 @@ struct HTTP2TransportTLSEnabledTests {
234240
{
235241
// Create a new certificate chain that has 4 certificate/key pairs: root, intermediate, client, server
236242
let certificateChain = try CertificateChain()
243+
let certificatesExpectedInCallback = [certificateChain.client.certificate]
237244
let filePaths = try certificateChain.writeToTemp()
245+
238246
let clientConfig = self.makeMTLSClientConfig(
239247
certificatePath: filePaths.clientCert,
240248
keyPath: filePaths.clientKey,
241249
trustRootsPath: filePaths.trustRoots,
242250
serverHostname: CertificateChain.serverName
243251
)
244-
let expectedCertificates = [certificateChain.client.certificate]
252+
253+
// The confirmation lets us check that the callback is not used.
245254
try await confirmation(expectedCount: 0) { confirmation in
246255
let serverConfig = self.makeMTLSServerConfigWithCallback(
247256
certificatePath: filePaths.serverCert,
@@ -252,13 +261,15 @@ struct HTTP2TransportTLSEnabledTests {
252261
let presentedCertificates = certificates.map {
253262
try! Certificate(derEncoded: $0.toDERBytes())
254263
}
255-
#expect(expectedCertificates == presentedCertificates)
264+
#expect(certificatesExpectedInCallback == presentedCertificates)
265+
// "Verify" the chain and set the certificate.
256266
promise.succeed(
257267
.certificateVerified(VerificationMetadata(ValidatedCertificateChain(certificates)))
258268
)
259-
// We expect this never to be called. The call count should stay at 0.
269+
// We expect this never to be called.
260270
confirmation()
261271
}
272+
262273
// Run the test
263274
try await self.withClientAndServer(
264275
clientConfig: clientConfig,
@@ -277,14 +288,17 @@ struct HTTP2TransportTLSEnabledTests {
277288
func testRPC_mTLS_customVerificationCallback_Failure() async throws {
278289
// Create a new certificate chain that has 4 certificate/key pairs: root, intermediate, client, server
279290
let certificateChain = try CertificateChain()
291+
let certificatesExpectedInCallback = [certificateChain.client.certificate]
280292
let filePaths = try certificateChain.writeToTemp()
293+
281294
let clientConfig = self.makeMTLSClientConfig(
282295
certificatePath: filePaths.clientCert,
283296
keyPath: filePaths.clientKey,
284297
trustRootsPath: filePaths.trustRoots,
285298
serverHostname: CertificateChain.serverName
286299
)
287-
let expectedCertificates = [certificateChain.client.certificate]
300+
301+
// The confirmation lets us check that the callback is used.
288302
await confirmation { confirmation in
289303
let serverConfig = self.makeMTLSServerConfigWithCallback(
290304
certificatePath: filePaths.serverCert,
@@ -294,11 +308,12 @@ struct HTTP2TransportTLSEnabledTests {
294308
let presentedCertificates = certificates.map {
295309
try! Certificate(derEncoded: $0.toDERBytes())
296310
}
297-
#expect(expectedCertificates == presentedCertificates)
311+
#expect(certificatesExpectedInCallback == presentedCertificates)
298312
// We are failing the certificate check here by propagating ".failed"!
299313
promise.succeed(.failed)
300314
confirmation.confirm()
301315
}
316+
302317
// Run the test
303318
await #expect {
304319
try await self.withClientAndServer(
@@ -308,13 +323,15 @@ struct HTTP2TransportTLSEnabledTests {
308323
try await self.executeUnaryRPC(control: control)
309324
}
310325
} throws: { error in
326+
// Check root error ...
311327
let rootError = try #require(error as? RPCError)
312328
#expect(rootError.code == .unavailable)
313-
314329
#expect(
315330
rootError.message
316331
== "The server accepted the TCP connection but closed the connection before completing the HTTP/2 connection preface."
317332
)
333+
334+
// ... and the its cause.
318335
let sslError = try #require(rootError.cause as? BoringSSLError)
319336
switch sslError {
320337
case .sslError:

0 commit comments

Comments
 (0)