Opened 4 years ago

Closed 4 years ago

#7693 closed enhancement (implemented)

add way to test with traffic through the tor server

Reported by: robinson Owned by: robinson
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: testing
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Stem tests should be able to set up traffic through the test tor server to stimulate specific conditions. A way to send arbitrary traffic through the SOCKS5 proxy port from Python code would be the most flexible.

Child Tickets

Change History (9)

comment:1 Changed 4 years ago by atagar

Agreed. I believe that Ravi tried to do this via his test/util.py module...

https://gitweb.torproject.org/stem.git/blob/HEAD:/test/util.py

... but better would be for stem to do a better job of supporting client use cases. On #7666 it looked like lunar is using pycurl - maybe that would be useful?

comment:2 in reply to: ↑ description Changed 4 years ago by neena

I wrote this for testing stream-handling methods. There's a method that does SOCKS negotiation, and another that gets the IP over the SOCKS negotiated circuit using a third party website.

I wrote the latter to check if the path selected was being used. (compare the exit node's IP with the IP returned by this method).

Is this sufficient? If not, this could be a good starting point.

<just noticed atagar's reply, but posting anyway>

comment:3 follow-up: Changed 4 years ago by robinson

  • Status changed from new to needs_information

I added this ticket as a way to task myself to implement something. Even better, someone else has already started or even finished it!

I'll start with your work, Ravi. Is there a particular reason you chose SOCKS4a over SOCKS5?

comment:4 in reply to: ↑ 3 Changed 4 years ago by neena

Replying to robinson:

I'll start with your work, Ravi. Is there a particular reason you chose SOCKS4a over SOCKS5?

SOCKS4a was much simpler to implement. And, I just needed this to avoid pulling in Socksipy as a dependency. It seemed sufficient at that time. Given, the only real benefit from SOCKS5 here (afaict), would be IPv6 support.

If you do need it, having this negotiate SOCKS5 would be great.

comment:5 Changed 4 years ago by robinson

  • Status changed from needs_information to needs_review

I ended up creating a new SOCKS5 handler[0]. I tried to keep our options open: Py2 and Py3 compatible, and ready for IPv6-support and authentication-support to be added. I catered to current tor abilities, only.

Before I recommend pulling this, there is a caveat: I made a change[1] to test.runner and test/settings.cfg that I am not sure are the correct approach. But, if so, I updated test.util.external_ip to use a SOCKS5 connection and my brief tests work adequately.

[0]: https://gitorious.org/stem-robinson/stem-robinson/commits/exp-socks5
[1]: https://gitorious.org/stem-robinson/stem-robinson/commit/30d882e69ddee5dcb6d09ef7b86fa270a3d49827

comment:6 follow-up: Changed 4 years ago by atagar

  • Status changed from needs_review to needs_revision

Hi Sean. Looks good, I pushed some changes to the 'exp-socks5' branch of my personal repo - thoughts?

https://gitweb.torproject.org/user/atagar/stem.git/commitdiff/c5c886bfa9229d153f0c05327a1b1ef732b276fa

I also had a few questions while reviewing this...

  • Would something like this be useful for stem users for scripting against tor (ie, should we have something like this in stem/socket.py instead)? If so then is there something that we can do about the DNS leakage that you mentioned? I'm thinking that this might be useful for #7505.
  • Can we merge in any remaining useful bits from 'test/util.py', or shall we drop external_ip()? It looks to be unused.
  • Speaking of util.py, the test that used it should probably be fixed (integ tests will now break if you give it the ONLINE target)...
atagar@morrigan:~/Desktop/stem$ grep -R "test.util" ./* | grep -v '.pyc'
./test/integ/control/controller.py:import test.util
./test/integ/control/controller.py:      test.util.negotiate_socks(s, '1.2.1.2', 80)
./test/integ/control/controller.py:      s.sendall(test.util.ip_request) # make the http request for the ip address
  • What does the trailing underscore in "type_" indicate and why is there an unused "_sock" argument? Do these come from the socket.socket's constructor definition?

Cheers! -Damian

comment:7 in reply to: ↑ 6 Changed 4 years ago by robinson

Replying to atagar:

Hi Sean. Looks good, I pushed some changes to the 'exp-socks5' branch of my personal repo - thoughts?

Thank you for your feedback. I re-worked my patch set based on your comments and I pushed it to a new branch[0].

I also had a few questions while reviewing this...

  • Would something like this be useful for stem users for scripting against tor (ie, should we have something like this in stem/socket.py instead)? If so then is there something that we can do about the DNS leakage that you mentioned? I'm thinking that this might be useful for #7505.

A) No, please resist feature creep. Stem is a Tor controller library, not a general network communications framework. For proxy use in Python, I recommend neehi[1].

B) I believe it may be possible to capture/re-direct DNS look-ups by monkey-patching socket.gethostbyname(), etc.

  • Can we merge in any remaining useful bits from 'test/util.py', or shall we drop external_ip()? It looks to be unused.

I left test/util.py alone in the new patch set. We can look at clean-up/removal after test/betwork.py is in-use.

  • Speaking of util.py, the test that used it should probably be fixed (integ tests will now break if you give it the ONLINE target)...

When I tried to fix this using "controller.get_conf()" to find the socks information, things went pear-shaped. With socks-option-related bugs in tor 0.2.2.x fixed in 0.2.3.x and option name changes I am unsure how to proceed to safely and compatibly set the socks information in the Tor test instance. I'll open a new ticket with details for this topic.

  • What does the trailing underscore in "type_" indicate and why is there an unused "_sock" argument? Do these come from the socket.socket's constructor definition?

The trailing underscore is to avoid a conflict with the built-in type function[2]. _sock is a barely documented parameter to allow (I think) socket.socket to re-use existing sockets. It's here just to provide compatibility. But, in my new patch set, I actually use it.

[0]: https://gitorious.org/stem-robinson/stem-robinson/commits/exp-socks5-v2
[1]: http://pypi.python.org/pypi/neehi I just released this as a bug-fix and modernization release of SocksiPy.
[2]: single_trailing_underscore_ @ http://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles

comment:8 Changed 4 years ago by robinson

  • Status changed from needs_revision to needs_review

When I tried to replicate the problems I found with SocksPort and SocksListenAddress for a new ticket, I could not find a problem. So, I'll work on a fix for test_mapaddress and push that to the exp-socks5-v2 branch.

comment:9 Changed 4 years ago by atagar

  • Resolution set to implemented
  • Status changed from needs_review to closed

Looks good! Pushed your exp-socks5-v2 branch.

A) No, please resist feature creep. Stem is a Tor controller library, not a general network communications framework. For proxy use in Python, I recommend neehi[1].

Agreed. Must... resist...

My thinking on this was just that client usage is going to be a common set of use cases for stem. We need to have tutorials for how to safely do it. If we can do it without additional dependencies then great, if not then that's fine too - I'm fine with suggesting something like your neehi library to our client users if you think that is the best option.

That said, shouldn't we use whatever we end up suggesting for own integ tests? I definitely like the idea of limiting stem's dependencies, but for integ testing using the library that we suggest might make sense.

I left test/util.py alone in the new patch set. We can look at clean-up/removal after test/betwork.py is in-use.

Ok. Honestly I've also been itching a bit to remove or rename test/util.py since its been throwing off my tab completion for test/unit/*. :P

Resolving. Feel free to reopen if there's something left to do on this ticket.

Note: See TracTickets for help on using tickets.