# HG changeset patch # Parent e5888f5b9cf899f05d4f37983db76b9de6473368 Issue #27815: Make the SSL suppress_ragged_eofs parameter False by default diff -r e5888f5b9cf8 Doc/library/ssl.rst --- a/Doc/library/ssl.rst Wed Sep 21 19:35:54 2016 +0300 +++ b/Doc/library/ssl.rst Thu Sep 22 08:59:13 2016 +0000 @@ -143,7 +143,7 @@ Python 3.2, it can be more flexible to use :meth:`SSLContext.wrap_socket` instead. -.. function:: wrap_socket(sock, keyfile=None, certfile=None, server_side=False, cert_reqs=CERT_NONE, ssl_version={see docs}, ca_certs=None, do_handshake_on_connect=True, suppress_ragged_eofs=True, ciphers=None) +.. function:: wrap_socket(sock, keyfile=None, certfile=None, server_side=False, cert_reqs=CERT_NONE, ssl_version={see docs}, ca_certs=None, do_handshake_on_connect=True, suppress_ragged_eofs=False, ciphers=None) Takes an instance ``sock`` of :class:`socket.socket`, and returns an instance of :class:`ssl.SSLSocket`, a subtype of :class:`socket.socket`, which wraps @@ -225,15 +225,18 @@ blocking behavior of the socket I/O involved in the handshake. The parameter ``suppress_ragged_eofs`` specifies how the - :meth:`SSLSocket.recv` method should signal unexpected EOF from the other end - of the connection. If specified as :const:`True` (the default), it returns a + :meth:`SSLSocket.recv` method should handle the connection being shut + down outside the protocol. If specified as :const:`True`, it returns a normal EOF (an empty bytes object) in response to unexpected EOF errors - raised from the underlying socket; if :const:`False`, it will raise the - exceptions back to the caller. + raised from the underlying socket; if :const:`False`, it will raise + :exc:`SSLEOFError` back to the caller. .. versionchanged:: 3.2 New optional argument *ciphers*. + .. versionchanged:: 3.7 + *suppress_ragged_eofs* now defaults to :const:`False`. + Context creation ^^^^^^^^^^^^^^^^ @@ -940,7 +943,8 @@ the same limitation) - :meth:`~socket.socket.sendfile()` (but :mod:`os.sendfile` will be used for plain-text sockets only, else :meth:`~socket.socket.send()` will be used) - - :meth:`~socket.socket.shutdown()` + - :meth:`~socket.socket.shutdown()` (bypasses the SSL layer and + shuts down the OS-level socket) However, since the SSL (and TLS) protocol has its own framing atop of TCP, the SSL sockets abstraction can, in certain respects, diverge from @@ -1545,7 +1549,7 @@ Vincent Bernat. .. method:: SSLContext.wrap_socket(sock, server_side=False, \ - do_handshake_on_connect=True, suppress_ragged_eofs=True, \ + do_handshake_on_connect=True, suppress_ragged_eofs=False, \ server_hostname=None, session=None) Wrap an existing Python socket *sock* and return an :class:`SSLSocket` @@ -1572,6 +1576,9 @@ .. versionchanged:: 3.6 *session* argument was added. + .. versionchanged:: 3.7 + *suppress_ragged_eofs* now defaults to :const:`False`. + .. method:: SSLContext.wrap_bio(incoming, outgoing, server_side=False, \ server_hostname=None, session=None) @@ -1954,8 +1961,8 @@ connstream = context.wrap_socket(newsocket, server_side=True) try: deal_with_client(connstream) + connstream = connstream.unwrap() finally: - connstream.shutdown(socket.SHUT_RDWR) connstream.close() Then you'll read data from the ``connstream`` and do something with it till you diff -r e5888f5b9cf8 Lib/ssl.py --- a/Lib/ssl.py Wed Sep 21 19:35:54 2016 +0300 +++ b/Lib/ssl.py Thu Sep 22 08:59:13 2016 +0000 @@ -392,7 +392,7 @@ def wrap_socket(self, sock, server_side=False, do_handshake_on_connect=True, - suppress_ragged_eofs=True, + suppress_ragged_eofs=False, server_hostname=None, session=None): return SSLSocket(sock=sock, server_side=server_side, do_handshake_on_connect=do_handshake_on_connect, @@ -719,7 +719,7 @@ ssl_version=PROTOCOL_TLS, ca_certs=None, do_handshake_on_connect=True, family=AF_INET, type=SOCK_STREAM, proto=0, fileno=None, - suppress_ragged_eofs=True, npn_protocols=None, ciphers=None, + suppress_ragged_eofs=False, npn_protocols=None, ciphers=None, server_hostname=None, _context=None, _session=None): @@ -1132,7 +1132,7 @@ server_side=False, cert_reqs=CERT_NONE, ssl_version=PROTOCOL_TLS, ca_certs=None, do_handshake_on_connect=True, - suppress_ragged_eofs=True, + suppress_ragged_eofs=False, ciphers=None): return SSLSocket(sock=sock, keyfile=keyfile, certfile=certfile, server_side=server_side, cert_reqs=cert_reqs, diff -r e5888f5b9cf8 Lib/test/ssl_servers.py --- a/Lib/test/ssl_servers.py Wed Sep 21 19:35:54 2016 +0300 +++ b/Lib/test/ssl_servers.py Thu Sep 22 08:59:13 2016 +0000 @@ -42,6 +42,22 @@ raise return sslconn, addr + def finish_request(self, request, client_address): + super().finish_request(request, client_address) + try: + request.unwrap() + except OSError: + # SSL_shutdown() can raise SSL_ERROR_SYSCALL without setting + # errno if the remote end closed the low-level connection (Issue + # 10808) + pass + + def handle_error(self, request, client_address): + [_, exc, _] = sys.exc_info() + # SSLEOFError is triggered when particular tests abort the request + if not isinstance(exc, ssl.SSLEOFError): + super().handle_error(request, client_address) + class RootedHTTPRequestHandler(SimpleHTTPRequestHandler): # need to override translate_path to get a known root, # instead of using os.curdir, since the test could be diff -r e5888f5b9cf8 Lib/test/test_imaplib.py --- a/Lib/test/test_imaplib.py Wed Sep 21 19:35:54 2016 +0300 +++ b/Lib/test/test_imaplib.py Thu Sep 22 08:59:13 2016 +0000 @@ -300,7 +300,11 @@ self.wfile.write(b'* OK') with self.reaped_server(EOFHandler) as server: - self.assertRaises(imaplib.IMAP4.abort, + if isinstance(server, SecureTCPServer): + exception = ssl.SSLEOFError + else: + exception = imaplib.IMAP4.abort + self.assertRaises(exception, self.imap_class, *server.server_address) @reap_threads diff -r e5888f5b9cf8 Lib/test/test_ssl.py --- a/Lib/test/test_ssl.py Wed Sep 21 19:35:54 2016 +0300 +++ b/Lib/test/test_ssl.py Thu Sep 22 08:59:13 2016 +0000 @@ -1768,6 +1768,14 @@ self.assertEqual(buf, b'foo\n') self.ssl_io_loop(sock, incoming, outgoing, sslobj.unwrap) + def test_insecure_shutdown(self): + s = socket.create_connection(self.server_addr) + self.addCleanup(s.close) + s = test_wrap_socket(s) + self.addCleanup(s.close) + s.send(b'shutdown') + self.assertRaises(ssl.SSLEOFError, s.recv, 300) + class NetworkedTests(unittest.TestCase): @@ -1925,26 +1933,27 @@ return while self.running: try: - msg = self.read() + try: + msg = self.read() + except ssl.SSLEOFError: + self.running = False + self.close() + continue stripped = msg.strip() if not stripped: # eof, so quit this handler self.running = False - try: - self.sock = self.sslconn.unwrap() - except OSError: - # Many tests shut the TCP connection down - # without an SSL shutdown. This causes - # unwrap() to raise OSError with errno=0! - pass - else: - self.sslconn = None + self.sock = self.sslconn.unwrap() self.close() elif stripped == b'over': if support.verbose and self.server.connectionchatty: sys.stdout.write(" server: client closed connection\n") self.close() return + elif stripped == b'shutdown': + self.sslconn.shutdown(socket.SHUT_RDWR) + self.close() + return elif (self.server.starttls_server and stripped == b'STARTTLS'): if support.verbose and self.server.connectionchatty: @@ -2066,9 +2075,17 @@ class ConnectionHandler (asyncore.dispatcher_with_send): def __init__(self, conn, certfile): + # The "asyncore" module already makes errors like + # ECONNRESET look like secure EOF signals, so there is + # not much benefit in setting suppress_ragged_eofs=False + # (and it would make the server behaviour more + # complicated due to the race between the server sending + # the last unread response and the client shutting the + # connection down) self.socket = test_wrap_socket(conn, server_side=True, certfile=certfile, - do_handshake_on_connect=False) + do_handshake_on_connect=False, + suppress_ragged_eofs=True) asyncore.dispatcher_with_send.__init__(self, self.socket) self._ssl_accepting = True self._do_ssl_handshake() @@ -3041,6 +3058,8 @@ evt.set() remote, peer = server.accept() remote.recv(1) + unwrapped = remote.unwrap() + unwrapped.close() t = threading.Thread(target=serve) t.start() @@ -3049,9 +3068,9 @@ client = context.wrap_socket(socket.socket()) client.connect((host, port)) client_addr = client.getsockname() + client = client.unwrap() client.close() t.join() - remote.close() server.close() # Sanity checks. self.assertIsInstance(remote, ssl.SSLSocket)