On 11 September 2016 at 05:20, Christian Heimes wrote:

On 2016-09-10 17:24, Nick Coghlan wrote:

...

On 11 September 2016 at 00:22, Christian Heimes wrote:

...

First I like to deprecated some old APIs and favor of SSLCotext. We have multiple ways to create a SSL socket or to configure libraries like urllib. The general idea is to make SSLContext the central object for TLS/SSL configuration. My patch deprecates ssl.wrap_socket()

I'll bring over my question from the tracker issue to here: there's a subset of ssl.wrap_socket() arguments which actually make sense as arguments to ssl.get_default_context().wrap_socket().

Accordingly, we can pick a subset of code (e.g. SSL/TLS clients) that we bless with not needing to change, leaving only code using deprecated parameters or creating server sockets that needs to be updated.

Do you consider ssl.wrap_socket() relevant for so many projects? The function hurts performance and is no longer best practice. The deprecation of ssl.wrap_socket() is a friendly nudge. I don't mind to keep it around for another four or six years.

I have no problem with ripping out and replacing the internals of ssl.wrap_socket(), and doing whatever is needed to improve its performance. What I'm mainly looking for is a decision tree in the overall API design that minimises the amount of fresh information a developer needs to supply, and that makes the purpose of their code relatively self-evident to someone that is reading low(ish) level Python SSL/TLS code for the first time. For example, I think this would be a desirably simple design from a usage perspective: # Client sockets as default, settings may change in maintenance releases my_context = ssl.get_default_context() my_tls_socket = ssl.wrap_socket(my_uncovered_socket) # Server sockets by request, settings may change in maintenance releases my_context = ssl.get_default_server_context() my_tls_socket = ssl.wrap_server_socket(my_uncovered_socket) # More control with more responsibility, defaults only change in feature releases my_context = ssl.SSLContext() my_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) With that approach, an API user only has to make two forced decisions: - am I securing a client connection or a server connection? - do I want to implicitly pick up modernised defaults in maintenance releases? And we can make the second one a non-decision in most cases by presenting the higher level convenience API as the preferred approach. There would be a third hidden decision implied by the convenience APIs (using the default system certificate store rather than loading a custom one), but most users wouldn't need to worry about that. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On 2016-09-10 18:24, Donald Stufft wrote:

...

On Sep 10, 2016, at 10:22 AM, Christian Heimes wrote:

I don't load any certs because it is not possible to remove a cert or X509 lookup once it is loaded. create_default_context() just have to load the certs and set more secure ciper suites.

This part is the most concerning to me, though I understand why it’s the case. Perhaps we can do something a little tricky to allow both things to happen? IOW do sort of a late binding of a call to loading the default certificates if no other certificates has been loaded when the call to SSLContext().wrap_socket() has been made.

So we’d do something like:

class SSLContext: def __init__(self, …): self._loaded_certificates = False … # Do Other Stuff

def load_default_certs(self, …): self._loaded_certificates = True … # Do Other Stuff

def load_verify_locations(self, …): self._loaded_certificates = True … # Do Other Stuff

def wrap_socket(self, …): if not self._loaded_certificates: self.load_default_certs()

… # Do Other Stuff

That way if someone does something like:

ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.load_verify_locations(cafile=“…”) ctx.wrap_socket(…)

Then they don’t get any default certificates added, HOWEVER if they do:

ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) ctx.wrap_socket(…)

Then they do.

The main draw back I can see with this is that you can’t wrap a socket and then add certificates after the fact… but I don’t even know if that makes sense to do?

It's a bit too clever and tricky for my taste. I prefer 'explicit is better than implicit' for trust anchors. My main concern are secure default settings. A SSLContext should be secure w/o further settings in order to prevent developers to shoot themselves in the knee. Missing root certs are not a direct security issue with CERT_REQUIRED. The connection will simply fail. I'd rather improve the error message than to auto-load certs. Christian