python · csabella · Jan 21, 2018 · Jan 22, 2018
@@ -158,7 +158,7 @@ The following function allows for standalone socket creation.  Starting from
 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
@@ -243,15 +243,19 @@ instead.
    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
 ^^^^^^^^^^^^^^^^
 
@@ -987,7 +991,8 @@ SSL Sockets
      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 SSL 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
@@ -1593,8 +1598,16 @@ to speed up repeated connections from the same clients.
       `SSL/TLS & Perfect Forward Secrecy <https://vincent.bernat.im/en/blog/2011-ssl-perfect-forward-secrecy>`_
          Vincent Bernat.
 
+.. attribute:: SSLContext.suppress_ragged_eofs
+
+   This flag has the same meaning as in the top-level :func:`wrap_socket`
+   function.  It affects the :class:`SSLSocket` objects returned by
+   the context's :meth:`~SSLContext.wrap_socket` method.
+
+   .. versionadded:: 3.7
+
 .. 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=None, \
       server_hostname=None, session=None)
 
    Wrap an existing Python socket *sock* and return an instance of
@@ -1603,9 +1616,10 @@ to speed up repeated connections from the same clients.
    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.
+   certificates.  The parameters *server_side* and *do_handshake_on_connect*
+   have the same meaning as in the top-level :func:`wrap_socket` function.
+   The parameter *suppress_ragged_eofs* overrides the context's
+   :attr:`suppress_ragged_eofs` setting.
 
    On client connections, the optional parameter *server_hostname* specifies
    the hostname of the service which we are connecting to.  This allows a
@@ -1626,6 +1640,10 @@ to speed up repeated connections from the same clients.
       The method returns on instance of :attr:`SSLContext.sslsocket_class`
       instead of hard-coded :class:`SSLSocket`.
 
+   .. versionchanged:: 3.7
+      *suppress_ragged_eofs* now defaults to the context's setting.
+
+
 .. attribute:: SSLContext.sslsocket_class
 
    The return type of :meth:`SSLContext.wrap_sockets`, defaults to
@@ -2038,8 +2056,8 @@ method to create a server-side SSL socket for the connection::
        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

@@ -400,11 +400,14 @@ def __new__(cls, protocol=PROTOCOL_TLS, *args, **kwargs):
 
     def __init__(self, protocol=PROTOCOL_TLS):
         self.protocol = protocol
+        self.suppress_ragged_eofs = False
 
     def wrap_socket(self, sock, server_side=False,
                     do_handshake_on_connect=True,
-                    suppress_ragged_eofs=True,
+                    suppress_ragged_eofs=None,
                     server_hostname=None, session=None):
+        if suppress_ragged_eofs is None:
+            suppress_ragged_eofs = self.suppress_ragged_eofs
         return self.sslsocket_class(
             sock=sock,
             server_side=server_side,
@@ -737,7 +740,7 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
                  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):
 
@@ -1156,7 +1159,7 @@ def wrap_socket(sock, keyfile=None, certfile=None,
                 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,

@@ -41,6 +41,23 @@ def get_request(self):
             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

@@ -635,7 +635,11 @@ def handle(self):
                 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

@@ -1893,6 +1893,26 @@ def test_bio_read_write_data(self):
         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)
+
+    def test_context_ragged_eof(self):
+        s = socket.create_connection(self.server_addr)
+        self.addCleanup(s.close)
+        ctx = ssl.SSLContext(ssl.PROTOCOL_TLS)
+        self.assertFalse(ctx.suppress_ragged_eofs)
+        ctx.suppress_ragged_eofs = True
+        self.assertTrue(ctx.suppress_ragged_eofs)
+        s = ctx.wrap_socket(s)
+        self.addCleanup(s.close)
+        s.send(b'shutdown')
+        self.assertEqual(s.recv(300), b'')
+
 
 class NetworkedTests(unittest.TestCase):
 
@@ -2028,26 +2048,27 @@ def run(self):
                     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:
@@ -2169,9 +2190,17 @@ class EchoServer (asyncore.dispatcher):
         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()
@@ -3200,6 +3229,8 @@ def serve():
             evt.set()
             remote, peer = server.accept()
             remote.recv(1)
+            unwrapped = remote.unwrap()
+            unwrapped.close()
 
         t = threading.Thread(target=serve)
         t.start()
@@ -3208,9 +3239,9 @@ def serve():
         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)

diff --git a/Misc/NEWS.d/next/Library/2018-01-21-18-44-09.bpo-27815.nF1zEi.rst b/Misc/NEWS.d/next/Library/2018-01-21-18-44-09.bpo-27815.nF1zEi.rst
@@ -0,0 +1,9 @@
+SSL
+
+The *suppress_ragged_eofs* setting is now disabled by default. This means
+that by default a TCP shutdown, which may not be secure, raises
+:exc:`~ssl.SSLEOFError`, and applications can distinguish a secure SSL
+shutdown from a truncation attack.  There is a new
+:attr:`SSLContext.suppress_ragged_eofs
+<ssl.SSLContext.suppress_ragged_eofs>` attribute, which can be used to re-
+enable the setting via a context object.  Patch by Martin Panter.