bpo-32951: Disable SSLSocket/SSLObject constructor (#5864)

Direct instantiation of SSLSocket and SSLObject objects is now prohibited.
The constructors were never documented, tested, or designed as public
constructors. The SSLSocket constructor had limitations. For example it was
not possible to enabled hostname verification except was
ssl_version=PROTOCOL_TLS_CLIENT with cert_reqs=CERT_REQUIRED.

SSLContext.wrap_socket() and SSLContext.wrap_bio are the recommended API
to construct SSLSocket and SSLObject instances. ssl.wrap_socket() is
also deprecated.

The only test case for direct instantiation was added a couple of days
ago for IDNA testing.

Signed-off-by: Christian Heimes <christian@python.org>
This commit is contained in:
Christian Heimes 2018-02-27 10:17:30 +01:00 committed by GitHub
parent 90f05a527c
commit 9d50ab563d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 82 deletions

View File

@ -998,7 +998,7 @@ SSL Sockets
the specification of normal, OS-level sockets. See especially the the specification of normal, OS-level sockets. See especially the
:ref:`notes on non-blocking sockets <ssl-nonblocking>`. :ref:`notes on non-blocking sockets <ssl-nonblocking>`.
:class:`SSLSocket` are not created directly, but using the Instances of :class:`SSLSocket` must be created using the
:meth:`SSLContext.wrap_socket` method. :meth:`SSLContext.wrap_socket` method.
.. versionchanged:: 3.5 .. versionchanged:: 3.5
@ -1013,6 +1013,11 @@ SSL Sockets
It is deprecated to create a :class:`SSLSocket` instance directly, use It is deprecated to create a :class:`SSLSocket` instance directly, use
:meth:`SSLContext.wrap_socket` to wrap a socket. :meth:`SSLContext.wrap_socket` to wrap a socket.
.. versionchanged:: 3.7
:class:`SSLSocket` instances must to created with
:meth:`~SSLContext.wrap_socket`. In earlier versions, it was possible
to create instances directly. This was never documented or officially
supported.
SSL sockets also have the following additional methods and attributes: SSL sockets also have the following additional methods and attributes:
@ -2249,11 +2254,12 @@ provided.
but does not provide any network IO itself. IO needs to be performed through but does not provide any network IO itself. IO needs to be performed through
separate "BIO" objects which are OpenSSL's IO abstraction layer. separate "BIO" objects which are OpenSSL's IO abstraction layer.
An :class:`SSLObject` instance can be created using the This class has no public constructor. An :class:`SSLObject` instance
:meth:`~SSLContext.wrap_bio` method. This method will create the must be created using the :meth:`~SSLContext.wrap_bio` method. This
:class:`SSLObject` instance and bind it to a pair of BIOs. The *incoming* method will create the :class:`SSLObject` instance and bind it to a
BIO is used to pass data from Python to the SSL protocol instance, while the pair of BIOs. The *incoming* BIO is used to pass data from Python to the
*outgoing* BIO is used to pass data the other way around. SSL protocol instance, while the *outgoing* BIO is used to pass data the
other way around.
The following methods are available: The following methods are available:
@ -2305,6 +2311,12 @@ provided.
:meth:`~SSLContext.wrap_socket`. An :class:`SSLObject` is always created :meth:`~SSLContext.wrap_socket`. An :class:`SSLObject` is always created
via an :class:`SSLContext`. via an :class:`SSLContext`.
.. versionchanged:: 3.7
:class:`SSLObject` instances must to created with
:meth:`~SSLContext.wrap_bio`. In earlier versions, it was possible to
create instances directly. This was never documented or officially
supported.
An SSLObject communicates with the outside world using memory buffers. The An SSLObject communicates with the outside world using memory buffers. The
class :class:`MemoryBIO` provides a memory buffer that can be used for this class :class:`MemoryBIO` provides a memory buffer that can be used for this
purpose. It wraps an OpenSSL memory BIO (Basic IO) object: purpose. It wraps an OpenSSL memory BIO (Basic IO) object:

View File

@ -677,6 +677,12 @@ OpenSSL 1.1.1. (Contributed by Christian Heimes in :issue:`32947`,
recommend :meth:`~ssl.SSLContext.wrap_socket` instead. recommend :meth:`~ssl.SSLContext.wrap_socket` instead.
(Contributed by Christian Heimes in :issue:`28124`.) (Contributed by Christian Heimes in :issue:`28124`.)
:class:`~ssl.SSLSocket` and :class:`~ssl.SSLObject` no longer have a public
constructor. Direct instantiation was never a documented and supported
feature. Instances must be created with :class:`~ssl.SSLContext` methods
:meth:`~ssl.SSLContext.wrap_socket` and :meth:`~ssl.SSLContext.wrap_bio`.
(Contributed by Christian Heimes in :issue:`32951`)
string string
------ ------

View File

@ -390,24 +390,24 @@ class SSLContext(_SSLContext):
server_hostname=None, session=None): server_hostname=None, session=None):
# SSLSocket class handles server_hostname encoding before it calls # SSLSocket class handles server_hostname encoding before it calls
# ctx._wrap_socket() # ctx._wrap_socket()
return self.sslsocket_class( return self.sslsocket_class._create(
sock=sock, sock=sock,
server_side=server_side, server_side=server_side,
do_handshake_on_connect=do_handshake_on_connect, do_handshake_on_connect=do_handshake_on_connect,
suppress_ragged_eofs=suppress_ragged_eofs, suppress_ragged_eofs=suppress_ragged_eofs,
server_hostname=server_hostname, server_hostname=server_hostname,
_context=self, context=self,
_session=session session=session
) )
def wrap_bio(self, incoming, outgoing, server_side=False, def wrap_bio(self, incoming, outgoing, server_side=False,
server_hostname=None, session=None): server_hostname=None, session=None):
# Need to encode server_hostname here because _wrap_bio() can only # Need to encode server_hostname here because _wrap_bio() can only
# handle ASCII str. # handle ASCII str.
return self.sslobject_class( return self.sslobject_class._create(
incoming, outgoing, server_side=server_side, incoming, outgoing, server_side=server_side,
server_hostname=self._encode_hostname(server_hostname), server_hostname=self._encode_hostname(server_hostname),
session=session, _context=self, session=session, context=self,
) )
def set_npn_protocols(self, npn_protocols): def set_npn_protocols(self, npn_protocols):
@ -612,14 +612,23 @@ class SSLObject:
* Any form of network IO incluging methods such as ``recv`` and ``send``. * Any form of network IO incluging methods such as ``recv`` and ``send``.
* The ``do_handshake_on_connect`` and ``suppress_ragged_eofs`` machinery. * The ``do_handshake_on_connect`` and ``suppress_ragged_eofs`` machinery.
""" """
def __init__(self, *args, **kwargs):
raise TypeError(
f"{self.__class__.__name__} does not have a public "
f"constructor. Instances are returned by SSLContext.wrap_bio()."
)
def __init__(self, incoming, outgoing, server_side=False, @classmethod
server_hostname=None, session=None, _context=None): def _create(cls, incoming, outgoing, server_side=False,
self._sslobj = _context._wrap_bio( server_hostname=None, session=None, context=None):
self = cls.__new__(cls)
sslobj = context._wrap_bio(
incoming, outgoing, server_side=server_side, incoming, outgoing, server_side=server_side,
server_hostname=server_hostname, server_hostname=server_hostname,
owner=self, session=session owner=self, session=session
) )
self._sslobj = sslobj
return self
@property @property
def context(self): def context(self):
@ -741,72 +750,48 @@ class SSLObject:
class SSLSocket(socket): class SSLSocket(socket):
"""This class implements a subtype of socket.socket that wraps """This class implements a subtype of socket.socket that wraps
the underlying OS socket in an SSL context when necessary, and the underlying OS socket in an SSL context when necessary, and
provides read and write methods over that channel.""" provides read and write methods over that channel. """
def __init__(self, sock=None, keyfile=None, certfile=None, def __init__(self, *args, **kwargs):
server_side=False, cert_reqs=CERT_NONE, raise TypeError(
ssl_version=PROTOCOL_TLS, ca_certs=None, f"{self.__class__.__name__} does not have a public "
do_handshake_on_connect=True, f"constructor. Instances are returned by "
family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None, f"SSLContext.wrap_socket()."
suppress_ragged_eofs=True, npn_protocols=None, ciphers=None, )
server_hostname=None,
_context=None, _session=None):
if _context: @classmethod
self._context = _context def _create(cls, sock, server_side=False, do_handshake_on_connect=True,
else: suppress_ragged_eofs=True, server_hostname=None,
if server_side and not certfile: context=None, session=None):
raise ValueError("certfile must be specified for server-side "
"operations")
if keyfile and not certfile:
raise ValueError("certfile must be specified")
if certfile and not keyfile:
keyfile = certfile
self._context = SSLContext(ssl_version)
self._context.verify_mode = cert_reqs
if ca_certs:
self._context.load_verify_locations(ca_certs)
if certfile:
self._context.load_cert_chain(certfile, keyfile)
if npn_protocols:
self._context.set_npn_protocols(npn_protocols)
if ciphers:
self._context.set_ciphers(ciphers)
self.keyfile = keyfile
self.certfile = certfile
self.cert_reqs = cert_reqs
self.ssl_version = ssl_version
self.ca_certs = ca_certs
self.ciphers = ciphers
# Can't use sock.type as other flags (such as SOCK_NONBLOCK) get
# mixed in.
if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM: if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM:
raise NotImplementedError("only stream sockets are supported") raise NotImplementedError("only stream sockets are supported")
if server_side: if server_side:
if server_hostname: if server_hostname:
raise ValueError("server_hostname can only be specified " raise ValueError("server_hostname can only be specified "
"in client mode") "in client mode")
if _session is not None: if session is not None:
raise ValueError("session can only be specified in " raise ValueError("session can only be specified in "
"client mode") "client mode")
if self._context.check_hostname and not server_hostname: if context.check_hostname and not server_hostname:
raise ValueError("check_hostname requires server_hostname") raise ValueError("check_hostname requires server_hostname")
self._session = _session
kwargs = dict(
family=sock.family, type=sock.type, proto=sock.proto,
fileno=sock.fileno()
)
self = cls.__new__(cls, **kwargs)
super(SSLSocket, self).__init__(**kwargs)
self.settimeout(sock.gettimeout())
sock.detach()
self._context = context
self._session = session
self._closed = False
self._sslobj = None
self.server_side = server_side self.server_side = server_side
self.server_hostname = self._context._encode_hostname(server_hostname) self.server_hostname = context._encode_hostname(server_hostname)
self.do_handshake_on_connect = do_handshake_on_connect self.do_handshake_on_connect = do_handshake_on_connect
self.suppress_ragged_eofs = suppress_ragged_eofs self.suppress_ragged_eofs = suppress_ragged_eofs
if sock is not None:
super().__init__(family=sock.family,
type=sock.type,
proto=sock.proto,
fileno=sock.fileno())
self.settimeout(sock.gettimeout())
sock.detach()
elif fileno is not None:
super().__init__(fileno=fileno)
else:
super().__init__(family=family, type=type, proto=proto)
# See if we are connected # See if we are connected
try: try:
@ -818,8 +803,6 @@ class SSLSocket(socket):
else: else:
connected = True connected = True
self._closed = False
self._sslobj = None
self._connected = connected self._connected = connected
if connected: if connected:
# create the SSL object # create the SSL object
@ -834,10 +817,10 @@ class SSLSocket(socket):
# non-blocking # non-blocking
raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets") raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets")
self.do_handshake() self.do_handshake()
except (OSError, ValueError): except (OSError, ValueError):
self.close() self.close()
raise raise
return self
@property @property
def context(self): def context(self):
@ -1184,12 +1167,25 @@ def wrap_socket(sock, keyfile=None, certfile=None,
do_handshake_on_connect=True, do_handshake_on_connect=True,
suppress_ragged_eofs=True, suppress_ragged_eofs=True,
ciphers=None): ciphers=None):
return SSLSocket(sock=sock, keyfile=keyfile, certfile=certfile,
server_side=server_side, cert_reqs=cert_reqs, if server_side and not certfile:
ssl_version=ssl_version, ca_certs=ca_certs, raise ValueError("certfile must be specified for server-side "
do_handshake_on_connect=do_handshake_on_connect, "operations")
suppress_ragged_eofs=suppress_ragged_eofs, if keyfile and not certfile:
ciphers=ciphers) raise ValueError("certfile must be specified")
context = SSLContext(ssl_version)
context.verify_mode = cert_reqs
if ca_certs:
context.load_verify_locations(ca_certs)
if certfile:
context.load_cert_chain(certfile, keyfile)
if ciphers:
context.set_ciphers(ciphers)
return context.wrap_socket(
sock=sock, server_side=server_side,
do_handshake_on_connect=do_handshake_on_connect,
suppress_ragged_eofs=suppress_ragged_eofs
)
# some utility functions # some utility functions

