This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author christian.heimes
Recipients alex, christian.heimes, dstufft, janssen, pitrou
Date 2018-02-25.17:18:04
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1519579084.45.0.467229070634.issue32951@psf.upfronthosting.co.za>
In-reply-to
Content
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
History
Date User Action Args
2018-02-25 17:18:04christian.heimessetrecipients: + christian.heimes, janssen, pitrou, alex, dstufft
2018-02-25 17:18:04christian.heimessetmessageid: <1519579084.45.0.467229070634.issue32951@psf.upfronthosting.co.za>
2018-02-25 17:18:04christian.heimeslinkissue32951 messages
2018-02-25 17:18:04christian.heimescreate