python-trio / python-trio/trio
Should we have a way to block open_tcp_stream from connecting to certain hosts?
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 7.3k
- Forks
- 431
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 6
Description
I was reading https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-New-Era-Of-SSRF-Exploiting-URL-Parser-In-Trending-Programming-Languages.pdf , and there are a few examples that follow this pattern:
- you have some program running behind your firewall (e.g., the code running Github, or Wordpress)
- it wants to make an outgoing HTTP request, but the destination URL is at least somewhat under the control of an untrusted user (e.g., Github webhooks)
- you want to make sure that user doesn't direct the request at some sensitive behind-the-firewall service and pwn you
- so you have some check, like you forbid any URL that says
localhostin it - but doing this correctly is extremely tricky, so everyone screws it up, and they get pwned.
The only correct place to do this check is after you've done the final conversion to a (normalized) IP address, and even there it has subtleties (e.g. sock.connect(0.0.0.0, port) seems to connect to localhost for me, and ipv4-mapped addresses are a thing that someone could potentially put in an AAAA record if they wanted to be perverse). If you're using open_tcp_stream – and we'd hope that HTTP clients on top of trio will use open_tcp_stream! – then that means that the checking has to happen inside open_tcp_stream.
One option would be to provide a forbidden_targets=... argument, that gives a list of ipaddress.IPv{4,6}Network objects, and for each resolved address checks if they fall into any of the given networks, and raises an error if so. The ipaddress module also provides mechanisms to check if an address is private, reserved, loopback, etc., which might be useful here.
Or I guess we could be even more minimal, and let the user pass in a validate_target=... function, that gets called on all the targets, and can raise an error or return False or whatever to indicate a bad target.
We should check with urllib3/requests maintainers whether this is something they would actually use.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the open_tcp_stream API and its address-resolution and connection flow. Check whether urllib3 or requests maintainers would use forbidden_targets or validate_target, then define the accepted validation behavior and error handling before implementation; done means a decided API that safely checks every resolved target.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100