View File

@ -263,6 +263,11 @@ class BasicSocketTests(unittest.TestCase):
ssl.OP_NO_TLSv1_2 ssl.OP_NO_TLSv1_2
self.assertEqual(ssl.PROTOCOL_TLS, ssl.PROTOCOL_SSLv23) self.assertEqual(ssl.PROTOCOL_TLS, ssl.PROTOCOL_SSLv23)
def test_private_init(self):
with self.assertRaisesRegex(TypeError, "public constructor"):
with socket.socket() as s:
ssl.SSLSocket(s)
def test_str_for_enums(self): def test_str_for_enums(self):
# Make sure that the PROTOCOL_* constants have enum-like string # Make sure that the PROTOCOL_* constants have enum-like string
# reprs. # reprs.
@ -1657,6 +1662,13 @@ class MemoryBIOTests(unittest.TestCase):
self.assertRaises(TypeError, bio.write, 1) self.assertRaises(TypeError, bio.write, 1)
class SSLObjectTests(unittest.TestCase):
def test_private_init(self):
bio = ssl.MemoryBIO()
with self.assertRaisesRegex(TypeError, "public constructor"):
ssl.SSLObject(bio, bio)
class SimpleBackgroundTests(unittest.TestCase): class SimpleBackgroundTests(unittest.TestCase):
"""Tests that connect to a simple server running in the background""" """Tests that connect to a simple server running in the background"""
@ -2735,12 +2747,6 @@ class ThreadedTests(unittest.TestCase):
self.assertEqual(s.server_hostname, expected_hostname) self.assertEqual(s.server_hostname, expected_hostname)
self.assertTrue(cert, "Can't get peer certificate.") self.assertTrue(cert, "Can't get peer certificate.")
with ssl.SSLSocket(socket.socket(),
server_hostname=server_hostname) as s:
s.connect((HOST, server.port))
s.getpeercert()
self.assertEqual(s.server_hostname, expected_hostname)
# incorrect hostname should raise an exception # incorrect hostname should raise an exception
server = ThreadedEchoServer(context=server_context, chatty=True) server = ThreadedEchoServer(context=server_context, chatty=True)
with server: with server:
@ -3999,7 +4005,7 @@ def test_main(verbose=False):
tests = [ tests = [
ContextTests, BasicSocketTests, SSLErrorTests, MemoryBIOTests, ContextTests, BasicSocketTests, SSLErrorTests, MemoryBIOTests,
SimpleBackgroundTests, ThreadedTests, SSLObjectTests, SimpleBackgroundTests, ThreadedTests,
] ]
if support.is_resource_enabled('network'): if support.is_resource_enabled('network'):

View File

@ -0,0 +1,3 @@
Direct instantiation of SSLSocket and SSLObject objects is now prohibited.
The constructors were never documented, tested, or designed as public
constructors. Users were suppose to use ssl.wrap_socket() or SSLContext.