Closed sethmlarson closed 1 year ago
I haven't done a close review yet, but I think this is moving in the direction of what most users of truststore probably want (secure by default, without needing to pay attention to specific call sites) and I'm happy to see it in progress.
I do have some concern because of the fact that the patching is dependent on import order (i.e. if a module imports SSLContext into its own namespace before the patch is injected, it won't take effect there). There are some devious ways to try to solve that which I wouldn't necessarily recommend (i.e. following references via gc.get_referrers
or replacing the builtin SSLContext's __new__
method). Another possibility that occurred to me would be to replace the openssl verify callback using ctypes similar to what Steve Dower showed in https://gist.github.com/zooba/bce89eaa865bf89611f7dab9a2a38276, instead of replacing all of SSLContext. But that would be a major refactoring and I haven't checked whether it would be compatible with what's needed for the Mac and Windows implementations.
@davisagli I'm interested in trying out swapping the verify callback, I'm assuming that would be essentially the next step for this library if we're planning on making it into the stdlib. Let's create a separate issue and investigate? I'd hope we'd be able to access the full chain at that point in the call, if so we can probably do what we're doing currently on macOS and Windows.
Okay this is ready for a review @davisagli. I'm going to open an issue to track down why using truststore.SSLContext as a server doesn't work on specifically macOS. I don't have one of those handy and testing via CI was getting tedious.
Closes https://github.com/sethmlarson/truststore/issues/75
cc @guerda @pfeigl @JonasR