Skip to content

Commit 24adeee

Browse files
gjcairogjcairo
committed
Add E2E TLS tests
1 parent 20829ca commit 24adeee

File tree

9 files changed

+468
-35
lines changed

9 files changed

+468
-35
lines changed

‎Package.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ let dependencies: [Package.Dependency] = [
5757
url: "https://github.com/apple/swift-nio-extras.git",
5858
from: "1.4.0"
5959
),
60+
.package(
61+
url: "https://github.com/apple/swift-certificates.git",
62+
from: "1.5.0"
63+
),
6064
]
6165

6266
let defaultSwiftSettings: [SwiftSetting] = [
@@ -104,6 +108,7 @@ let targets: [Target] = [
104108
.target(name: "GRPCNIOTransportCore"),
105109
.product(name: "GRPCCore", package: "grpc-swift"),
106110
.product(name: "NIOPosix", package: "swift-nio"),
111+
.product(name: "NIOSSL", package: "swift-nio-ssl"),
107112
],
108113
swiftSettings: defaultSwiftSettings
109114
),
@@ -132,6 +137,8 @@ let targets: [Target] = [
132137
name: "GRPCNIOTransportHTTP2Tests",
133138
dependencies: [
134139
.target(name: "GRPCNIOTransportHTTP2"),
140+
.product(name: "X509", package: "swift-certificates"),
141+
.product(name: "NIOSSL", package: "swift-nio-ssl"),
135142
]
136143
)
137144
]

‎Sources/GRPCNIOTransportCore/Client/Connection/Connection.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ package final class Connection: Sendable {
236236
// This state is tracked here so that if the connection events sequence finishes and the
237237
// connection never became ready then the connection can report that the connect failed.
238238
var isReady = false
239+
var unexpectedCloseError: (any Error)?
239240

240241
func makeNeverReadyError(cause: (any Error)?) -> RPCError {
241242
return RPCError(
@@ -265,10 +266,15 @@ package final class Connection: Sendable {
265266
// The connection will close at some point soon, yield a notification for this
266267
// because the close might not be imminent and this could result in address resolution.
267268
self.event.continuation.yield(.goingAway(errorCode, reason))
268-
case .idle, .keepaliveExpired, .initiatedLocally, .unexpected:
269+
case .idle, .keepaliveExpired, .initiatedLocally:
269270
// The connection will be closed imminently in these cases there's no need to do
270271
// anything.
271272
()
273+
case .unexpected(let error, _):
274+
// The connection will be closed imminently in this case.
275+
// We'll store the error that caused the unexpected closure so we
276+
// can surface it.
277+
unexpectedCloseError = error
272278
}
273279

274280
// Take the reason with the highest precedence. A GOAWAY may be superseded by user
@@ -318,7 +324,7 @@ package final class Connection: Sendable {
318324
finalEvent = .closed(connectionCloseReason)
319325
} else {
320326
// The connection never became ready, this therefore counts as a failed connect attempt.
321-
finalEvent = .connectFailed(makeNeverReadyError(cause: nil))
327+
finalEvent = .connectFailed(makeNeverReadyError(cause: unexpectedCloseError))
322328
}
323329

324330
// The connection events sequence has finished: the connection is now closed.

‎Sources/GRPCNIOTransportCore/Client/Connection/GRPCChannel.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ extension GRPCChannel {
290290
}
291291

292292
case .failRPC:
293-
return .stopTrying(RPCError(code: .unavailable, message: "channel isn't ready"))
293+
return .stopTrying(RPCError(code: .unavailable, message: "Channel isn't ready."))
294294
}
295295
}
296296

@@ -300,7 +300,7 @@ extension GRPCChannel {
300300
loadBalancer: LoadBalancer
301301
) async -> MakeStreamResult {
302302
guard let subchannel = loadBalancer.pickSubchannel() else {
303-
return .tryAgain(RPCError(code: .unavailable, message: "channel isn't ready"))
303+
return .tryAgain(RPCError(code: .unavailable, message: "Channel isn't ready."))
304304
}
305305

306306
let methodConfig = self.config(forMethod: descriptor)
@@ -768,8 +768,9 @@ extension GRPCChannel.StateMachine {
768768
result: .failure(
769769
RPCError(
770770
code: .unavailable,
771-
message: "channel isn't ready",
772-
cause: cause
771+
message: "Channel isn't ready.",
772+
cause: cause,
773+
flatteningCauses: true
773774
)
774775
)
775776
)
@@ -781,7 +782,7 @@ extension GRPCChannel.StateMachine {
781782
let continuations = state.queue.removeFastFailingEntries()
782783
actions.resumeContinuations = ConnectivityStateChangeActions.ResumableContinuations(
783784
continuations: continuations,
784-
result: .failure(RPCError(code: .unavailable, message: "channel isn't ready"))
785+
result: .failure(RPCError(code: .unavailable, message: "Channel isn't ready."))
785786
)
786787

787788
case .idle, .connecting:

‎Sources/GRPCNIOTransportCore/Client/Connection/LoadBalancers/RoundRobinLoadBalancer.swift

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,10 @@ extension ConnectivityState {
735735
static func aggregate(_ states: some Collection<ConnectivityState>) -> ConnectivityState {
736736
// See https://github.com/grpc/grpc/blob/master/doc/load-balancing.md
737737

738+
if states.isEmpty {
739+
return .shutdown
740+
}
741+
738742
// If any one subchannel is in READY state, the channel's state is READY.
739743
if states.contains(where: { $0 == .ready }) {
740744
return .ready
@@ -751,15 +755,15 @@ extension ConnectivityState {
751755
}
752756

753757
// Otherwise, if all subchannels are in state TRANSIENT_FAILURE, the channel's state is TRANSIENT_FAILURE.
758+
// Pick one of the errors to surface, as we can't surface all of them.
759+
var cause: RPCError!
754760
for state in states {
755-
guard case .transientFailure = state else {
761+
guard case .transientFailure(let error) = state else {
756762
return .shutdown
757763
}
764+
cause = error
758765
}
759766

760-
return .transientFailure(cause: RPCError(
761-
code: .internalError,
762-
message: "All subchannels are in TRANSIENT_FAILURE state."
763-
))
767+
return .transientFailure(cause: cause)
764768
}
765769
}

‎Sources/GRPCNIOTransportCore/Client/Connection/LoadBalancers/Subchannel.swift

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,8 @@ extension Subchannel {
256256
switch event {
257257
case .connectSucceeded:
258258
self.handleConnectSucceededEvent()
259-
case .connectFailed:
260-
self.handleConnectFailedEvent(in: &group)
259+
case .connectFailed(let cause):
260+
self.handleConnectFailedEvent(in: &group, error: cause)
261261
case .goingAway:
262262
self.handleGoingAwayEvent()
263263
case .closed(let reason):
@@ -282,21 +282,23 @@ extension Subchannel {
282282
}
283283
}
284284

285-
private func handleConnectFailedEvent(in group: inout DiscardingTaskGroup) {
285+
private func handleConnectFailedEvent(in group: inout DiscardingTaskGroup, error: any Error) {
286286
let onConnectFailed = self.state.withLock { $0.connectFailed(connector: self.connector) }
287287
switch onConnectFailed {
288288
case .connect(let connection):
289289
// Try the next address.
290290
self.runConnection(connection, in: &group)
291291

292292
case .backoff(let duration):
293+
let transientFailureCause = (error as? RPCError) ?? RPCError(
294+
code: .unavailable,
295+
message: "All addresses have been tried: backing off.",
296+
cause: error
297+
)
293298
// All addresses have been tried, backoff for some time.
294299
self.event.continuation.yield(
295300
.connectivityStateChanged(
296-
.transientFailure(cause: RPCError(
297-
code: .unavailable,
298-
message: "All addresses have been tried: backing off."
299-
))
301+
.transientFailure(cause: transientFailureCause)
300302
)
301303
)
302304
group.addTask {

‎Sources/GRPCNIOTransportHTTP2Posix/TLSConfig.swift

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,27 @@ extension HTTP2ServerTransport.Posix.Config {
171171
///
172172
/// If this is set to `true` but the client does not support ALPN, then the connection will be rejected.
173173
public var requireALPN: Bool
174+
175+
/// Create a new HTTP2 NIO Posix server transport TLS config.
176+
/// - Parameters:
177+
/// - certificateChain: The certificates the server will offer during negotiation.
178+
/// - privateKey: The private key associated with the leaf certificate.
179+
/// - clientCertificateVerification: How to verify the client certificate, if one is presented.
180+
/// - trustRoots: The trust roots to be used when verifying client certificates.
181+
/// - requireALPN: Whether ALPN is required.
182+
public init(
183+
certificateChain: [TLSConfig.CertificateSource],
184+
privateKey: TLSConfig.PrivateKeySource,
185+
clientCertificateVerification: TLSConfig.CertificateVerification,
186+
trustRoots: TLSConfig.TrustRootsSource,
187+
requireALPN: Bool
188+
) {
189+
self.certificateChain = certificateChain
190+
self.privateKey = privateKey
191+
self.clientCertificateVerification = clientCertificateVerification
192+
self.trustRoots = trustRoots
193+
self.requireALPN = requireALPN
194+
}
174195

175196
/// Create a new HTTP2 NIO Posix transport TLS config, with some values defaulted:
176197
/// - `clientCertificateVerificationMode` equals `doNotVerify`
@@ -180,18 +201,22 @@ extension HTTP2ServerTransport.Posix.Config {
180201
/// - Parameters:
181202
/// - certificateChain: The certificates the server will offer during negotiation.
182203
/// - privateKey: The private key associated with the leaf certificate.
204+
/// - configure: A closure which allows you to modify the defaults before returning them.
183205
/// - Returns: A new HTTP2 NIO Posix transport TLS config.
184206
public static func defaults(
185207
certificateChain: [TLSConfig.CertificateSource],
186-
privateKey: TLSConfig.PrivateKeySource
208+
privateKey: TLSConfig.PrivateKeySource,
209+
configure: (_ config: inout Self) -> Void = { _ in }
187210
) -> Self {
188-
Self(
211+
var config = Self(
189212
certificateChain: certificateChain,
190213
privateKey: privateKey,
191214
clientCertificateVerification: .noVerification,
192215
trustRoots: .systemDefault,
193216
requireALPN: false
194217
)
218+
configure(&config)
219+
return config
195220
}
196221

197222
/// Create a new HTTP2 NIO Posix transport TLS config, with some values defaulted to match
@@ -203,18 +228,22 @@ extension HTTP2ServerTransport.Posix.Config {
203228
/// - Parameters:
204229
/// - certificateChain: The certificates the server will offer during negotiation.
205230
/// - privateKey: The private key associated with the leaf certificate.
231+
/// - configure: A closure which allows you to modify the defaults before returning them.
206232
/// - Returns: A new HTTP2 NIO Posix transport TLS config.
207233
public static func mTLS(
208234
certificateChain: [TLSConfig.CertificateSource],
209-
privateKey: TLSConfig.PrivateKeySource
235+
privateKey: TLSConfig.PrivateKeySource,
236+
configure: (_ config: inout Self) -> Void = { _ in }
210237
) -> Self {
211-
Self(
238+
var config = Self(
212239
certificateChain: certificateChain,
213240
privateKey: privateKey,
214241
clientCertificateVerification: .noHostnameVerification,
215242
trustRoots: .systemDefault,
216243
requireALPN: false
217244
)
245+
configure(&config)
246+
return config
218247
}
219248
}
220249
}
@@ -255,6 +284,27 @@ extension HTTP2ClientTransport.Posix.Config {
255284

256285
/// An optional server hostname to use when verifying certificates.
257286
public var serverHostname: String?
287+
288+
/// Create a new HTTP2 NIO Posix client transport TLS config.
289+
/// - Parameters:
290+
/// - certificateChain: The certificates the client will offer during negotiation.
291+
/// - privateKey: The private key associated with the leaf certificate.
292+
/// - serverCertificateVerification: How to verify the server certificate, if one is presented.
293+
/// - trustRoots: The trust roots to be used when verifying server certificates.
294+
/// - serverHostname: An optional server hostname to use when verifying certificates.
295+
public init(
296+
certificateChain: [TLSConfig.CertificateSource],
297+
privateKey: TLSConfig.PrivateKeySource? = nil,
298+
serverCertificateVerification: TLSConfig.CertificateVerification,
299+
trustRoots: TLSConfig.TrustRootsSource,
300+
serverHostname: String? = nil
301+
) {
302+
self.certificateChain = certificateChain
303+
self.privateKey = privateKey
304+
self.serverCertificateVerification = serverCertificateVerification
305+
self.trustRoots = trustRoots
306+
self.serverHostname = serverHostname
307+
}
258308

259309
/// Create a new HTTP2 NIO Posix transport TLS config, with some values defaulted:
260310
/// - `certificateChain` equals `[]`
@@ -263,35 +313,46 @@ extension HTTP2ClientTransport.Posix.Config {
263313
/// - `trustRoots` equals `systemDefault`
264314
/// - `serverHostname` equals `nil`
265315
///
316+
/// - Parameters:
317+
/// - configure: A closure which allows you to modify the defaults before returning them.
266318
/// - Returns: A new HTTP2 NIO Posix transport TLS config.
267-
public static var defaults: Self {
268-
Self(
319+
public static func defaults(
320+
configure: (_ config: inout Self) -> Void = { _ in }
321+
) -> Self {
322+
var config = Self(
269323
certificateChain: [],
270324
privateKey: nil,
271325
serverCertificateVerification: .fullVerification,
272326
trustRoots: .systemDefault,
273327
serverHostname: nil
274328
)
329+
configure(&config)
330+
return config
275331
}
276332

277333
/// Create a new HTTP2 NIO Posix transport TLS config, with some values defaulted to match
278334
/// the requirements of mTLS:
279335
/// - `trustRoots` equals `systemDefault`
336+
/// - `serverCertificateVerification` equals `fullVerification`
280337
///
281338
/// - Parameters:
282339
/// - certificateChain: The certificates the client will offer during negotiation.
283340
/// - privateKey: The private key associated with the leaf certificate.
341+
/// - configure: A closure which allows you to modify the defaults before returning them.
284342
/// - Returns: A new HTTP2 NIO Posix transport TLS config.
285343
public static func mTLS(
286344
certificateChain: [TLSConfig.CertificateSource],
287-
privateKey: TLSConfig.PrivateKeySource
345+
privateKey: TLSConfig.PrivateKeySource,
346+
configure: (_ config: inout Self) -> Void = { _ in }
288347
) -> Self {
289-
Self(
348+
var config = Self(
290349
certificateChain: certificateChain,
291350
privateKey: privateKey,
292351
serverCertificateVerification: .fullVerification,
293352
trustRoots: .systemDefault
294353
)
354+
configure(&config)
355+
return config
295356
}
296357
}
297358
}

‎Sources/GRPCNIOTransportHTTP2TransportServices/TLSConfig.swift

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,26 @@ extension HTTP2ServerTransport.TransportServices.Config {
4545
/// If this is set to `true` but the client does not support ALPN, then the connection will be rejected.
4646
public var requireALPN: Bool
4747

48+
/// Create a new HTTP2 NIO Transport Services transport TLS config.
49+
/// - Parameters:
50+
/// - requireALPN: Whether ALPN is required.
51+
/// - identityProvider: A provider for the `SecIdentity` to be used when setting up TLS.
52+
public init(
53+
requireALPN: Bool,
54+
identityProvider: @Sendable @escaping () throws -> SecIdentity
55+
) {
56+
self.requireALPN = requireALPN
57+
self.identityProvider = identityProvider
58+
}
59+
4860
/// Create a new HTTP2 NIO Transport Services transport TLS config, with some values defaulted:
4961
/// - `requireALPN` equals `false`
5062
///
5163
/// - Returns: A new HTTP2 NIO Transport Services transport TLS config.
5264
public static func defaults(
5365
identityProvider: @Sendable @escaping () throws -> SecIdentity
5466
) -> Self {
55-
Self(
56-
identityProvider: identityProvider,
57-
requireALPN: false
58-
)
67+
Self(requireALPN: false, identityProvider: identityProvider)
5968
}
6069
}
6170
}

‎Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportNIOPosixTests.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ final class HTTP2TransportNIOPosixTests: XCTestCase {
410410
}
411411

412412
func testClientTLSConfig_Defaults() throws {
413-
let grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults
413+
let grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults()
414414
let nioSSLTLSConfig = try TLSConfiguration(grpcTLSConfig)
415415

416416
XCTAssertEqual(nioSSLTLSConfig.certificateChain, [])
@@ -422,7 +422,7 @@ final class HTTP2TransportNIOPosixTests: XCTestCase {
422422
}
423423

424424
func testClientTLSConfig_CustomCertificateChainAndPrivateKey() throws {
425-
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults
425+
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults()
426426
grpcTLSConfig.certificateChain = [
427427
.bytes(Array(Self.samplePemCert.utf8), format: .pem)
428428
]
@@ -451,7 +451,7 @@ final class HTTP2TransportNIOPosixTests: XCTestCase {
451451
}
452452

453453
func testClientTLSConfig_CustomTrustRoots() throws {
454-
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults
454+
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults()
455455
grpcTLSConfig.trustRoots = .certificates([.bytes(Array(Self.samplePemCert.utf8), format: .pem)])
456456
let nioSSLTLSConfig = try TLSConfiguration(grpcTLSConfig)
457457

@@ -467,7 +467,7 @@ final class HTTP2TransportNIOPosixTests: XCTestCase {
467467
}
468468

469469
func testClientTLSConfig_CustomCertificateVerification() throws {
470-
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults
470+
var grpcTLSConfig = HTTP2ClientTransport.Posix.Config.TLS.defaults()
471471
grpcTLSConfig.serverCertificateVerification = .noHostnameVerification
472472
let nioSSLTLSConfig = try TLSConfiguration(grpcTLSConfig)
473473

0 commit comments

Comments
 (0)