[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

New submission from Christian Heimes <lists at cheimes.de>:

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

----------
assignee: christian.heimes
components: SSL
messages: 312823
nosy: alex, christian.heimes, dstufft, janssen, pitrou
priority: normal
severity: normal
stage: needs patch
status: open
title: Prohibit direct instantiation of SSLSocket and SSLObject
type: behavior
versions: Python 3.7, Python 3.8

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue32951>
_______________________________________