| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent a14a4a6 commit 1697ea3
8 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -581,6 +581,10 @@ Opening network connections | |||
| 581 | 581 | .. versionchanged:: 3.12 | |
| 582 | 582 | *all_errors* was added. | |
| 583 | 583 | ||
| 584 | + .. versionchanged:: next | ||
| 585 | + Raises a ``ValueError`` if ``ssl.check_hostname`` is ``True`` | ||
| 586 | + and ``server_hostname`` is not supplied. | ||
| 587 | + | ||
| 584 | 588 | .. seealso:: | |
| 585 | 589 | ||
| 586 | 590 | The :func:`open_connection` function is a high-level alternative | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -425,6 +425,10 @@ StreamWriter | |||
| 425 | 425 | .. versionchanged:: 3.12 | |
| 426 | 426 | Added the *ssl_shutdown_timeout* parameter. | |
| 427 | 427 | ||
| 428 | + .. versionchanged:: next | ||
| 429 | + Raises a ``ValueError`` if ``sslcontext.check_hostname`` is ``True`` | ||
| 430 | + and ``server_hostname`` is not supplied. | ||
| 431 | + | ||
| 428 | 432 | ||
| 429 | 433 | .. method:: is_closing() | |
| 430 | 434 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2004,7 +2004,11 @@ to speed up repeated connections from the same clients. | |||
| 2004 | 2004 | outgoing BIO. | |
| 2005 | 2005 | ||
| 2006 | 2006 | The *server_side*, *server_hostname* and *session* parameters have the | |
| 2007 | - same meaning as in :meth:`SSLContext.wrap_socket`. | ||
| 2007 | + same meaning as in :meth:`SSLContext.wrap_socket`, and are validated in | ||
| 2008 | + the same way: in particular a :exc:`ValueError` is raised when | ||
| 2009 | + :attr:`~SSLContext.check_hostname` is enabled but no *server_hostname* is | ||
| 2010 | + given, since there would be no name to match the peer's certificate | ||
| 2011 | + against. | ||
| 2008 | 2012 | ||
| 2009 | 2013 | .. versionchanged:: 3.6 | |
| 2010 | 2014 | *session* argument was added. | |
@@ -2013,6 +2017,13 @@ to speed up repeated connections from the same clients. | |||
| 2013 | 2017 | The method returns an instance of :attr:`SSLContext.sslobject_class` | |
| 2014 | 2018 | instead of hard-coded :class:`SSLObject`. | |
| 2015 | 2019 | ||
| 2020 | + .. versionchanged:: next | ||
| 2021 | + The *server_side*, *server_hostname* and *session* parameters are now | ||
| 2022 | + validated as :meth:`SSLContext.wrap_socket` validates them. Previously | ||
| 2023 | + a context with :attr:`~SSLContext.check_hostname` enabled and no | ||
| 2024 | + *server_hostname* was accepted, and verified the certificate chain but | ||
| 2025 | + never the peer's identity. | ||
| 2026 | + | ||
| 2016 | 2027 | .. attribute:: SSLContext.sslobject_class | |
| 2017 | 2028 | ||
| 2018 | 2029 | The return type of :meth:`SSLContext.wrap_bio`, defaults to | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -374,6 +374,20 @@ def _ipaddress_match(cert_ipaddress, host_ip): | |||
| 374 | 374 | return ip == host_ip | |
| 375 | 375 | ||
| 376 | 376 | ||
| 377 | + def _check_sslobject_params(server_side, context=None, server_hostname=None, session=None): | ||
| 378 | + """Raises a ValueError if SSLObject._create() parameters aren't valid. | ||
| 379 | + """ | ||
| 380 | + if server_side: | ||
| 381 | + if server_hostname: | ||
| 382 | + raise ValueError("server_hostname can only be specified " | ||
| 383 | + "in client mode") | ||
| 384 | + if session is not None: | ||
| 385 | + raise ValueError("session can only be specified in " | ||
| 386 | + "client mode") | ||
| 387 | + if context.check_hostname and not server_hostname: | ||
| 388 | + raise ValueError("check_hostname requires server_hostname") | ||
| 389 | + | ||
| 390 | + | ||
| 377 | 391 | DefaultVerifyPaths = namedtuple("DefaultVerifyPaths", | |
| 378 | 392 | "cafile capath openssl_cafile_env openssl_cafile openssl_capath_env " | |
| 379 | 393 | "openssl_capath") | |
@@ -812,6 +826,8 @@ def __init__(self, *args, **kwargs): | |||
| 812 | 826 | @classmethod | |
| 813 | 827 | def _create(cls, incoming, outgoing, server_side=False, | |
| 814 | 828 | server_hostname=None, session=None, context=None): | |
| 829 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 830 | + server_hostname=server_hostname, session=session) | ||
| 815 | 831 | self = cls.__new__(cls) | |
| 816 | 832 | sslobj = context._wrap_bio( | |
| 817 | 833 | incoming, outgoing, server_side=server_side, | |
@@ -1008,15 +1024,8 @@ def _create(cls, sock, server_side=False, do_handshake_on_connect=True, | |||
| 1008 | 1024 | context=None, session=None): | |
| 1009 | 1025 | if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM: | |
| 1010 | 1026 | raise NotImplementedError("only stream sockets are supported") | |
| 1011 | - if server_side: | ||
| 1012 | - if server_hostname: | ||
| 1013 | - raise ValueError("server_hostname can only be specified " | ||
| 1014 | - "in client mode") | ||
| 1015 | - if session is not None: | ||
| 1016 | - raise ValueError("session can only be specified in " | ||
| 1017 | - "client mode") | ||
| 1018 | - if context.check_hostname and not server_hostname: | ||
| 1019 | - raise ValueError("check_hostname requires server_hostname") | ||
| 1027 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 1028 | + server_hostname=server_hostname, session=session) | ||
| 1020 | 1029 | ||
| 1021 | 1030 | sock_timeout = sock.gettimeout() | |
| 1022 | 1031 | 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 | |
|---|---|---|---|
@@ -399,6 +399,34 @@ def do_ssl_object_handshake(sslobject, outgoing, max_retry=25): | |||
| 399 | 399 | return data | |
| 400 | 400 | ||
| 401 | 401 | ||
| 402 | + def connected_bio_pair(client_context, server_context, hostname, max_retry=5): | ||
| 403 | + """Handshake a client and a server SSLObject against each other. | ||
| 404 | + | ||
| 405 | + Everything happens in memory, so this needs no socket and no thread. | ||
| 406 | + Returns the two objects followed by their four BIOs, in the order | ||
| 407 | + client, server, c_in, c_out, s_in, s_out. | ||
| 408 | + """ | ||
| 409 | + c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 410 | + s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 411 | + client = client_context.wrap_bio(c_in, c_out, server_hostname=hostname) | ||
| 412 | + server = server_context.wrap_bio(s_in, s_out, server_side=True) | ||
| 413 | + | ||
| 414 | + # Loop on the handshake for a bit to get it settled | ||
| 415 | + for _ in range(max_retry): | ||
| 416 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 417 | + client.do_handshake() | ||
| 418 | + if c_out.pending: | ||
| 419 | + s_in.write(c_out.read()) | ||
| 420 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 421 | + server.do_handshake() | ||
| 422 | + if s_out.pending: | ||
| 423 | + c_in.write(s_out.read()) | ||
| 424 | + # Now the handshakes should be complete (don't raise WantReadError) | ||
| 425 | + client.do_handshake() | ||
| 426 | + server.do_handshake() | ||
| 427 | + return client, server, c_in, c_out, s_in, s_out | ||
| 428 | + | ||
| 429 | + | ||
| 402 | 430 | class BasicSocketTests(unittest.TestCase): | |
| 403 | 431 | ||
| 404 | 432 | def test_constants(self): | |
@@ -1859,6 +1887,7 @@ def test__create_stdlib_context_check_hostname(self): | |||
| 1859 | 1887 | def test_delete_sslobject_attributes(self): | |
| 1860 | 1888 | # None of the attributes of _ssl._SSLSocket can be deleted. | |
| 1861 | 1889 | ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) | |
| 1890 | + ctx.check_hostname = False | ||
| 1862 | 1891 | sslobj = ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO())._sslobj | |
| 1863 | 1892 | for name in 'context', 'owner', 'session', 'session_reused': | |
| 1864 | 1893 | with self.subTest(name=name): | |
@@ -2053,6 +2082,10 @@ def test_subclass(self): | |||
| 2053 | 2082 | ||
| 2054 | 2083 | def test_bad_server_hostname(self): | |
| 2055 | 2084 | ctx = ssl.create_default_context() | |
| 2085 | + # Omitting the name entirely is bad too: this context checks it. | ||
| 2086 | + with self.assertRaises(ValueError): | ||
| 2087 | + ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2088 | + server_hostname=None) | ||
| 2056 | 2089 | with self.assertRaises(ValueError): | |
| 2057 | 2090 | ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | |
| 2058 | 2091 | server_hostname="") | |
@@ -2137,6 +2170,64 @@ def test_private_init(self): | |||
| 2137 | 2170 | with self.assertRaisesRegex(TypeError, "public constructor"): | |
| 2138 | 2171 | ssl.SSLObject(bio, bio) | |
| 2139 | 2172 | ||
| 2173 | + def test_check_hostname_requires_server_hostname(self): | ||
| 2174 | + # wrap_bio() used to accept a context asking for hostname checking | ||
| 2175 | + # without a name to check against, and then verify the certificate | ||
| 2176 | + # chain but never the peer's identity, with check_hostname still | ||
| 2177 | + # reporting True and nothing reporting the check had been skipped. | ||
| 2178 | + # It must refuse that call, as wrap_socket() already did. | ||
| 2179 | + client_context, _, hostname = testing_context() | ||
| 2180 | + self.assertTrue(client_context.check_hostname) | ||
| 2181 | + | ||
| 2182 | + for server_hostname in (None, ""): | ||
| 2183 | + with self.subTest(server_hostname=server_hostname): | ||
| 2184 | + with self.assertRaisesRegex( | ||
| 2185 | + ValueError, | ||
| 2186 | + "check_hostname requires server_hostname"): | ||
| 2187 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2188 | + server_hostname=server_hostname) | ||
| 2189 | + # The sibling constructor refuses the very same call. | ||
| 2190 | + with socket.socket() as sock: | ||
| 2191 | + with self.assertRaisesRegex( | ||
| 2192 | + ValueError, | ||
| 2193 | + "check_hostname requires server_hostname"): | ||
| 2194 | + client_context.wrap_socket( | ||
| 2195 | + sock, server_hostname=server_hostname) | ||
| 2196 | + | ||
| 2197 | + # A name was all that was missing. | ||
| 2198 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2199 | + server_hostname=hostname) | ||
| 2200 | + | ||
| 2201 | + # Asking for no hostname check remains a way to say so explicitly. | ||
| 2202 | + context = make_test_context() | ||
| 2203 | + self.assertFalse(context.check_hostname) | ||
| 2204 | + context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO()) | ||
| 2205 | + | ||
| 2206 | + def test_server_side_bad_params(self): | ||
| 2207 | + # A server neither sends a hostname nor resumes a client's session, | ||
| 2208 | + # so wrap_bio() rejects both in server mode like wrap_socket() | ||
| 2209 | + client_context, server_context, hostname = testing_context() | ||
| 2210 | + | ||
| 2211 | + with self.assertRaisesRegex( | ||
| 2212 | + ValueError, | ||
| 2213 | + "server_hostname can only be specified in client mode"): | ||
| 2214 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2215 | + server_side=True, | ||
| 2216 | + server_hostname=hostname) | ||
| 2217 | + | ||
| 2218 | + client, server, *_ = connected_bio_pair( | ||
| 2219 | + client_context, server_context, hostname) | ||
| 2220 | + session = client.session | ||
| 2221 | + self.assertIsNotNone(session) | ||
| 2222 | + with self.assertRaisesRegex( | ||
| 2223 | + ValueError, "session can only be specified in client mode"): | ||
| 2224 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2225 | + server_side=True, session=session) | ||
| 2226 | + | ||
| 2227 | + # Neither argument is what a server passes, so this still works. | ||
| 2228 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2229 | + server_side=True) | ||
| 2230 | + | ||
| 2140 | 2231 | def test_unwrap(self): | |
| 2141 | 2232 | client_ctx, server_ctx, hostname = testing_context() | |
| 2142 | 2233 | 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