| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 8b1df7a commit a7e1724
8 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -592,6 +592,10 @@ Opening network connections | |||
| 592 | 592 | .. versionchanged:: 3.12 | |
| 593 | 593 | *all_errors* was added. | |
| 594 | 594 | ||
| 595 | + .. versionchanged:: next | ||
| 596 | + Raises a ``ValueError`` if ``ssl.check_hostname`` is ``True`` | ||
| 597 | + and ``server_hostname`` is not supplied. | ||
| 598 | + | ||
| 595 | 599 | .. seealso:: | |
| 596 | 600 | ||
| 597 | 601 | The :func:`open_connection` function is a high-level alternative | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -418,6 +418,10 @@ StreamWriter | |||
| 418 | 418 | .. versionchanged:: 3.12 | |
| 419 | 419 | Added the *ssl_shutdown_timeout* parameter. | |
| 420 | 420 | ||
| 421 | + .. versionchanged:: next | ||
| 422 | + Raises a ``ValueError`` if ``sslcontext.check_hostname`` is ``True`` | ||
| 423 | + and ``server_hostname`` is not supplied. | ||
| 424 | + | ||
| 421 | 425 | ||
| 422 | 426 | .. method:: is_closing() | |
| 423 | 427 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1992,7 +1992,11 @@ to speed up repeated connections from the same clients. | |||
| 1992 | 1992 | outgoing BIO. | |
| 1993 | 1993 | ||
| 1994 | 1994 | The *server_side*, *server_hostname* and *session* parameters have the | |
| 1995 | - same meaning as in :meth:`SSLContext.wrap_socket`. | ||
| 1995 | + same meaning as in :meth:`SSLContext.wrap_socket`, and are validated in | ||
| 1996 | + the same way: in particular a :exc:`ValueError` is raised when | ||
| 1997 | + :attr:`~SSLContext.check_hostname` is enabled but no *server_hostname* is | ||
| 1998 | + given, since there would be no name to match the peer's certificate | ||
| 1999 | + against. | ||
| 1996 | 2000 | ||
| 1997 | 2001 | .. versionchanged:: 3.6 | |
| 1998 | 2002 | *session* argument was added. | |
@@ -2001,6 +2005,13 @@ to speed up repeated connections from the same clients. | |||
| 2001 | 2005 | The method returns an instance of :attr:`SSLContext.sslobject_class` | |
| 2002 | 2006 | instead of hard-coded :class:`SSLObject`. | |
| 2003 | 2007 | ||
| 2008 | + .. versionchanged:: next | ||
| 2009 | + The *server_side*, *server_hostname* and *session* parameters are now | ||
| 2010 | + validated as :meth:`SSLContext.wrap_socket` validates them. Previously | ||
| 2011 | + a context with :attr:`~SSLContext.check_hostname` enabled and no | ||
| 2012 | + *server_hostname* was accepted, and verified the certificate chain but | ||
| 2013 | + never the peer's identity. | ||
| 2014 | + | ||
| 2004 | 2015 | .. attribute:: SSLContext.sslobject_class | |
| 2005 | 2016 | ||
| 2006 | 2017 | The return type of :meth:`SSLContext.wrap_bio`, defaults to | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -373,6 +373,20 @@ def _ipaddress_match(cert_ipaddress, host_ip): | |||
| 373 | 373 | return ip == host_ip | |
| 374 | 374 | ||
| 375 | 375 | ||
| 376 | + def _check_sslobject_params(server_side, context=None, server_hostname=None, session=None): | ||
| 377 | + """Raises a ValueError if SSLObject._create() parameters aren't valid. | ||
| 378 | + """ | ||
| 379 | + if server_side: | ||
| 380 | + if server_hostname: | ||
| 381 | + raise ValueError("server_hostname can only be specified " | ||
| 382 | + "in client mode") | ||
| 383 | + if session is not None: | ||
| 384 | + raise ValueError("session can only be specified in " | ||
| 385 | + "client mode") | ||
| 386 | + if context.check_hostname and not server_hostname: | ||
| 387 | + raise ValueError("check_hostname requires server_hostname") | ||
| 388 | + | ||
| 389 | + | ||
| 376 | 390 | DefaultVerifyPaths = namedtuple("DefaultVerifyPaths", | |
| 377 | 391 | "cafile capath openssl_cafile_env openssl_cafile openssl_capath_env " | |
| 378 | 392 | "openssl_capath") | |
@@ -813,6 +827,8 @@ def __init__(self, *args, **kwargs): | |||
| 813 | 827 | @classmethod | |
| 814 | 828 | def _create(cls, incoming, outgoing, server_side=False, | |
| 815 | 829 | server_hostname=None, session=None, context=None): | |
| 830 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 831 | + server_hostname=server_hostname, session=session) | ||
| 816 | 832 | self = cls.__new__(cls) | |
| 817 | 833 | sslobj = context._wrap_bio( | |
| 818 | 834 | incoming, outgoing, server_side=server_side, | |
@@ -1009,15 +1025,8 @@ def _create(cls, sock, server_side=False, do_handshake_on_connect=True, | |||
| 1009 | 1025 | context=None, session=None): | |
| 1010 | 1026 | if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM: | |
| 1011 | 1027 | raise NotImplementedError("only stream sockets are supported") | |
| 1012 | - if server_side: | ||
| 1013 | - if server_hostname: | ||
| 1014 | - raise ValueError("server_hostname can only be specified " | ||
| 1015 | - "in client mode") | ||
| 1016 | - if session is not None: | ||
| 1017 | - raise ValueError("session can only be specified in " | ||
| 1018 | - "client mode") | ||
| 1019 | - if context.check_hostname and not server_hostname: | ||
| 1020 | - raise ValueError("check_hostname requires server_hostname") | ||
| 1028 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 1029 | + server_hostname=server_hostname, session=session) | ||
| 1021 | 1030 | ||
| 1022 | 1031 | sock_timeout = sock.gettimeout() | |
| 1023 | 1032 | kwargs = dict( | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -70,6 +70,50 @@ def test_handshake_timeout_negative(self): | |||
| 70 | 70 | sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter, | |
| 71 | 71 | ssl_handshake_timeout=-10) | |
| 72 | 72 | ||
| 73 | + def test_check_hostname_accepts_server_hostname(self): | ||
| 74 | + # Supplying a server_hostname succeeds with check_hostname enabled. | ||
| 75 | + sslcontext = test_utils.simple_client_sslcontext(disable_verify=False) | ||
| 76 | + sslcontext.check_hostname = True | ||
| 77 | + app_proto = mock.Mock() | ||
| 78 | + waiter = mock.Mock() | ||
| 79 | + | ||
| 80 | + # No ValueError is raised from SSLProtocol with 'server_hostname'. | ||
| 81 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter, | ||
| 82 | + server_hostname='example.org') | ||
| 83 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 84 | + | ||
| 85 | + @support.subTests("server_hostname", [None, '']) | ||
| 86 | + def test_check_hostname_requires_server_hostname(self, server_hostname): | ||
| 87 | + # A caller-supplied context asking for hostname checking used to be | ||
| 88 | + # taken through wrap_bio() with no name to check against, verifying | ||
| 89 | + # the certificate chain but never the peer's identity. | ||
| 90 | + # loop.start_tls() defaults server_hostname to None, and | ||
| 91 | + # loop.create_connection() turns server_hostname='' into None here, | ||
| 92 | + # so both reached that state. | ||
| 93 | + sslcontext = test_utils.simple_client_sslcontext(disable_verify=False) | ||
| 94 | + sslcontext.check_hostname = True | ||
| 95 | + app_proto = mock.Mock() | ||
| 96 | + waiter = mock.Mock() | ||
| 97 | + | ||
| 98 | + # Supplying an empty server_hostname fails with check_hostname enabled. | ||
| 99 | + with self.assertRaisesRegex( | ||
| 100 | + ValueError, | ||
| 101 | + 'check_hostname requires server_hostname'): | ||
| 102 | + sslproto.SSLProtocol(self.loop, app_proto, sslcontext, | ||
| 103 | + waiter, | ||
| 104 | + server_hostname=server_hostname) | ||
| 105 | + | ||
| 106 | + # Disabling check_hostname allows for an empty or unset server_hostname. | ||
| 107 | + sslcontext.check_hostname = False | ||
| 108 | + | ||
| 109 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter) | ||
| 110 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 111 | + | ||
| 112 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, | ||
| 113 | + waiter, | ||
| 114 | + server_hostname=server_hostname) | ||
| 115 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 116 | + | ||
| 73 | 117 | def test_eof_received_waiter(self): | |
| 74 | 118 | waiter = self.loop.create_future() | |
| 75 | 119 | ssl_proto = self.ssl_protocol(waiter=waiter) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -402,6 +402,34 @@ def do_ssl_object_handshake(sslobject, outgoing, max_retry=25): | |||
| 402 | 402 | return data | |
| 403 | 403 | ||
| 404 | 404 | ||
| 405 | + def connected_bio_pair(client_context, server_context, hostname, max_retry=5): | ||
| 406 | + """Handshake a client and a server SSLObject against each other. | ||
| 407 | + | ||
| 408 | + Everything happens in memory, so this needs no socket and no thread. | ||
| 409 | + Returns the two objects followed by their four BIOs, in the order | ||
| 410 | + client, server, c_in, c_out, s_in, s_out. | ||
| 411 | + """ | ||
| 412 | + c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 413 | + s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 414 | + client = client_context.wrap_bio(c_in, c_out, server_hostname=hostname) | ||
| 415 | + server = server_context.wrap_bio(s_in, s_out, server_side=True) | ||
| 416 | + | ||
| 417 | + # Loop on the handshake for a bit to get it settled | ||
| 418 | + for _ in range(max_retry): | ||
| 419 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 420 | + client.do_handshake() | ||
| 421 | + if c_out.pending: | ||
| 422 | + s_in.write(c_out.read()) | ||
| 423 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 424 | + server.do_handshake() | ||
| 425 | + if s_out.pending: | ||
| 426 | + c_in.write(s_out.read()) | ||
| 427 | + # Now the handshakes should be complete (don't raise WantReadError) | ||
| 428 | + client.do_handshake() | ||
| 429 | + server.do_handshake() | ||
| 430 | + return client, server, c_in, c_out, s_in, s_out | ||
| 431 | + | ||
| 432 | + | ||
| 405 | 433 | class BasicSocketTests(unittest.TestCase): | |
| 406 | 434 | ||
| 407 | 435 | def test_constants(self): | |
@@ -1990,6 +2018,10 @@ def test_subclass(self): | |||
| 1990 | 2018 | ||
| 1991 | 2019 | def test_bad_server_hostname(self): | |
| 1992 | 2020 | ctx = ssl.create_default_context() | |
| 2021 | + # Omitting the name entirely is bad too: this context checks it. | ||
| 2022 | + with self.assertRaises(ValueError): | ||
| 2023 | + ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2024 | + server_hostname=None) | ||
| 1993 | 2025 | with self.assertRaises(ValueError): | |
| 1994 | 2026 | ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | |
| 1995 | 2027 | server_hostname="") | |
@@ -2074,6 +2106,64 @@ def test_private_init(self): | |||
| 2074 | 2106 | with self.assertRaisesRegex(TypeError, "public constructor"): | |
| 2075 | 2107 | ssl.SSLObject(bio, bio) | |
| 2076 | 2108 | ||
| 2109 | + def test_check_hostname_requires_server_hostname(self): | ||
| 2110 | + # wrap_bio() used to accept a context asking for hostname checking | ||
| 2111 | + # without a name to check against, and then verify the certificate | ||
| 2112 | + # chain but never the peer's identity, with check_hostname still | ||
| 2113 | + # reporting True and nothing reporting the check had been skipped. | ||
| 2114 | + # It must refuse that call, as wrap_socket() already did. | ||
| 2115 | + client_context, _, hostname = testing_context() | ||
| 2116 | + self.assertTrue(client_context.check_hostname) | ||
| 2117 | + | ||
| 2118 | + for server_hostname in (None, ""): | ||
| 2119 | + with self.subTest(server_hostname=server_hostname): | ||
| 2120 | + with self.assertRaisesRegex( | ||
| 2121 | + ValueError, | ||
| 2122 | + "check_hostname requires server_hostname"): | ||
| 2123 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2124 | + server_hostname=server_hostname) | ||
| 2125 | + # The sibling constructor refuses the very same call. | ||
| 2126 | + with socket.socket() as sock: | ||
| 2127 | + with self.assertRaisesRegex( | ||
| 2128 | + ValueError, | ||
| 2129 | + "check_hostname requires server_hostname"): | ||
| 2130 | + client_context.wrap_socket( | ||
| 2131 | + sock, server_hostname=server_hostname) | ||
| 2132 | + | ||
| 2133 | + # A name was all that was missing. | ||
| 2134 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2135 | + server_hostname=hostname) | ||
| 2136 | + | ||
| 2137 | + # Asking for no hostname check remains a way to say so explicitly. | ||
| 2138 | + context = make_test_context() | ||
| 2139 | + self.assertFalse(context.check_hostname) | ||
| 2140 | + context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO()) | ||
| 2141 | + | ||
| 2142 | + def test_server_side_bad_params(self): | ||
| 2143 | + # A server neither sends a hostname nor resumes a client's session, | ||
| 2144 | + # so wrap_bio() rejects both in server mode like wrap_socket() | ||
| 2145 | + client_context, server_context, hostname = testing_context() | ||
| 2146 | + | ||
| 2147 | + with self.assertRaisesRegex( | ||
| 2148 | + ValueError, | ||
| 2149 | + "server_hostname can only be specified in client mode"): | ||
| 2150 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2151 | + server_side=True, | ||
| 2152 | + server_hostname=hostname) | ||
| 2153 | + | ||
| 2154 | + client, server, *_ = connected_bio_pair( | ||
| 2155 | + client_context, server_context, hostname) | ||
| 2156 | + session = client.session | ||
| 2157 | + self.assertIsNotNone(session) | ||
| 2158 | + with self.assertRaisesRegex( | ||
| 2159 | + ValueError, "session can only be specified in client mode"): | ||
| 2160 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2161 | + server_side=True, session=session) | ||
| 2162 | + | ||
| 2163 | + # Neither argument is what a server passes, so this still works. | ||
| 2164 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2165 | + server_side=True) | ||
| 2166 | + | ||
| 2077 | 2167 | def test_unwrap(self): | |
| 2078 | 2168 | client_ctx, server_ctx, hostname = testing_context() | |
| 2079 | 2169 | c_in = ssl.MemoryBIO() | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,9 @@ | |||
| 1 | + :meth:`ssl.SSLContext.wrap_bio` now validates its *server_side*, | ||
| 2 | + *server_hostname* and *session* arguments similar to | ||
| 3 | + :meth:`ssl.SSLContext.wrap_socket`. | ||
| 4 | + | ||
| 5 | + In particular, a context with :attr:`~ssl.SSLContext.check_hostname` enabled | ||
| 6 | + and no *server_hostname* passed to :meth:`!wrap_bio` now raises :exc:`ValueError` | ||
| 7 | + instead of completing a handshake that verified the certificate chain | ||
| 8 | + without verifying the peer's identity, with no indication that the | ||
| 9 | + check had been skipped. | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,4 @@ | |||
| 1 | + :mod:`asyncio`: :meth:`loop.start_tls() <asyncio.loop.start_tls>` and | ||
| 2 | + :meth:`loop.create_connection() <asyncio.loop.create_connection>` now | ||
| 3 | + validate the *server_hostname* argument if an :class:`ssl.SSLContext` is | ||
| 4 | + passed with *check_hostname* set to ``True``. | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments