From d748ca11f116c0192532dc149bf5fde216ef0e5f Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Thu, 21 Nov 2019 04:41:50 -0500 Subject: [PATCH 01/13] bpo-37228: Fix loop.create_datagram_endpoint()'s usage of SO_REUSEADDR --- Doc/library/asyncio-eventloop.rst | 16 +++++++++----- Lib/asyncio/base_events.py | 11 +++++----- Lib/test/test_asyncio/test_base_events.py | 26 +++++++++++------------ 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst index 240d814f13dea9..945235c8cc08cf 100644 --- a/Doc/library/asyncio-eventloop.rst +++ b/Doc/library/asyncio-eventloop.rst @@ -473,6 +473,13 @@ Opening network connections reuse_address=None, reuse_port=None, \ allow_broadcast=None, sock=None) + .. note:: + The parameter *reuse_address* is no longer supported, as using + :py:data:`~sockets.SO_REUSEADDR` poses a significant security concern for + UDP. Explicitly passing `reuse_address=True` will raise an exception. + For supported platforms, *reuse_port* can be used instead for similar + functionality. + Create a datagram connection. The socket family can be either :py:data:`~socket.AF_INET`, @@ -501,11 +508,6 @@ Opening network connections resolution. If given, these should all be integers from the corresponding :mod:`socket` module constants. - * *reuse_address* tells the kernel to reuse a local socket in - ``TIME_WAIT`` state, without waiting for its natural timeout to - expire. If not specified will automatically be set to ``True`` on - Unix. - * *reuse_port* tells the kernel to allow this endpoint to be bound to the same port as other existing endpoints are bound to, so long as they all set this flag when being created. This option is not supported on Windows @@ -527,6 +529,10 @@ Opening network connections The *family*, *proto*, *flags*, *reuse_address*, *reuse_port, *allow_broadcast*, and *sock* parameters were added. + .. versionchanged:: 3.5.10 + The *reuse_address* parameter is no longer supported due to security + concerns. + .. versionchanged:: 3.8 Added support for Windows. diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 031071281b38f7..581596c9b86542 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1306,8 +1306,12 @@ async def create_datagram_endpoint(self, protocol_factory, exceptions = [] - if reuse_address is None: - reuse_address = os.name == 'posix' and sys.platform != 'cygwin' + if reuse_address: + # bpo-37228 + raise RuntimeError("Passing `reuse_address=True` is no " + "longer supported, as the usage of " + "SO_REUSEPORT in UDP poses a significant " + "security concern.") for ((family, proto), (local_address, remote_address)) in addr_pairs_info: @@ -1316,9 +1320,6 @@ async def create_datagram_endpoint(self, protocol_factory, try: sock = socket.socket( family=family, type=socket.SOCK_DGRAM, proto=proto) - if reuse_address: - sock.setsockopt( - socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) if reuse_port: _set_reuseport(sock) if allow_broadcast: diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index ca99d5c6c0ac85..05f01d00493c1b 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -1752,7 +1752,6 @@ class FakeSock: def test_create_datagram_endpoint_sockopts(self): # Socket options should not be applied unless asked for. - # SO_REUSEADDR defaults to on for UNIX. # SO_REUSEPORT is not available on all platforms. coro = self.loop.create_datagram_endpoint( @@ -1761,18 +1760,8 @@ def test_create_datagram_endpoint_sockopts(self): transport, protocol = self.loop.run_until_complete(coro) sock = transport.get_extra_info('socket') - reuse_address_default_on = ( - os.name == 'posix' and sys.platform != 'cygwin') reuseport_supported = hasattr(socket, 'SO_REUSEPORT') - if reuse_address_default_on: - self.assertTrue( - sock.getsockopt( - socket.SOL_SOCKET, socket.SO_REUSEADDR)) - else: - self.assertFalse( - sock.getsockopt( - socket.SOL_SOCKET, socket.SO_REUSEADDR)) if reuseport_supported: self.assertFalse( sock.getsockopt( @@ -1788,13 +1777,12 @@ def test_create_datagram_endpoint_sockopts(self): coro = self.loop.create_datagram_endpoint( lambda: MyDatagramProto(create_future=True, loop=self.loop), local_addr=('127.0.0.1', 0), - reuse_address=True, reuse_port=reuseport_supported, allow_broadcast=True) transport, protocol = self.loop.run_until_complete(coro) sock = transport.get_extra_info('socket') - self.assertTrue( + self.assertFalse( sock.getsockopt( socket.SOL_SOCKET, socket.SO_REUSEADDR)) if reuseport_supported: @@ -1809,6 +1797,18 @@ def test_create_datagram_endpoint_sockopts(self): self.loop.run_until_complete(protocol.done) self.assertEqual('CLOSED', protocol.state) + def test_create_datagram_endpoint_reuse_address_error(self): + # bpo-37228: Ensure that explicit passing of `reuse_address=True` + # raises an error, as it is not safe to use SO_REUSEADDR when using UDP + + coro = self.loop.create_datagram_endpoint( + lambda: MyDatagramProto(create_future=True, loop=self.loop), + local_addr=('127.0.0.1', 0), + reuse_address=True) + + with self.assertRaises(RuntimeError): + self.loop.run_until_complete(coro) + @patch_socket def test_create_datagram_endpoint_nosoreuseport(self, m_socket): del m_socket.SO_REUSEPORT From 69faa22470df9d44953512368028542d14a7361b Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Thu, 21 Nov 2019 04:47:37 -0500 Subject: [PATCH 02/13] Formatting fix --- Doc/library/asyncio-eventloop.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst index 945235c8cc08cf..63770a7ed62142 100644 --- a/Doc/library/asyncio-eventloop.rst +++ b/Doc/library/asyncio-eventloop.rst @@ -476,7 +476,7 @@ Opening network connections .. note:: The parameter *reuse_address* is no longer supported, as using :py:data:`~sockets.SO_REUSEADDR` poses a significant security concern for - UDP. Explicitly passing `reuse_address=True` will raise an exception. + UDP. Explicitly passing ``reuse_address=True`` will raise an exception. For supported platforms, *reuse_port* can be used instead for similar functionality. From dea5eb3fae9717c9db2ab1064371f82738cad7c5 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 21 Nov 2019 21:36:56 +0000 Subject: [PATCH 03/13] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst diff --git a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst new file mode 100644 index 00000000000000..97a6e865d7171a --- /dev/null +++ b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst @@ -0,0 +1,3 @@ +Due to significant security concerns, the *reuse_address* parameter of +:meth:`asyncio.loop.create_datagram_endpoint` is no longer supported. This is +due to the behavior of `SO_REUSEADDR` in UDP. \ No newline at end of file From a96fe616ec96cfba34eb8ba3bd49a1c41b409d14 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Thu, 21 Nov 2019 16:38:33 -0500 Subject: [PATCH 04/13] Misc/NEWS formatting fix --- .../next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst index 97a6e865d7171a..a013dcddcf393e 100644 --- a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst +++ b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst @@ -1,3 +1,3 @@ Due to significant security concerns, the *reuse_address* parameter of :meth:`asyncio.loop.create_datagram_endpoint` is no longer supported. This is -due to the behavior of `SO_REUSEADDR` in UDP. \ No newline at end of file +due to the behavior of ``SO_REUSEADDR`` in UDP. From 2cebb8d8ef15df6db15e06ba61f96ab08d39e4b7 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 25 Nov 2019 16:41:40 -0500 Subject: [PATCH 05/13] Change RuntimeError to ValueError --- Lib/asyncio/base_events.py | 2 +- Lib/test/test_asyncio/test_base_events.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 581596c9b86542..39492f914d08ba 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1308,7 +1308,7 @@ async def create_datagram_endpoint(self, protocol_factory, if reuse_address: # bpo-37228 - raise RuntimeError("Passing `reuse_address=True` is no " + raise ValueError("Passing `reuse_address=True` is no " "longer supported, as the usage of " "SO_REUSEPORT in UDP poses a significant " "security concern.") diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index 05f01d00493c1b..47a8e380c41b64 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -1806,7 +1806,7 @@ def test_create_datagram_endpoint_reuse_address_error(self): local_addr=('127.0.0.1', 0), reuse_address=True) - with self.assertRaises(RuntimeError): + with self.assertRaises(ValueError): self.loop.run_until_complete(coro) @patch_socket From 92cf3010ef15428c9218eb5cd5a4460a8790f1f4 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 25 Nov 2019 18:01:19 -0500 Subject: [PATCH 06/13] Make description of security concern more explicit --- Doc/library/asyncio-eventloop.rst | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst index 63770a7ed62142..b248db61a19534 100644 --- a/Doc/library/asyncio-eventloop.rst +++ b/Doc/library/asyncio-eventloop.rst @@ -477,8 +477,16 @@ Opening network connections The parameter *reuse_address* is no longer supported, as using :py:data:`~sockets.SO_REUSEADDR` poses a significant security concern for UDP. Explicitly passing ``reuse_address=True`` will raise an exception. - For supported platforms, *reuse_port* can be used instead for similar - functionality. + + When multiple processes with differing UIDs assign sockets to an + indentical UDP socket address with ``SO_REUSEADDR``, incoming packets can + become randomly distributed among the sockets. + + For supported platforms, *reuse_port* can be used as a replacement for + similar functionality. With *reuse_port*, + :py:data:`~sockets.SO_REUSEPORT` is used instead, which specifically + prevents processes with differing UIDs from assigning sockets to the same + socket address. Create a datagram connection. From 3292cb1c7a590368636d07c456ec67cd84e551b8 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Tue, 26 Nov 2019 23:02:51 -0500 Subject: [PATCH 07/13] Minor Misc/NEWS improvement --- .../next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst index a013dcddcf393e..666567c741f9bb 100644 --- a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst +++ b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst @@ -1,3 +1,4 @@ Due to significant security concerns, the *reuse_address* parameter of :meth:`asyncio.loop.create_datagram_endpoint` is no longer supported. This is -due to the behavior of ``SO_REUSEADDR`` in UDP. +because of the behavior of ``SO_REUSEADDR`` in UDP. For more details, see the +documentation for ``loop.create_datagram_endpoint()``. From 3146c7f20fddfc44eade31fb16829da56dd1327c Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 2 Dec 2019 19:49:54 -0500 Subject: [PATCH 08/13] Fix alignment of ValueError string Co-authored-by: Yury Selivanov --- Lib/asyncio/base_events.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 39492f914d08ba..9915502cf5e1c2 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1309,9 +1309,9 @@ async def create_datagram_endpoint(self, protocol_factory, if reuse_address: # bpo-37228 raise ValueError("Passing `reuse_address=True` is no " - "longer supported, as the usage of " - "SO_REUSEPORT in UDP poses a significant " - "security concern.") + "longer supported, as the usage of " + "SO_REUSEPORT in UDP poses a significant " + "security concern.") for ((family, proto), (local_address, remote_address)) in addr_pairs_info: From b83052e58d56e3e11c567b184992762b96ed97d4 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 2 Dec 2019 20:29:44 -0500 Subject: [PATCH 09/13] Deprecate reuse_address=False Co-authored-by: Yury Selivanov --- Lib/asyncio/base_events.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 9915502cf5e1c2..749638a278b1aa 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -66,6 +66,10 @@ # Maximum timeout passed to select to avoid OS limitations MAXIMUM_SELECT_TIMEOUT = 24 * 3600 +# Used for deprecation and removal of `loop.create_datagram_endpoint()`'s +# *reuse_address* parameter +_unset = object() + def _format_handle(handle): cb = handle._callback @@ -1230,7 +1234,7 @@ async def start_tls(self, transport, protocol, sslcontext, *, async def create_datagram_endpoint(self, protocol_factory, local_addr=None, remote_addr=None, *, family=0, proto=0, flags=0, - reuse_address=None, reuse_port=None, + reuse_address=_unset, reuse_port=None, allow_broadcast=None, sock=None): """Create datagram connection.""" if sock is not None: @@ -1306,12 +1310,18 @@ async def create_datagram_endpoint(self, protocol_factory, exceptions = [] - if reuse_address: - # bpo-37228 - raise ValueError("Passing `reuse_address=True` is no " - "longer supported, as the usage of " - "SO_REUSEPORT in UDP poses a significant " - "security concern.") + # bpo-37228 + if reuse_address is not _unset: + if reuse_address: + raise ValueError("Passing `reuse_address=True` is no " + "longer supported, as the usage of " + "SO_REUSEPORT in UDP poses a significant " + "security concern.") + else: + warnings.warn("The *reuse_address* parameter has been " + "deprecated as of 3.5.10 and is scheduled " + "for removal in 3.11.", DeprecationWarning, + stacklevel=2) for ((family, proto), (local_address, remote_address)) in addr_pairs_info: From f0e0369d53929c910205a694517d6e518ce0eddc Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 2 Dec 2019 20:29:52 -0500 Subject: [PATCH 10/13] Update tests --- Lib/test/test_asyncio/test_base_events.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index 47a8e380c41b64..6c0f00dc936474 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -1738,10 +1738,6 @@ class FakeSock: MyDatagramProto, flags=1, sock=FakeSock()) self.assertRaises(ValueError, self.loop.run_until_complete, fut) - fut = self.loop.create_datagram_endpoint( - MyDatagramProto, reuse_address=True, sock=FakeSock()) - self.assertRaises(ValueError, self.loop.run_until_complete, fut) - fut = self.loop.create_datagram_endpoint( MyDatagramProto, reuse_port=True, sock=FakeSock()) self.assertRaises(ValueError, self.loop.run_until_complete, fut) @@ -1809,6 +1805,17 @@ def test_create_datagram_endpoint_reuse_address_error(self): with self.assertRaises(ValueError): self.loop.run_until_complete(coro) + def test_create_datagram_endpoint_reuse_address_warning(self): + # bpo-37228: Deprecate *reuse_address* parameter + + coro = self.loop.create_datagram_endpoint( + lambda: MyDatagramProto(create_future=True, loop=self.loop), + local_addr=('127.0.0.1', 0), + reuse_address=False) + + with self.assertWarns(DeprecationWarning): + self.loop.run_until_complete(coro) + @patch_socket def test_create_datagram_endpoint_nosoreuseport(self, m_socket): del m_socket.SO_REUSEPORT From f8b44efce6745aede565bf2bcafe7806dab44449 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 2 Dec 2019 20:51:32 -0500 Subject: [PATCH 11/13] Add "Contributed by" to Misc/NEWS --- .../next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst index 666567c741f9bb..0fafb63402e468 100644 --- a/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst +++ b/Misc/NEWS.d/next/Security/2019-11-21-21-36-54.bpo-37228.yBZnFG.rst @@ -2,3 +2,5 @@ Due to significant security concerns, the *reuse_address* parameter of :meth:`asyncio.loop.create_datagram_endpoint` is no longer supported. This is because of the behavior of ``SO_REUSEADDR`` in UDP. For more details, see the documentation for ``loop.create_datagram_endpoint()``. +(Contributed by Kyle Stanley, Antoine Pitrou, and Yury Selivanov in +:issue:`37228`.) From 55314d85955b58c1f222f8c11474f275ffaf894b Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Mon, 2 Dec 2019 21:36:11 -0500 Subject: [PATCH 12/13] Remove reuse_address from socket modifiers conditional --- Lib/asyncio/base_events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index 749638a278b1aa..adcdec171b07a9 100644 --- a/Lib/asyncio/base_events.py +++ b/Lib/asyncio/base_events.py @@ -1243,7 +1243,7 @@ async def create_datagram_endpoint(self, protocol_factory, f'A UDP Socket was expected, got {sock!r}') if (local_addr or remote_addr or family or proto or flags or - reuse_address or reuse_port or allow_broadcast): + reuse_port or allow_broadcast): # show the problematic kwargs in exception msg opts = dict(local_addr=local_addr, remote_addr=remote_addr, family=family, proto=proto, flags=flags, From dc521c5c5c715f37d69f9dfd28b0a83d017422c7 Mon Sep 17 00:00:00 2001 From: Kyle Stanley Date: Sat, 7 Dec 2019 18:56:59 -0500 Subject: [PATCH 13/13] Modify versionchanged to 3.8.1 --- Doc/library/asyncio-eventloop.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/asyncio-eventloop.rst b/Doc/library/asyncio-eventloop.rst index b248db61a19534..d9b1cf7a8d8351 100644 --- a/Doc/library/asyncio-eventloop.rst +++ b/Doc/library/asyncio-eventloop.rst @@ -537,7 +537,7 @@ Opening network connections The *family*, *proto*, *flags*, *reuse_address*, *reuse_port, *allow_broadcast*, and *sock* parameters were added. - .. versionchanged:: 3.5.10 + .. versionchanged:: 3.8.1 The *reuse_address* parameter is no longer supported due to security concerns.