-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add secure proxy support in the client #5992
Conversation
This comment has been minimized.
This comment has been minimized.
Note of the 4 cases (http or https proxy and http or https url being requested) 3 of the 4 work. http proxy - http url - works |
Here are some notes from a discussion with @webknjaz
Things for me to try:
|
Here's an interesting post summarizing how httpcore (another asyncio based http client) is persuing resolving this encode/httpcore#254 (comment) It has details about why this doesn't work. Also here's what I believe is the only remaining fix needed for cPython: python/cpython#28073 |
Correction: 3.7.x won't have a release but 3.8 will. But master will be 4.0+ w/o a specific plan currently. |
When I try using Traceback (most recent call last):
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 23, in <module>
loop.run_until_complete(main())
File "uvloop/loop.pyx", line 1501, in uvloop.loop.Loop.run_until_complete
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 16, in main
async with session.get(url, proxy=proxy_url, verify_ssl=False) as resp:
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 1117, in __aenter__
self._resp = await self._coro
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 520, in _request
conn = await self._connector.connect(
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 535, in connect
proto = await self._create_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 890, in _create_connection
_, proto = await self._create_proxy_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 1139, in _create_proxy_connection
transport, proto = await self._wrap_create_connection(
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 969, in _wrap_create_connection
return await self._loop.create_connection(*args, **kwargs) # type: ignore # noqa
File "uvloop/loop.pyx", line 2069, in create_connection
File "uvloop/loop.pyx", line 2064, in uvloop.loop.Loop.create_connection
File "uvloop/sslproto.pyx", line 517, in uvloop.loop.SSLProtocol._on_handshake_complete
ConnectionResetError |
I've had a play around with this today and while I haven't found a solution I think I may know why the connection is being reset. This process Lines 1076 to 1124 in 0fe5b62
CONNECT request to the proxy and then duplicates the socket for the newly wrapped transport. The closing of the original transport on line 1113 seems to bring down the HTTPS context and thus _wrap_create_connection raises the ConnectionResetError . If I was to comment out the transport.close() line and change the self._wrap_create_connection to
setattr(transport, "_start_tls_compatible", True) # for https://github.com/python/cpython/pull/28073
proto = self._factory()
transport = await self._loop.start_tls(transport, proto, sslcontext, server_hostname=req.host)
proto.connection_made(transport) I am able to complete the TLS handle for a HTTPS connection over a HTTPS proxy but once the scope of the function is exited the garbage collector then closes the connection. The whole asyncio protocol and transport mechanism is something I don't have much knowledge on but there seems to be some sort of relationship between the original connection that the |
This comment has been minimized.
This comment has been minimized.
@jborean93 can you post a diff of this? I was applying it, but since it doesn't fully resolve the issue due to GC I wanted to make sure I was getting as far as you got. Thanks! |
To make testing easier I was able to reproduce curl working with secure proxy and aiohttp not working with secure proxy with:
|
By the way, |
Trustme was easier!
|
Interestingly
|
I switched my branch to import aiohttp
import asyncio
import ssl
external_url = "https://httpbin.org/get"
proxy_url = "https://localhost:8899/"
client_key = "/home/vagrant/client.pem"
async def main():
# Setup SSL Fun
ssl_ctx = ssl.create_default_context(cafile=client_key)
async with aiohttp.ClientSession(headers={"Cache-Control": "no-cache"}) as session:
async with session.get(external_url, proxy=proxy_url, ssl=ssl_ctx) as resp:
print(resp.status)
print(resp)
print(await resp.text())
loop = asyncio.get_event_loop()
loop.run_until_complete(main()) For me (with proxy.py) it produces: Traceback (most recent call last):
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 946, in _wrap_create_connection
return await self._loop.create_connection(*args, **kwargs) # type: ignore[return-value] # noqa
File "/usr/lib64/python3.9/asyncio/base_events.py", line 1081, in create_connection
transport, protocol = await self._create_connection_transport(
File "/usr/lib64/python3.9/asyncio/base_events.py", line 1112, in _create_connection_transport
await waiter
ConnectionResetError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 20, in <module>
loop.run_until_complete(main())
File "/usr/lib64/python3.9/asyncio/base_events.py", line 642, in run_until_complete
return future.result()
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 14, in main
async with session.get(external_url, proxy=proxy_url, ssl=ssl_ctx) as resp:
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 1077, in __aenter__
self._resp = await self._coro
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 479, in _request
conn = await self._connector.connect(
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 508, in connect
proto = await self._create_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 867, in _create_connection
_, proto = await self._create_proxy_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 1115, in _create_proxy_connection
transport, proto = await self._wrap_create_connection(
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 952, in _wrap_create_connection
raise client_error(req.connection_key, exc) from exc
aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host httpbin.org:443 ssl:<ssl.SSLContext object at 0x7f0fcabcc4c0> [None] |
e3446e4
to
9e119e4
Compare
This comment has been minimized.
This comment has been minimized.
So with the recent commit (e76ef53) the following repro works for me: # tls-proxy-repro.py
import aiohttp
import asyncio
url = "https://httpbin.org/get"
proxy_url = "https://localhost:8899/"
async def main():
async with aiohttp.ClientSession(
connector=aiohttp.TCPConnector(ssl=False),
headers={"Cache-Control": "no-cache"},
trust_env=True,
) as session:
async with session.get(url, proxy=proxy_url) as resp:
print(resp.status)
print(resp)
print(await resp.text())
__name__ == "__main__" and asyncio.run(main()) |
I tried this patch (e76ef53)) and your reproducer on my dev machine and also on a fresh F34 install (which has Python 3.9.7). It didn't work for me in either environment and yielded the same traceback. Traceback (most recent call last):
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 22, in <module>
__name__ == "__main__" and asyncio.run(main())
File "/usr/lib64/python3.9/asyncio/runners.py", line 44, in run
return loop.run_until_complete(main)
File "/usr/lib64/python3.9/asyncio/base_events.py", line 642, in run_until_complete
return future.result()
File "/home/vagrant/devel/aiohttp_proxy_test.py", line 16, in main
async with session.get(url, proxy=proxy_url) as resp:
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 1077, in __aenter__
self._resp = await self._coro
File "/home/vagrant/devel/aiohttp/aiohttp/client.py", line 479, in _request
conn = await self._connector.connect(
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 508, in connect
proto = await self._create_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 867, in _create_connection
_, proto = await self._create_proxy_connection(req, traces, timeout)
File "/home/vagrant/devel/aiohttp/aiohttp/connector.py", line 1125, in _create_proxy_connection
tls_transport.get_protocol().connection_made(
AttributeError: 'NoneType' object has no attribute 'get_protocol' |
That's weird. I wonder in what cases |
The issue is the aiohttp/aiohttp/client_reqrep.py Line 846 in 0fe5b62
FIN once the CONNECT response is processed.
|
I did receive the 200 OK response and the proxy shows a payload being returned correctly when I apply this patch to the 3.7 branch and commenting out the python ~/devel/aiohttp_proxy_test.py
200
<ClientResponse(https://httpbin.org/get) [200 OK]>
<CIMultiDictProxy('Date': 'Mon, 13 Sep 2021 15:19:07 GMT', 'Content-Type': 'application/json', 'Content-Length': '349', 'Connection': 'keep-alive', 'Server': 'gunicorn/19.9.0', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Credentials': 'true')>
{
"args": {},
"headers": {
"Accept": "*/*",
"Accept-Encoding": "gzip, deflate",
"Cache-Control": "no-cache",
"Host": "httpbin.org",
"User-Agent": "Python/3.9 aiohttp/3.7.4.post0",
"X-Amzn-Trace-Id": "Root=1-613f6beb-3790eb756b11edfb70c500a5"
},
"origin": "174.99.22.188",
"url": "https://httpbin.org/get"
}
Fatal error on SSL transport
protocol: <asyncio.sslproto.SSLProtocol object at 0x7ff27bce1430>
transport: <_SelectorSocketTransport closing fd=6>
Traceback (most recent call last):
File "/usr/lib64/python3.9/asyncio/selector_events.py", line 918, in write
n = self._sock.send(data)
OSError: [Errno 9] Bad file descriptor
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/lib64/python3.9/asyncio/sslproto.py", line 684, in _process_write_backlog
self._transport.write(chunk)
File "/usr/lib64/python3.9/asyncio/selector_events.py", line 924, in write
self._fatal_error(exc, 'Fatal write error on socket transport')
File "/usr/lib64/python3.9/asyncio/selector_events.py", line 719, in _fatal_error
self._force_close(exc)
File "/usr/lib64/python3.9/asyncio/selector_events.py", line 731, in _force_close
self._loop.call_soon(self._call_connection_lost, exc)
File "/usr/lib64/python3.9/asyncio/base_events.py", line 746, in call_soon
self._check_closed()
File "/usr/lib64/python3.9/asyncio/base_events.py", line 510, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed So what is the right way to solve this? What can be done? |
e76ef53
to
6bb910d
Compare
When testing offline changing https://github.com/aio-libs/aiohttp/pull/5992/files#diff-c143bd72e7f6748d879e5a0466a2872e05dc2d0b4a1157e9221702cc3c5516bbR1124 to the below was able to delay the connection being closed. protocol.set_response_params(read_until_eof=True) This works as it switches the behaviour in the raw HttpPayloadParser to not return a |
This comment has been minimized.
This comment has been minimized.
c336e66
to
6d95407
Compare
6d95407
to
995136e
Compare
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * https://bugs.python.org/issue37179 * python/cpython#28073 * https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * aio-libs#6044 Resolves aio-libs#3816 Resolves aio-libs#4268 Co-Authored-By: Brian Bouterse <[email protected]> Co-Authored-By: Jordan Borean <[email protected]> Co-Authored-By: Sviatoslav Sydorenko <[email protected]>
995136e
to
3cb2979
Compare
Backport to 3.8: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply c29e5fb on top of patchback/backports/3.8/c29e5fb58efb65c6198ea75b95805ab972e98adc/pr-5992 Backporting merged PR #5992 into master
🤖 @patchback |
💔 Backport was not successfulThe PR was attempted backported to the following branches:
|
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * https://bugs.python.org/issue37179 * python/cpython#28073 * https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * aio-libs#6044 PR aio-libs#5992 Resolves aio-libs#3816 Resolves aio-libs#4268 Co-authored-by: Brian Bouterse <[email protected]> Co-authored-by: Jordan Borean <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]> (cherry picked from commit c29e5fb)
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * https://bugs.python.org/issue37179 * python/cpython#28073 * https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * aio-libs#6044 PR aio-libs#5992 Resolves aio-libs#3816 Resolves aio-libs#4268 Co-authored-by: Brian Bouterse <[email protected]> Co-authored-by: Jordan Borean <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]> (cherry picked from commit c29e5fb)
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * https://bugs.python.org/issue37179 * python/cpython#28073 * https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * aio-libs#6044 PR aio-libs#5992 Resolves aio-libs#3816 Resolves aio-libs#4268 Co-authored-by: Brian Bouterse <[email protected]> Co-authored-by: Jordan Borean <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]> (cherry picked from commit c29e5fb)
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * https://bugs.python.org/issue37179 * python/cpython#28073 * https://docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * aio-libs#6044 PR aio-libs#5992 Resolves aio-libs#3816 Resolves aio-libs#4268 Co-authored-by: Brian Bouterse <[email protected]> Co-authored-by: Jordan Borean <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]> (cherry picked from commit c29e5fb)
…ent (#6049) This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies. The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for `asyncio` with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box. Currently the tests monkey-patch `asyncio` in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated. Refs: * bugs.python.org/issue37179 * python/cpython#28073 * docs.aiohttp.org/en/stable/client_advanced.html#proxy-support * #6044 PR #5992 Resolves #3816 Resolves #4268 Co-authored-by: Brian Bouterse <[email protected]> Co-authored-by: Jordan Borean <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]>
What do these changes do?
This patch opens up the code path and adds the implementation that allows end-users to start sending HTTPS requests through HTTPS proxies.
The support for TLS-in-TLS (needed for this to work) in the stdlib is kinda available since Python 3.7 but is disabled for
asyncio
with an attribute/flag/toggle. When the upstream CPython enables it finally, aiohttp v3.8+ will be able to work with it out of the box.Currently the tests monkey-patch
asyncio
in order to verify that this works. The users who are willing to do the same, will be able to take advantage of it right now. Eventually (hopefully starting Python 3.11), the need for monkey-patching should be eliminated.Refs:
Original details
I test with this script:
And I get
The squid proxy does show
1631129861.057 45 ::1 TCP_TUNNEL/200 39 CONNECT example.com:443 - HIER_DIRECT/93.184.216.34 -
and I can see the 200 OK from the direct connection.What fails is when taking the TCP socket from the direct connection and TLS wrapping it I get a
ConnectionResetError
.Are there changes in behavior for the user?
They get the ability to send out HTTPS queries through HTTPS proxies if they monkey-patch the stdlib
asyncio
.Related issue number
Resolves #3816
Resolves #4268
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.