From 3e86ba4e321d20931648d110e1be12643cb8ff04 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 28 Dec 2013 17:26:33 +0100 Subject: [PATCH] Issue #19422: Explicitly disallow non-SOCK_STREAM sockets in the ssl module, rather than silently let them emit clear text data. --- Doc/library/ssl.rst | 22 ++++++++++++++-------- Lib/ssl.py | 5 +++++ Lib/test/test_ssl.py | 12 ++++++++++++ Misc/NEWS | 3 +++ 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst index b13861d4ce9..ebc9a4eab88 100644 --- a/Doc/library/ssl.rst +++ b/Doc/library/ssl.rst @@ -141,13 +141,16 @@ instead. Takes an instance ``sock`` of :class:`socket.socket`, and returns an instance of :class:`ssl.SSLSocket`, a subtype of :class:`socket.socket`, which wraps - the underlying socket in an SSL context. For client-side sockets, the - context construction is lazy; if the underlying socket isn't connected yet, - the context construction will be performed after :meth:`connect` is called on - the socket. For server-side sockets, if the socket has no remote peer, it is - assumed to be a listening socket, and the server-side SSL wrapping is - automatically performed on client connections accepted via the :meth:`accept` - method. :func:`wrap_socket` may raise :exc:`SSLError`. + the underlying socket in an SSL context. ``sock`` must be a + :data:`~socket.SOCK_STREAM` socket; other socket types are unsupported. + + For client-side sockets, the context construction is lazy; if the + underlying socket isn't connected yet, the context construction will be + performed after :meth:`connect` is called on the socket. For + server-side sockets, if the socket has no remote peer, it is assumed + to be a listening socket, and the server-side SSL wrapping is + automatically performed on client connections accepted via the + :meth:`accept` method. :func:`wrap_socket` may raise :exc:`SSLError`. The ``keyfile`` and ``certfile`` parameters specify optional files which contain a certificate to be used to identify the local side of the @@ -836,7 +839,10 @@ to speed up repeated connections from the same clients. server_hostname=None) Wrap an existing Python socket *sock* and return an :class:`SSLSocket` - object. The SSL socket is tied to the context, its settings and + object. *sock* must be a :data:`~socket.SOCK_STREAM` socket; other socket + types are unsupported. + + The returned SSL socket is tied to the context, its settings and certificates. The parameters *server_side*, *do_handshake_on_connect* and *suppress_ragged_eofs* have the same meaning as in the top-level :func:`wrap_socket` function. diff --git a/Lib/ssl.py b/Lib/ssl.py index 06437b3046b..cd8d6b4c9ee 100644 --- a/Lib/ssl.py +++ b/Lib/ssl.py @@ -111,6 +111,7 @@ else: from socket import getnameinfo as _getnameinfo from socket import error as socket_error from socket import socket, AF_INET, SOCK_STREAM, create_connection +from socket import SOL_SOCKET, SO_TYPE import base64 # for DER-to-PEM translation import traceback import errno @@ -296,6 +297,10 @@ class SSLSocket(socket): 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: + raise NotImplementedError("only stream sockets are supported") if server_side and server_hostname: raise ValueError("server_hostname can only be specified " "in client mode") diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py index f3b5695a1cd..104a1edc6c9 100644 --- a/Lib/test/test_ssl.py +++ b/Lib/test/test_ssl.py @@ -493,6 +493,18 @@ class BasicSocketTests(unittest.TestCase): support.gc_collect() self.assertIn(r, str(cm.warning.args[0])) + def test_unsupported_dtls(self): + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + self.addCleanup(s.close) + with self.assertRaises(NotImplementedError) as cx: + ssl.wrap_socket(s, cert_reqs=ssl.CERT_NONE) + self.assertEqual(str(cx.exception), "only stream sockets are supported") + ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + with self.assertRaises(NotImplementedError) as cx: + ctx.wrap_socket(s) + self.assertEqual(str(cx.exception), "only stream sockets are supported") + + class ContextTests(unittest.TestCase): @skip_if_broken_ubuntu_ssl diff --git a/Misc/NEWS b/Misc/NEWS index bc0e0b41948..b7924cb4bb1 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -29,6 +29,9 @@ Core and Builtins Library ------- +- Issue #19422: Explicitly disallow non-SOCK_STREAM sockets in the ssl + module, rather than silently let them emit clear text data. + - Issue #18116: getpass was always getting an error when testing /dev/tty, and thus was always falling back to stdin, and would then raise an exception if stdin could not be used (such as /dev/null). It also leaked an open file.