search

apple / swift-nio-ssl

TLS Support for SwiftNIO, based on BoringSSL.
https://swiftpackageindex.com/apple/swift-nio-ssl/main/documentation/niossl
Apache License 2.0
385 stars 139 forks source link

Implement SSL_CTX_set_cert_cb #456

Open AndrewBarba opened 6 months ago

AndrewBarba commented 6 months ago

This is a first pass at reviving https://github.com/apple/swift-nio-ssl/pull/311 in order to fix https://github.com/apple/swift-nio-ssl/issues/310

Credit to @TechnikEmpire, I did my best to address the comments in the original PR from @Lukasa

If the approach looks okay I'll move to testing

AndrewBarba commented 6 months ago

@Lukasa Thanks so much for the comments earlier. I've since changed quite a bit now that I have a better understanding of whats going on. I was able to create a test that at least ensures the handshake is completing successfully, where as before I was never signaling to the connection that our async lookup was complete. Im sure there's still a lot more testing and error handling we can do but I wanted to pause here and wait for more feedback.

I also want to call out that I'm concerned about this API returning a new NIOSSLContext given how expensive it is to create one. The main purpose of this API would be to serve many dynamic certificates, and at scale if we're constantly creating new contexts could this be a big problem? I'm wondering if a better API is return a new TLSConfiguration struct, and then apply the relevant parts of the configuration instead of swapping the entire context.

AndrewBarba commented 6 months ago

@Lukasa Sorry for the delay, the PR is cleaned up to remove that lookup state and keep everything inside the callback and new context manager

AndrewBarba commented 5 months ago

@Lukasa Just wanted to checkin and see if you had any other feedback. Sorry if this isn't heading in the right direction, don't want to take up too much of your time on it.

Lukasa commented 4 months ago

Sorry for the delay @AndrewBarba, it's not a reflection on your work: I was just on vacation! 🏝️

AndrewBarba commented 4 months ago

No problem @Lukasa hope you enjoyed! I addressed your comments, ready for another round of review.

AndrewBarba commented 1 month ago

@Lukasa Hope you had a great WWDC week, I just pushed up first go at a new extension change object. Let me know what you think

AndrewBarba commented 1 week ago

@Lukasa Friendly nudge to keep this moving. Let me know what I can tackle next

Lukasa commented 4 days ago

@swift-server-bot add to allowlist