Skip to content

Commit

Permalink
Fix TLS E2E test flakiness (#46)
Browse files Browse the repository at this point in the history
Some of the E2E tests with TLS enabled were flaky on Darwin.

This PR:
- Slightly modifies the way they are implemented to make them not-flaky
- Modifies the options passed to `SecPKCS12Import` to make sure the
creation of the required `SecIdentity` happens in memory only and
doesn't use the keychain
- Tolerates a broken pipe error when the server fails client cert
validation during an mTLS handshake, as it's possible for
Network.framework to expose this error instead of the actual SSL
validation error on the client
  • Loading branch information
gjcairo authored Dec 12, 2024
1 parent a7b2f50 commit e76bc65
Showing 1 changed file with 81 additions and 68 deletions.
149 changes: 81 additions & 68 deletions Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTLSEnabledTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,52 +103,52 @@ struct HTTP2TransportTLSEnabledTests {
let clientTransportConfig = self.makeDefaultTLSClientConfig(
for: clientTransport,
certificateKeyPairs: certificateKeyPairs,
authority: nil
authority: "wrong-hostname"
)
let serverTransportConfig = self.makeDefaultTLSServerConfig(
for: serverTransport,
certificateKeyPairs: certificateKeyPairs
)

try await self.withClientAndServer(
clientConfig: clientTransportConfig,
serverConfig: serverTransportConfig
) { control in
await #expect {
await #expect {
try await self.withClientAndServer(
clientConfig: clientTransportConfig,
serverConfig: serverTransportConfig
) { control in
try await self.executeUnaryRPC(control: control)
} throws: { error in
let rootError = try #require(error as? RPCError)
#expect(rootError.code == .unavailable)

switch clientTransport {
case .posix:
#expect(
rootError.message
== "The server accepted the TCP connection but closed the connection before completing the HTTP/2 connection preface."
)
let sslError = try #require(rootError.cause as? NIOSSLExtraError)
guard sslError == .failedToValidateHostname else {
Issue.record(
"Should be a NIOSSLExtraError.failedToValidateHostname error, but was: \(String(describing: rootError.cause))"
)
return false
}
}
} throws: { error in
let rootError = try #require(error as? RPCError)
#expect(rootError.code == .unavailable)

#if canImport(Network)
case .transportServices:
#expect(rootError.message.starts(with: "Could not establish a connection to"))
let nwError = try #require(rootError.cause as? NWError)
guard case .tls(Security.errSSLBadCert) = nwError else {
Issue.record(
"Should be a NWError.tls(-9808/errSSLBadCert) error, but was: \(String(describing: rootError.cause))"
)
return false
}
#endif
switch clientTransport {
case .posix:
#expect(
rootError.message
== "The server accepted the TCP connection but closed the connection before completing the HTTP/2 connection preface."
)
let sslError = try #require(rootError.cause as? NIOSSLExtraError)
guard sslError == .failedToValidateHostname else {
Issue.record(
"Should be a NIOSSLExtraError.failedToValidateHostname error, but was: \(String(describing: rootError.cause))"
)
return false
}

return true
#if canImport(Network)
case .transportServices:
#expect(rootError.message.starts(with: "Could not establish a connection to"))
let nwError = try #require(rootError.cause as? NWError)
guard case .tls(Security.errSSLBadCert) = nwError else {
Issue.record(
"Should be a NWError.tls(-9808/errSSLBadCert) error, but was: \(String(describing: rootError.cause))"
)
return false
}
#endif
}

return true
}
}

Expand All @@ -174,44 +174,53 @@ struct HTTP2TransportTLSEnabledTests {
includeClientCertificateInTrustRoots: true
)

try await self.withClientAndServer(
clientConfig: clientTransportConfig,
serverConfig: serverTransportConfig
) { control in
await #expect {
await #expect {
try await self.withClientAndServer(
clientConfig: clientTransportConfig,
serverConfig: serverTransportConfig
) { control in
try await self.executeUnaryRPC(control: control)
} throws: { error in
let rootError = try #require(error as? RPCError)
#expect(rootError.code == .unavailable)
#expect(
rootError.message
== "The server accepted the TCP connection but closed the connection before completing the HTTP/2 connection preface."
)
}
} throws: { error in
let rootError = try #require(error as? RPCError)
#expect(rootError.code == .unavailable)
#expect(
rootError.message
== "The server accepted the TCP connection but closed the connection before completing the HTTP/2 connection preface."
)

switch clientTransport {
case .posix:
let sslError = try #require(rootError.cause as? NIOSSL.BoringSSLError)
guard case .sslError = sslError else {
Issue.record(
"Should be a NIOSSL.sslError error, but was: \(String(describing: rootError.cause))"
)
return false
}
switch clientTransport {
case .posix:
let sslError = try #require(rootError.cause as? NIOSSL.BoringSSLError)
guard case .sslError = sslError else {
Issue.record(
"Should be a NIOSSL.sslError error, but was: \(String(describing: rootError.cause))"
)
return false
}

#if canImport(Network)
case .transportServices:
let nwError = try #require(rootError.cause as? NWError)
guard case .tls(Security.errSSLPeerCertUnknown) = nwError else {
Issue.record(
"Should be a NWError.tls(-9829/errSSLPeerCertUnknown) error, but was: \(String(describing: rootError.cause))"
)
return false
#if canImport(Network)
case .transportServices:
let nwError = try #require(rootError.cause as? NWError)
guard case .tls(Security.errSSLPeerCertUnknown) = nwError else {
// When the TLS handshake fails, the connection will be closed from the client.
// Network.framework will generally surface the right SSL error (in this case, an "unknown
// certificate" from the server), but it will sometimes instead return the broken pipe
// error caused by the underlying TLS handshake handler closing the connection:
// we should tolerate this.
if case .posix(POSIXErrorCode.EPIPE) = nwError {
return true
}
#endif
}

return true
Issue.record(
"Should be a NWError.tls(-9829/errSSLPeerCertUnknown) error, but was: \(String(describing: rootError.cause))"
)
return false
}
#endif
}

return true
}
}

Expand Down Expand Up @@ -341,7 +350,11 @@ struct HTTP2TransportTLSEnabledTests {
privateKey: try NIOSSLPrivateKey(bytes: privateKeyBytes, format: .der)
)
let pkcs12Bytes = try bundle.serialize(passphrase: password.utf8)
let options = [kSecImportExportPassphrase as String: password]
let options =
[
kSecImportExportPassphrase as String: password,
kSecImportToMemoryOnly: kCFBooleanTrue!,
] as [AnyHashable: Any]
var rawItems: CFArray?
let status = SecPKCS12Import(
Data(pkcs12Bytes) as CFData,
Expand Down

0 comments on commit e76bc65

Please sign in to comment.