You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Context: I'm using swift-nio-ssl through Swift GRPC. And found some improvement points along the way of their API.
see: grpc/grpc-swift#1459
API Calls that make use of this initializer
/// Initialize a context that will create multiple connections, all with the same/// configuration.internalinit(
configuration:TLSConfiguration,
callbackManager:CallbackManagerProtocol?,
additionalCertificateChainVerification:AdditionalCertificateChainVerificationCallback?)throws{
end up being hard to follow due to the complex throwing interface. Source Code
Clients of this API could receive a lot of errors for many reasons and they are not simple to streamline to clients. I don't know much about this at all, so I read through the code and made some notes on what the failure points are and the errors they return.
Throwing reason 1: PEM Files could fail to read throws self-explanatory error 🥰 ✅
// On non-Linux platforms, when using the platform default trust roots, we make use of a
// custom verify callback. If we have also been presented with additional trust roots of
// type `.file`, we take the opportunity now to load them in memory to avoid doing so
// repeatedly on the request path.
//
// However, to avoid closely coupling this code with other parts (e.g. the platform-specific
// concerns, and the defaulting of `trustRoots` to `.default` when `nil`), we unilaterally
// convert any `additionalTrustRoots` of type `.file` to `.certificates`.
var configuration = configuration
configuration.additionalTrustRoots = try configuration.additionalTrustRoots.map { trustRoots in
switch trustRoots {
case .file(let path):
return .certificates(try NIOSSLCertificate.fromPEMFile(path))
default:
return trustRoots
}
}
Throwing reason 2: Verification Algorithms cause some error code that's not detailed but it could be better typed ⚠️
// Configure verification algorithms
if let verifySignatureAlgorithms = configuration.verifySignatureAlgorithms {
returnCode = verifySignatureAlgorithms
.map { $0.rawValue }
.withUnsafeBufferPointer { algo in
CNIOBoringSSL_SSL_CTX_set_verify_algorithm_prefs(context, algo.baseAddress, algo.count)
}
if returnCode != 1 {
let errorStack = BoringSSLError.buildErrorStack()
throw BoringSSLError.unknownError(errorStack)
}
}
Throwing reason 3: Configuring signing algorithms fail with some error that's not detailed but it could be better typed ⚠️
// Configure signing algorithms
if let signingSignatureAlgorithms = configuration.resolvedSigningSignatureAlgorithms {
returnCode = signingSignatureAlgorithms
.map{ $0.rawValue }.withUnsafeBufferPointer{ algo inCNIOBoringSSL_SSL_CTX_set_signing_algorithm_prefs(context, algo.baseAddress, algo.count)}
if returnCode !=1{leterrorStack=BoringSSLError.buildErrorStack()throwBoringSSLError.unknownError(errorStack)}}
Throwing Reason 4: Certificate chain setup failures that DO have typed self-explanatory errors 🥰✅
throws a typed error BoringSSLError.failedToSetALPN(errorStack)
Throwing Reason 7: adding key log callback calls throws but it does not call any throwing functions. ⚠️
// Add a key log callback.
if let keyLogCallback = configuration.keyLogCallback {
self.keyLogManager = KeyLogCallbackManager(callback: keyLogCallback)
try NIOSSLContext.setKeylogCallback(context: context)
} else {
self.keyLogManager = nil
}
This function doesn't appear to be throwing anything
private static func setKeylogCallback(context: OpaquePointer) throws {
CNIOBoringSSL_SSL_CTX_set_keylog_callback(context) { (ssl, linePointer) in
guard let ssl = ssl, let linePointer = linePointer else {
return
}
// We want to take the SSL pointer and extract the parent Swift object. These force-unwraps are for
// safety: a correct NIO program can never fail to set these pointers, and if it does failing loudly is
// more useful than failing quietly.
let parentCtx = CNIOBoringSSL_SSL_get_SSL_CTX(ssl)!
let parentPtr = CNIOBoringSSLShims_SSL_CTX_get_app_data(parentCtx)!
let parentSwiftContext: NIOSSLContext = Unmanaged.fromOpaque(parentPtr).takeUnretainedValue()
// Similarly, this force-unwrap is safe because a correct NIO program can never fail to unwrap this entry
// either.
parentSwiftContext.keyLogManager!.log(linePointer)
}
}
It would be great if some error interface would be documented for developers to expect, wrap and upstream with the adecquate to callers.
Great work and thanks for your time!
The text was updated successfully, but these errors were encountered:
pacu
changed the title
Enhancement - NIOSSLContext throwing initializer error API is difficult to follow
Enhancement - NIOSSLContext throwing initializer error API may be difficult to follow
Jul 14, 2022
Context: I'm using swift-nio-ssl through Swift GRPC. And found some improvement points along the way of their API.
see: grpc/grpc-swift#1459
API Calls that make use of this initializer
end up being hard to follow due to the complex throwing interface. Source Code
Clients of this API could receive a lot of errors for many reasons and they are not simple to streamline to clients. I don't know much about this at all, so I read through the code and made some notes on what the failure points are and the errors they return.
Throwing reason 1: PEM Files could fail to read throws self-explanatory error 🥰 ✅
Throwing reason 2: Verification Algorithms cause some error code that's not detailed but it could be better typed⚠️
Throwing reason 3: Configuring signing algorithms fail with some error that's not detailed but it could be better typed⚠️
Throwing Reason 4: Certificate chain setup failures that DO have typed self-explanatory errors 🥰✅
Throwing reason 5: private key configuration with self-explanatory error 🥰 ✅
Throwing Reason 6: AlpnProtocol setup Error throws some typed error 🥰 ✅
throws a typed error
BoringSSLError.failedToSetALPN(errorStack)
Throwing Reason 7: adding key log callback calls throws but it does not call any throwing functions.⚠️
This function doesn't appear to be throwing anything
It would be great if some error interface would be documented for developers to expect, wrap and upstream with the adecquate to callers.
Great work and thanks for your time!
The text was updated successfully, but these errors were encountered: