[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

Change by Christian Heimes :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Can this issue be closed as resolved?

--
nosy: +cheryl.sabella

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

Christian Heimes  added the comment:


New changeset 89c2051a554d2053ac87b0adbf11ed0f1bb65db3 by Christian Heimes in 
branch '3.7':
[3.7] bpo-32951: Disable SSLSocket/SSLObject constructor (GH-5864) (#5925)
https://github.com/python/cpython/commit/89c2051a554d2053ac87b0adbf11ed0f1bb65db3


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

Change by Christian Heimes :


--
pull_requests: +5696

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

Change by Christian Heimes :


--
keywords: +patch
pull_requests: +5662
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

Christian Heimes  added the comment:

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 :

- Deprecate ssl.wrap_socket() and slate it to be removed around 3.8 or
3.9.  SSLContext is now is any recent 2.7 or 3.x version, so the
compatibility argument doesn't hold anymore.

- Severely de-emphasize ssl.wrap_socket() in the documentation (relegate
it in a "legacy API" section at the end), and put a warning that it's
insecure.


---
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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32951] Prohibit direct instantiation of SSLSocket and SSLObject

[Christian Heimes]

New submission from Christian Heimes :

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com