Issue 32951: Prohibit direct instantiation of SSLSocket and SSLObject (original) (raw)

The constructors of SSLObject and SSLSocket were never documented, tested, or meant to be used directly. Instead users were suppose to use ssl.wrap_socket or an SSLContext object. The ssl.wrap_socket() function and direct instantiation of SSLSocket has multiple issues. From my mail "No hostname matching with ssl.wrap_socket() and SSLSocket() constructor" to PSRT:

The ssl module has three ways to create a SSLSocket object:

  1. ssl.wrap_socket() [1]
  2. ssl.SSLSocket() can be instantiated directly without a context [2]
  3. SSLContext.wrap_socket() [3]

Variant (1) and (2) are old APIs with insecure default settings.

Variant (3) is the new and preferred way. With ssl.create_default_context() or ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) the socket is configured securely with hostname matching and cert validation enabled.

While Martin Panter was reviewing my documentation improvements for the ssl module, he pointed out an issue, https://github.com/python/cpython/pull/3530#discussion_r170407478 . ssl.wrap_socket() and ssl.SSLSocket() default to CERT_NONE but PROTOCOL_TLS_CLIENT is documented to set CERT_REQUIRED. After a closer look, it turned out that the code is robust and refuses to accept PROTOCOL_TLS_CLIENT + default values with "Cannot set verify_mode to CERT_NONE when check_hostname is enabled.". I consider the behavior a feature.

However ssl.SSLSocket() constructor and ssl.wrap_socket() have more fundamental security issues. I haven't looked at the old legacy APIs in a while and only concentrated on SSLContext. To my surprise both APIs do NOT perform or allow hostname matching. The wrap_socket() function does not even take a server_hostname argument, so it doesn't send a SNI TLS extension either. These bad default settings can lead to suprising security bugs in 3rd party code. This example doesn't fail although the hostname doesn't match the certificate:

import socket import ssl

cafile = ssl.get_default_verify_paths().cafile

with socket.socket() as sock: ssock = ssl.SSLSocket( sock, cert_reqs=ssl.CERT_REQUIRED, ca_certs=cafile, server_hostname='www.python.org' ) ssock.connect(('www.evil.com', 443))

I don't see a way to fix the issue in a secure way while keeping backwards compatibility. We could either modify the default behavior of ssl.wrap_socket() and SSLSocket() constructor, or drop both features completely. Either way it's going to break software that uses them. Since I like to get rid of variants (1) and (2), I would favor to remove them in favor of SSLContext.wrap_socket(). At least we should implement my documentation bug 28124 [4] and make ssl.wrap_socket() less prominent. I'd appreciate any assistance.

By the way, SSLObject is sane because it always goes through SSLContext.wrap_bio(). Thanks Benjamin!

Regards, Christian

[1] https://docs.python.org/3/library/ssl.html#ssl.wrap_socket [2] https://docs.python.org/3/library/ssl.html#ssl.SSLSocket [3] https://docs.python.org/3/library/ssl.html#ssl.SSLContext.wrap_socket [4] https://bugs.python.org/issue28124

Antoine Pitrou replied:

The ssl.SSLSocket constructor was never meant to be called by user code directly (and I don't think we document it as such). Anyone doing this is asking for trouble (including compatibility breakage as we change the constructor signature).

ssl.wrap_socket() is essentially a legacy API.

I would suggest the following measures :

Alex Gaynor replied:

If SSLSocket.init is meant to be private and not called by users, perhaps we could clean up the API and remove all the legacy arguments it takes, bring it down to just taking a context?

It'd break anyone who was relying on it, but they weren't supposed to be relying on it in the first place... (Is it documented even?)

I have implemented Antoine's second proposal in #28124.