| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 197663d commit b327cd6
7 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -505,6 +505,10 @@ Opening network connections | |||
| 505 | 505 | ||
| 506 | 506 | For more information: https://tools.ietf.org/html/rfc6555 | |
| 507 | 507 | ||
| 508 | + .. versionchanged:: next | ||
| 509 | + Raises a ``ValueError`` if ``ssl.check_hostname`` is ``True`` | ||
| 510 | + and ``server_hostname`` is not supplied. | ||
| 511 | + | ||
| 508 | 512 | .. seealso:: | |
| 509 | 513 | ||
| 510 | 514 | The :func:`open_connection` function is a high-level alternative | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1890,7 +1890,11 @@ to speed up repeated connections from the same clients. | |||
| 1890 | 1890 | outgoing BIO. | |
| 1891 | 1891 | ||
| 1892 | 1892 | The *server_side*, *server_hostname* and *session* parameters have the | |
| 1893 | - same meaning as in :meth:`SSLContext.wrap_socket`. | ||
| 1893 | + same meaning as in :meth:`SSLContext.wrap_socket`, and are validated in | ||
| 1894 | + the same way: in particular a :exc:`ValueError` is raised when | ||
| 1895 | + :attr:`~SSLContext.check_hostname` is enabled but no *server_hostname* is | ||
| 1896 | + given, since there would be no name to match the peer's certificate | ||
| 1897 | + against. | ||
| 1894 | 1898 | ||
| 1895 | 1899 | .. versionchanged:: 3.6 | |
| 1896 | 1900 | *session* argument was added. | |
@@ -1899,6 +1903,13 @@ to speed up repeated connections from the same clients. | |||
| 1899 | 1903 | The method returns on instance of :attr:`SSLContext.sslobject_class` | |
| 1900 | 1904 | instead of hard-coded :class:`SSLObject`. | |
| 1901 | 1905 | ||
| 1906 | + .. versionchanged:: next | ||
| 1907 | + The *server_side*, *server_hostname* and *session* parameters are now | ||
| 1908 | + validated as :meth:`SSLContext.wrap_socket` validates them. Previously | ||
| 1909 | + a context with :attr:`~SSLContext.check_hostname` enabled and no | ||
| 1910 | + *server_hostname* was accepted, and verified the certificate chain but | ||
| 1911 | + never the peer's identity. | ||
| 1912 | + | ||
| 1902 | 1913 | .. attribute:: SSLContext.sslobject_class | |
| 1903 | 1914 | ||
| 1904 | 1915 | The return type of :meth:`SSLContext.wrap_bio`, defaults to | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -431,6 +431,20 @@ def match_hostname(cert, hostname): | |||
| 431 | 431 | "subjectAltName fields were found") | |
| 432 | 432 | ||
| 433 | 433 | ||
| 434 | + def _check_sslobject_params(server_side, context=None, server_hostname=None, session=None): | ||
| 435 | + """Raises a ValueError if SSLObject._create() parameters aren't valid. | ||
| 436 | + """ | ||
| 437 | + if server_side: | ||
| 438 | + if server_hostname: | ||
| 439 | + raise ValueError("server_hostname can only be specified " | ||
| 440 | + "in client mode") | ||
| 441 | + if session is not None: | ||
| 442 | + raise ValueError("session can only be specified in " | ||
| 443 | + "client mode") | ||
| 444 | + if context.check_hostname and not server_hostname: | ||
| 445 | + raise ValueError("check_hostname requires server_hostname") | ||
| 446 | + | ||
| 447 | + | ||
| 434 | 448 | DefaultVerifyPaths = namedtuple("DefaultVerifyPaths", | |
| 435 | 449 | "cafile capath openssl_cafile_env openssl_cafile openssl_capath_env " | |
| 436 | 450 | "openssl_capath") | |
@@ -862,6 +876,8 @@ def __init__(self, *args, **kwargs): | |||
| 862 | 876 | @classmethod | |
| 863 | 877 | def _create(cls, incoming, outgoing, server_side=False, | |
| 864 | 878 | server_hostname=None, session=None, context=None): | |
| 879 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 880 | + server_hostname=server_hostname, session=session) | ||
| 865 | 881 | self = cls.__new__(cls) | |
| 866 | 882 | sslobj = context._wrap_bio( | |
| 867 | 883 | incoming, outgoing, server_side=server_side, | |
@@ -1017,15 +1033,8 @@ def _create(cls, sock, server_side=False, do_handshake_on_connect=True, | |||
| 1017 | 1033 | context=None, session=None): | |
| 1018 | 1034 | if sock.getsockopt(SOL_SOCKET, SO_TYPE) != SOCK_STREAM: | |
| 1019 | 1035 | raise NotImplementedError("only stream sockets are supported") | |
| 1020 | - if server_side: | ||
| 1021 | - if server_hostname: | ||
| 1022 | - raise ValueError("server_hostname can only be specified " | ||
| 1023 | - "in client mode") | ||
| 1024 | - if session is not None: | ||
| 1025 | - raise ValueError("session can only be specified in " | ||
| 1026 | - "client mode") | ||
| 1027 | - if context.check_hostname and not server_hostname: | ||
| 1028 | - raise ValueError("check_hostname requires server_hostname") | ||
| 1036 | + _check_sslobject_params(server_side=server_side, context=context, | ||
| 1037 | + server_hostname=server_hostname, session=session) | ||
| 1029 | 1038 | ||
| 1030 | 1039 | kwargs = dict( | |
| 1031 | 1040 | family=sock.family, type=sock.type, proto=sock.proto, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -71,6 +71,74 @@ def test_handshake_timeout_negative(self): | |||
| 71 | 71 | sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter, | |
| 72 | 72 | ssl_handshake_timeout=-10) | |
| 73 | 73 | ||
| 74 | + def test_check_hostname_accepts_server_hostname(self): | ||
| 75 | + # Supplying a server_hostname succeeds with check_hostname enabled. | ||
| 76 | + sslcontext = test_utils.simple_client_sslcontext(disable_verify=False) | ||
| 77 | + sslcontext.check_hostname = True | ||
| 78 | + app_proto = mock.Mock() | ||
| 79 | + waiter = mock.Mock() | ||
| 80 | + | ||
| 81 | + # On this branch wrap_bio() runs when the connection is made, not | ||
| 82 | + # in the constructor, so drive that with a mock transport. | ||
| 83 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, waiter, | ||
| 84 | + server_hostname='example.org') | ||
| 85 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 86 | + transport = mock.Mock() | ||
| 87 | + ssl_proto.connection_made(transport) | ||
| 88 | + transport._force_close.assert_not_called() | ||
| 89 | + self.assertIsNotNone(ssl_proto._sslpipe.ssl_object) | ||
| 90 | + | ||
| 91 | + @support.subTests("server_hostname", [None, '']) | ||
| 92 | + def test_check_hostname_requires_server_hostname(self, server_hostname): | ||
| 93 | + # A caller-supplied context asking for hostname checking used to be | ||
| 94 | + # taken through wrap_bio() with no name to check against, verifying | ||
| 95 | + # the certificate chain but never the peer's identity. | ||
| 96 | + # loop.start_tls() defaults server_hostname to None, and | ||
| 97 | + # loop.create_connection() turns server_hostname='' into None here, | ||
| 98 | + # so both reached that state. | ||
| 99 | + sslcontext = test_utils.simple_client_sslcontext(disable_verify=False) | ||
| 100 | + sslcontext.check_hostname = True | ||
| 101 | + app_proto = mock.Mock() | ||
| 102 | + waiter = self.loop.create_future() | ||
| 103 | + | ||
| 104 | + # Supplying an empty server_hostname fails with check_hostname enabled. | ||
| 105 | + # On this branch wrap_bio() runs when the connection is made, not in | ||
| 106 | + # the constructor: the ValueError is reported as a fatal error that | ||
| 107 | + # closes the transport. | ||
| 108 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, | ||
| 109 | + waiter, | ||
| 110 | + server_hostname=server_hostname) | ||
| 111 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 112 | + transport = mock.Mock() | ||
| 113 | + with test_utils.disable_logger(): | ||
| 114 | + ssl_proto.connection_made(transport) | ||
| 115 | + transport._force_close.assert_called_once() | ||
| 116 | + exc = transport._force_close.call_args.args[0] | ||
| 117 | + self.assertIsInstance(exc, ValueError) | ||
| 118 | + self.assertRegex(str(exc), 'check_hostname requires server_hostname') | ||
| 119 | + | ||
| 120 | + # A real transport reports the close back to the protocol, which is | ||
| 121 | + # how loop.create_connection() gets to see the error. | ||
| 122 | + ssl_proto.connection_lost(exc) | ||
| 123 | + with self.assertRaisesRegex( | ||
| 124 | + ValueError, | ||
| 125 | + 'check_hostname requires server_hostname'): | ||
| 126 | + waiter.result() | ||
| 127 | + | ||
| 128 | + # Disabling check_hostname allows for an empty or unset server_hostname. | ||
| 129 | + sslcontext.check_hostname = False | ||
| 130 | + | ||
| 131 | + for kwargs in ({}, {'server_hostname': server_hostname}): | ||
| 132 | + with self.subTest(kwargs=kwargs): | ||
| 133 | + waiter = mock.Mock() | ||
| 134 | + ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext, | ||
| 135 | + waiter, **kwargs) | ||
| 136 | + self.addCleanup(ssl_proto._app_transport.close) | ||
| 137 | + transport = mock.Mock() | ||
| 138 | + ssl_proto.connection_made(transport) | ||
| 139 | + transport._force_close.assert_not_called() | ||
| 140 | + self.assertIsNotNone(ssl_proto._sslpipe.ssl_object) | ||
| 141 | + | ||
| 74 | 142 | def test_eof_received_waiter(self): | |
| 75 | 143 | waiter = self.loop.create_future() | |
| 76 | 144 | ssl_proto = self.ssl_protocol(waiter=waiter) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -9,6 +9,7 @@ | |||
| 9 | 9 | from test.support import socket_helper | |
| 10 | 10 | from test.support import threading_helper | |
| 11 | 11 | from test.support import warnings_helper | |
| 12 | + import contextlib | ||
| 12 | 13 | import re | |
| 13 | 14 | import socket | |
| 14 | 15 | import select | |
@@ -328,6 +329,34 @@ def testing_context(server_cert=SIGNED_CERTFILE, *, server_chain=True): | |||
| 328 | 329 | return client_context, server_context, hostname | |
| 329 | 330 | ||
| 330 | 331 | ||
| 332 | + def connected_bio_pair(client_context, server_context, hostname, max_retry=5): | ||
| 333 | + """Handshake a client and a server SSLObject against each other. | ||
| 334 | + | ||
| 335 | + Everything happens in memory, so this needs no socket and no thread. | ||
| 336 | + Returns the two objects followed by their four BIOs, in the order | ||
| 337 | + client, server, c_in, c_out, s_in, s_out. | ||
| 338 | + """ | ||
| 339 | + c_in, c_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 340 | + s_in, s_out = ssl.MemoryBIO(), ssl.MemoryBIO() | ||
| 341 | + client = client_context.wrap_bio(c_in, c_out, server_hostname=hostname) | ||
| 342 | + server = server_context.wrap_bio(s_in, s_out, server_side=True) | ||
| 343 | + | ||
| 344 | + # Loop on the handshake for a bit to get it settled | ||
| 345 | + for _ in range(max_retry): | ||
| 346 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 347 | + client.do_handshake() | ||
| 348 | + if c_out.pending: | ||
| 349 | + s_in.write(c_out.read()) | ||
| 350 | + with contextlib.suppress(ssl.SSLWantReadError): | ||
| 351 | + server.do_handshake() | ||
| 352 | + if s_out.pending: | ||
| 353 | + c_in.write(s_out.read()) | ||
| 354 | + # Now the handshakes should be complete (don't raise WantReadError) | ||
| 355 | + client.do_handshake() | ||
| 356 | + server.do_handshake() | ||
| 357 | + return client, server, c_in, c_out, s_in, s_out | ||
| 358 | + | ||
| 359 | + | ||
| 331 | 360 | class BasicSocketTests(unittest.TestCase): | |
| 332 | 361 | ||
| 333 | 362 | def test_constants(self): | |
@@ -1888,6 +1917,10 @@ def test_subclass(self): | |||
| 1888 | 1917 | ||
| 1889 | 1918 | def test_bad_server_hostname(self): | |
| 1890 | 1919 | ctx = ssl.create_default_context() | |
| 1920 | + # Omitting the name entirely is bad too: this context checks it. | ||
| 1921 | + with self.assertRaises(ValueError): | ||
| 1922 | + ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 1923 | + server_hostname=None) | ||
| 1891 | 1924 | with self.assertRaises(ValueError): | |
| 1892 | 1925 | ctx.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | |
| 1893 | 1926 | server_hostname="") | |
@@ -1968,6 +2001,65 @@ def test_private_init(self): | |||
| 1968 | 2001 | with self.assertRaisesRegex(TypeError, "public constructor"): | |
| 1969 | 2002 | ssl.SSLObject(bio, bio) | |
| 1970 | 2003 | ||
| 2004 | + def test_check_hostname_requires_server_hostname(self): | ||
| 2005 | + # wrap_bio() used to accept a context asking for hostname checking | ||
| 2006 | + # without a name to check against, and then verify the certificate | ||
| 2007 | + # chain but never the peer's identity, with check_hostname still | ||
| 2008 | + # reporting True and nothing reporting the check had been skipped. | ||
| 2009 | + # It must refuse that call, as wrap_socket() already did. | ||
| 2010 | + client_context, _, hostname = testing_context() | ||
| 2011 | + self.assertTrue(client_context.check_hostname) | ||
| 2012 | + | ||
| 2013 | + for server_hostname in (None, ""): | ||
| 2014 | + with self.subTest(server_hostname=server_hostname): | ||
| 2015 | + with self.assertRaisesRegex( | ||
| 2016 | + ValueError, | ||
| 2017 | + "check_hostname requires server_hostname"): | ||
| 2018 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2019 | + server_hostname=server_hostname) | ||
| 2020 | + # The sibling constructor refuses the very same call. | ||
| 2021 | + with socket.socket() as sock: | ||
| 2022 | + with self.assertRaisesRegex( | ||
| 2023 | + ValueError, | ||
| 2024 | + "check_hostname requires server_hostname"): | ||
| 2025 | + client_context.wrap_socket( | ||
| 2026 | + sock, server_hostname=server_hostname) | ||
| 2027 | + | ||
| 2028 | + # A name was all that was missing. | ||
| 2029 | + client_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2030 | + server_hostname=hostname) | ||
| 2031 | + | ||
| 2032 | + # Asking for no hostname check remains a way to say so explicitly. | ||
| 2033 | + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) | ||
| 2034 | + context.check_hostname = False | ||
| 2035 | + self.assertFalse(context.check_hostname) | ||
| 2036 | + context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO()) | ||
| 2037 | + | ||
| 2038 | + def test_server_side_bad_params(self): | ||
| 2039 | + # A server neither sends a hostname nor resumes a client's session, | ||
| 2040 | + # so wrap_bio() rejects both in server mode like wrap_socket() | ||
| 2041 | + client_context, server_context, hostname = testing_context() | ||
| 2042 | + | ||
| 2043 | + with self.assertRaisesRegex( | ||
| 2044 | + ValueError, | ||
| 2045 | + "server_hostname can only be specified in client mode"): | ||
| 2046 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2047 | + server_side=True, | ||
| 2048 | + server_hostname=hostname) | ||
| 2049 | + | ||
| 2050 | + client, server, *_ = connected_bio_pair( | ||
| 2051 | + client_context, server_context, hostname) | ||
| 2052 | + session = client.session | ||
| 2053 | + self.assertIsNotNone(session) | ||
| 2054 | + with self.assertRaisesRegex( | ||
| 2055 | + ValueError, "session can only be specified in client mode"): | ||
| 2056 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2057 | + server_side=True, session=session) | ||
| 2058 | + | ||
| 2059 | + # Neither argument is what a server passes, so this still works. | ||
| 2060 | + server_context.wrap_bio(ssl.MemoryBIO(), ssl.MemoryBIO(), | ||
| 2061 | + server_side=True) | ||
| 2062 | + | ||
| 1971 | 2063 | def test_unwrap(self): | |
| 1972 | 2064 | client_ctx, server_ctx, hostname = testing_context() | |
| 1973 | 2065 | 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