Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 191
Add support for synchronous TLS-in-TLS connections.#732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
5d01fac9015e03a2ec2da81bcf16ab232f973e00b58cb7324d3acf7f52b282254f23105727312e31572e54275555f73c52ae51284ca37fccc4e9b629fe3ea6ac8ee56f9e0d4b6db047f98767013fcb610d87a9010e1eae48e38d22bb8726186e5e70ebd79905307e0c1adc291f156785a268f2253f45c4358f07f968ed16be8dd627b9b019aa784eb3d952caf80e1b080c34f747a8c2a2178d271aeda0507818c47324ddc0ff81e3f68e76892041d49d74b6aFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,6 +2,8 @@ | ||
| import ssl | ||
| import sys | ||
| import typing | ||
| from functools import partial | ||
| from time import perf_counter | ||
| from .._exceptions import ( | ||
| ConnectError, | ||
| @@ -17,6 +19,117 @@ | ||
| from .base import SOCKET_OPTION, NetworkBackend, NetworkStream | ||
| class SyncTLSStream(NetworkStream): | ||
| """ | ||
| Because the standard `SSLContext.wrap_socket` method does | ||
| not work for `SSLSocket` objects, we need this class | ||
| to implement TLS stream using an underlying `SSLObject` | ||
| instance in order to support TLS on top of TLS. | ||
| """ | ||
| # Defined in RFC 8449 | ||
| TLS_RECORD_SIZE = 16384 | ||
| def __init__( | ||
| self, | ||
| sock: socket.socket, | ||
| ssl_context: ssl.SSLContext, | ||
| server_hostname: typing.Optional[str] = None, | ||
| timeout: typing.Optional[float] = None, | ||
| ): | ||
| self._sock = sock | ||
| self._incoming = ssl.MemoryBIO() | ||
| self._outgoing = ssl.MemoryBIO() | ||
| self.ssl_obj = ssl_context.wrap_bio( | ||
| incoming=self._incoming, | ||
| outgoing=self._outgoing, | ||
| server_hostname=server_hostname, | ||
| ) | ||
| self._perform_io(self.ssl_obj.do_handshake, timeout) | ||
| def _perform_io( | ||
| self, | ||
| func: typing.Callable[..., typing.Any], | ||
| timeout: typing.Optional[float], | ||
| ) -> typing.Any: | ||
| ret = None | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| timeout = timeout or None # Replaces `0` with `None` | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to distinguish between cases where we got 0 after decreasing our timeout and cases where we don't want to handle timeout at all. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if given paramter's value was | ||
| while True: | ||
| errno = None | ||
| try: | ||
| ret = func() | ||
| except (ssl.SSLWantReadError, ssl.SSLWantWriteError) as e: | ||
| errno = e.errno | ||
| if timeout is not None and timeout <= 0: # pragma: no cover | ||
| raise socket.timeout() | ||
| self._sock.settimeout(timeout) | ||
| operation_start = perf_counter() | ||
| self._sock.sendall(self._outgoing.read()) | ||
| # If the timeout is `None`, don't touch it. | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| timeout = timeout and timeout - (perf_counter() - operation_start) | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if errno == ssl.SSL_ERROR_WANT_READ: | ||
| if timeout is not None and timeout <= 0: # pragma: no cover | ||
| raise socket.timeout() | ||
| self._sock.settimeout(timeout) | ||
| operation_start = perf_counter() | ||
| buf = self._sock.recv(self.TLS_RECORD_SIZE) | ||
| # If the timeout is `None`, don't touch it. | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| timeout = timeout and timeout - (perf_counter() - operation_start) | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if buf: | ||
| self._incoming.write(buf) | ||
| else: | ||
| self._incoming.write_eof() # pragma: no cover | ||
| if errno is None: | ||
| return ret | ||
| def read(self, max_bytes: int, timeout: typing.Optional[float] = None) -> bytes: | ||
| exc_map: ExceptionMapping = {socket.timeout: ReadTimeout, OSError: ReadError} | ||
| with map_exceptions(exc_map): | ||
| return typing.cast( | ||
| bytes, self._perform_io(partial(self.ssl_obj.read, max_bytes), timeout) | ||
| ) | ||
| def write(self, buffer: bytes, timeout: typing.Optional[float] = None) -> None: | ||
| exc_map: ExceptionMapping = {socket.timeout: WriteTimeout, OSError: WriteError} | ||
| with map_exceptions(exc_map): | ||
| while buffer: | ||
| nsent = self._perform_io(partial(self.ssl_obj.write, buffer), timeout) | ||
| buffer = buffer[nsent:] | ||
| def close(self) -> None: | ||
| self._sock.close() | ||
| def start_tls( | ||
| self, | ||
| ssl_context: ssl.SSLContext, | ||
| server_hostname: typing.Optional[str] = None, | ||
| timeout: typing.Optional[float] = None, | ||
| ) -> "NetworkStream": | ||
| raise NotImplementedError() # pragma: no cover | ||
| def get_extra_info(self, info: str) -> typing.Any: # pragma: no cover | ||
| if info == "ssl_object": | ||
| return self.ssl_obj | ||
| if info == "client_addr": | ||
| return self._sock.getsockname() | ||
| if info == "server_addr": | ||
| return self._sock.getpeername() | ||
| if info == "socket": | ||
| return self._sock | ||
| if info == "is_readable": | ||
| return is_socket_readable(self._sock) | ||
| return None | ||
| class SyncStream(NetworkStream): | ||
| def __init__(self, sock: socket.socket) -> None: | ||
| self._sock = sock | ||
| @@ -53,10 +166,18 @@ def start_tls( | ||
| } | ||
| with map_exceptions(exc_map): | ||
| try: | ||
| self._sock.settimeout(timeout) | ||
| sock = ssl_context.wrap_socket( | ||
| self._sock, server_hostname=server_hostname | ||
| ) | ||
| if isinstance(self._sock, ssl.SSLSocket): | ||
karpetrosyan marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| # If the underlying socket has already been upgraded | ||
| # to the TLS layer (i.e. is an instance of SSLSocket), | ||
| # we want to use another stream object that supports TLS-in-TLS. | ||
| return SyncTLSStream( | ||
| self._sock, ssl_context, server_hostname, timeout | ||
| ) | ||
| else: | ||
| self._sock.settimeout(timeout) | ||
| sock = ssl_context.wrap_socket( | ||
| self._sock, server_hostname=server_hostname | ||
| ) | ||
| except Exception as exc: # pragma: nocover | ||
| self.close() | ||
| raise exc | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| import pytest | ||
| import httpcore | ||
| READ_TIMEOUT = 2 | ||
| WRITE_TIMEOUT = 2 | ||
| CONNECT_TIMEOUT = 2 | ||
| @pytest.mark.anyio | ||
| async def test_connect_without_tls(tcp_server): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tcp_server.host, tcp_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| await stream.aclose() | ||
| @pytest.mark.anyio | ||
| async def test_write_without_tls(tcp_server): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tcp_server.host, tcp_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| await stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| @pytest.mark.anyio | ||
| async def test_read_without_tls(tcp_server): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tcp_server.host, tcp_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| await stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| await stream.read(1024, timeout=READ_TIMEOUT) | ||
| @pytest.mark.anyio | ||
| async def test_connect_with_tls(tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_server.host, tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| await tls_stream.aclose() | ||
| @pytest.mark.anyio | ||
| async def test_write_with_tls(tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_server.host, tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with tls_stream: | ||
| await tls_stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| @pytest.mark.anyio | ||
| async def test_read_with_tls(tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_server.host, tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with tls_stream: | ||
| await tls_stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| await tls_stream.read(1024, timeout=READ_TIMEOUT) | ||
lovelydinosaur marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| @pytest.mark.anyio | ||
| async def test_connect_with_tls_in_tls(tls_in_tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_in_tls_server.host, tls_in_tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| async with tls_stream: | ||
| tls_in_tls_stream = await tls_stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| await tls_in_tls_stream.aclose() | ||
| @pytest.mark.anyio | ||
| async def test_write_with_tls_in_tls(tls_in_tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_in_tls_server.host, tls_in_tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| async with tls_stream: | ||
| tls_in_tls_stream = await tls_stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| async with tls_in_tls_stream: | ||
| await tls_in_tls_stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| @pytest.mark.anyio | ||
| async def test_read_with_tls_in_tls(tls_in_tls_server, client_context): | ||
| backend = httpcore.AnyIOBackend() | ||
| stream = await backend.connect_tcp( | ||
| tls_in_tls_server.host, tls_in_tls_server.port, timeout=CONNECT_TIMEOUT | ||
| ) | ||
| async with stream: | ||
| tls_stream = await stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| async with tls_stream: | ||
| tls_in_tls_stream = await tls_stream.start_tls( | ||
| ssl_context=client_context, | ||
| server_hostname="localhost", | ||
| timeout=CONNECT_TIMEOUT, | ||
| ) | ||
| async with tls_in_tls_stream: | ||
| await tls_in_tls_stream.write(b"ping", timeout=WRITE_TIMEOUT) | ||
| await tls_in_tls_stream.read(1024, timeout=READ_TIMEOUT) | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the
typing_extensions.Selfhere. (?)Can we just have this return
NetworkStream.The override point is
close(), not the__enter__/__exit__which will stay the same even for subclasses.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can inherit, I believe we should always use Self to avoid strange type issues, such as when the instance of SlowNetworkStream is NetworkStream.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example:
OUTPUT test.py:11: note: Revealed type is "httpcore._backends.base.NetworkStream"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay right.
Could we use the
TypeVarstyle, then?Eg... in
httpxhttps://github.com/encode/httpx/blob/76c9cb65f2a159adb764c2236d139f85b46e1506/httpx/_client.py#L60
https://github.com/encode/httpx/blob/76c9cb65f2a159adb764c2236d139f85b46e1506/httpx/_client.py#L1263
Really prefer us avoiding introducing new third party packages wherever possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/encode/httpcore/blob/e31572e0371557d6163d2b7c28676e2b1727673b/httpcore/_backends/base.py
Initially, we used TypeVar, but it was too complicated.