Facilitator should not give client registrations to Tor exits

added component::archived/flashproxy owner::jct priority::medium resolution::wontfix status::closed type::enhancement labels

Trac:
Username: jct

detect_proxy_within_tor_in_tor_situation.patch

A patch with the candidate code in order to detect that the public address of the Proxy belongs to a TOR Exit node

Trac:
Username: jct

I just attached a patch with a candidate code in order to solve the ticket. The main idea for the patch is the following:

A thread is created by the Facilitator in order to maintain a locally cached database of exits:
- The locally cached database is managed by the class '''TorExitsSet'''
- The thread is running the function '''get_exit_nodes''' in order to get a list of Tor Exit nodes
  - The function is querying by HTTP to a Tor Directory Server in order to get the list
- The thread is running each '''ELAPSED_REFRESHING_TIME''' seconds
The function do_GET was modified in order to check if the Proxy address is belonging to a Tor Exit node:
- In this case not Client/Relay address is sent to the Proxy, but an error signalling the situation:
  - The flashproxy.js was modified in order to disable the Proxy when an error is received:
    - If the Proxy is malicious it could avoid to disable itself, but the benign ones are disabling themselves (hopefully)

Trac:
Username: jct

Replying to jct:

I just attached a patch with a candidate code in order to solve the ticket.

This looks pretty good, nice job.

I don't want the "disable the proxy" thing to be a part of this ticket. Please assume that flashproxy.js will be unchanged and send back an empty reply, not an error message.

I would like to add as little extra complexity to the facilitator process as possible. I don't like having the facilitator itself doing background HTTP requests and parsing of directory documents. I would rather have a completely separate process polling for the exit list, and either writing the list to a file shared with the facilitator, or else operating as a daemon that the facilitator can query. Now that I think about it, I kind of like the daemon idea: it receives a single line containing an address, and writes back a byte that indicates whether the address given is an exit. The only change needed in the facilitator will be a function that knows how to query this local daemon. Failure to connect to this local daemon would not be a fatal error.

I'm not comfortable with the method of getting the exit list. I don't know much about how the directories work, but I think you are supposed to make an authenticated connection to them, and take a consensus among many of them. I'm guessing that the right way to do this is to run a Tor instance (with no listening ports) on the same host as the facilitator, just for the sake of retrieving a network consensus. See if there is a way to easily export this data in a form that the exit daemon can use.

Trac:
Status: new to needs_revision

Working on that!

Trac:
Username: jct

test.py

Testing the connection to a Tor instance running in the same host as the Facilitator in order to get a list of Tor Exit nodes.

Trac:
Username: jct

The attached test.py has a proof of concept of a process connecting with a Tor instance through the Tor Control Protocol. Here the process is connecting to the Tor instance in order to ask for the router status info for all the routers that this Tor instance have an opinion about. Once the process is getting the list, it is filtering the list in order to only get the Tor Exit nodes (the good and the bad ones).

The Tor instance configuration must be similar to the follow:

ControlPort 64000
HashedControlPassword 16:17867273713930AB60BF206385EA5CFB1E71844AA6AE0F4DEFA3006579
SocksListenAddress 127.0.0.1
SocksPort 0

The ControlPort is where the process must connect in order to ask for the routers status info through the Tor Control Protocol.

The HashedControlPassword must have the salt hash for the password used by the process in order to authenticate with the Tor instance. This hash is generated running the following command (where the string aa is the password used by the process):

 tor --hash-password aa

The SocksPort must be '0' in order to have a Tor instance with not listening ports (with the exception of the Control Port).

If you are agree with the previous solution, then I'm implementing the exit daemon as a threaded TCP server that is communicating with a local Tor instance in that way.

Trac:
Username: jct

Thank you for making this test. Your approach looks like a good one. I think it will be more effective if you use an existing library like Stem or TorCtl to interact with the control port.

https://trac.torproject.org/projects/tor/wiki/doc/stem https://stem.readthedocs.org/en/latest/index.html

http://fscked.org/projects/torctl

I haven't used either of these, but perhaps there is a way to look up a relay by IP address, and then check its exit policy. If so, that is easy enough that it can happen in a blocking way within the facilitator, and there is no need for a separate daemon. If it is only possible to get a big list of relays all at once, then having a separate daemon might still make sense.

Maybe use an hourly updated JSON file - https://onionoo.torproject.org/details to search for exit relays and their exit policies.

See tor-dev discussion about obtaining the exit list.

Trac:
Username: jct

fac-onionoo.zip

Threaded TCP server to answer about IP's belonging to Tor Exit nodes.

Trac:
Username: jct

The attached fac-onionoo.zip is a proof of concept of the following:

It is a threaded TCP server that is capable of:

Query an Onionoo server in order to build a local database of Tor Exit nodes
Answer queries from a client (mainly the Facilitator) about if a given IP belongs to a Tor Exit node or not.

The main module is the new module fac-onionoo and it is using the new modules base_server.py and custom_ssl.py. It is also using the old module fac.py, but with the new function fac.ip_ver.

Comments about the modules:

The module base_server.py is a kind of "factor out" from the module facilitator
- I've extracted from the last the base code for a Threaded TCP server:
  - If the '''base_server.py''' is accepted, then it will be needed to refactor '''facilitator''' in order to use it (but that isn't difficult).
The module custom_ssl.py has the code needed to use HTTPS in a secure way, cause there is an issue with Python < 3 using the standard SSL module:
- The module has a dependency with the module '''certifi'''. It was the easiest way to have a list of root certificates.
The module fac_onionoo has two roles:
- Mantain a local database of Tor Exit nodes:
  - It uses the '''Onionoo''' project in order to build this database:
    - It wasn't possible to use the simple query https://onionoo.torproject.org/summary?type=relay&running=true (https://lists.torproject.org/pipermail/tor-dev/2012-December/004235.html), cause in this case I'm receiving all the relays and I don't have enough information if it is an Exit node or not (according the specification the addresses returned are "IPv4 or IPv6 addresses where the relay accepts onion-routing connections or which the relay used to exit to the Internet"). For this reason I was needing to retrieve the '''Details documents''' and calculate from the '''exit_policy''' array if the relay is an Exit node or not. In order to do that, I was using the Stem library (it already has a function in order to calculate that -> https://stem.readthedocs.org).
- The second role is to answer queries asking if a given IP belongs to a Tor Exit node:
  - The server defines a protocol for that and the module '''fac-onionoo-test''' is showing the client function ('''ask_is_exit''') that is using this protocol in order to query the server:
    - This function should be located later at the '''fac.py''' module and it will be used from the Facilitator in order to check if a Proxy 's IP belongs to a Tor Exit node or not.

The module fac-onionoo-test is a small test that is showing how is working the server. It is a bit slow cause the query to the Onionoo server takes time (it is using HTTPS and also is processing a lot of data). Also the test isn't very automatic, cause it is possible that the IP for the known Tor Exit (I've extracted it using the TBB) is changing (or down, etc.).

Trac:
Username: jct

fac-onionoo2.zip

Second generation of a Threaded TCP server to answer about IP's belonging to Tor Exit nodes.

Trac:
Username: jct

The attached fac-onionoo2.zip is more or less the same that fac-onionoo.zip, with some improvements:

The query to the Onionoo server was 'factored out' in the new module query_onionoo.py
- The function '''query_onionoo.query_onionoo_server''' has a new flag to detect if is running in a test mode:
  - This is implemented cause the Onionoo server protocol at the moment is very unstable, so the new flag it is useful to test the Onionoo server protocol.
The class fac-onionoo.Handler that is implementing the 'fac-onionoo' protocol with its clients was improved in order to give more information.
The function fac-onionoo.update_local_db was improved in order to be more robust to errors.
The module fac-onionoo-test has new tests and has improved the old tests with the goal of covering as much as possible of the implemented functionality.

Trac:
Username: jct

Replying to jct:

The attached fac-onionoo2.zip is more or less the same that fac-onionoo.zip, with some improvements:

The query to the Onionoo server was 'factored out' in the new module query_onionoo.py

The function query_onionoo.query_onionoo_server has a new flag to detect if is running in a test mode:

This is implemented cause the Onionoo server protocol at the moment is very unstable, so the new flag it is useful to test the Onionoo server protocol.

The class fac-onionoo.Handler that is implementing the 'fac-onionoo' protocol with its clients was improved in order to give more information.

The function fac-onionoo.update_local_db was improved in order to be more robust to errors.

The module fac-onionoo-test has new tests and has improved the old tests with the goal of covering as much as possible of the implemented functionality.

You have the right idea about verifying the TLS connection to onionoo. However we already solve this problem in other flash proxy programs in a different way, so you should use the same way. This certifi module is not present in Debian testing, so I can't try it easily. Check flashproxy-email-poller for an example. Use the M2Crypto library. Don't do certificate pinning. Use the default trusted cert list built into OpenSSL through M2Crypto.

I don't like the factoring of base_server.py. I would prefer cut-and-paste code to premature generalization. There's no reason to make the exit daemon use the same protocol as the facilitator. It should take just an IP address on a single line as input, not a CHECK_IP command.

Please get rid of the ip_ver function. I think that function is misguided. The patch doesn't use the function of returning the address family, which is what distinguishes it from parse_addr_spec.

Don't make random network connections in tests:

response = opener.open("https://www.google.es")
self.assertRaises(custom_ssl.InvalidCertificateException, opener.open, "https://74.125.232.50")

I don't want a dependency on stem. I don't think you need to parse the exit policy anyway. Just consider a relay an exit if its exit policy is not exactly ["reject :"]. I much prefer simplicity over elimination of false positives.

In general, you should seek to simplify your changes. There really should be only one added file: the source code of the onionoo-querying daemon. Other files will need to be patched. Add tests to facilitator-test rather than making new test files.

I think you should start by writing only the onionoo-querying daemon. That should be able to stand alone without any other changes. The whole program should only be about 200 lines, I estimate; if it's much more than that you might be doing something wrong. The database should not be a complicated data structure: only a set with a lock around it. As you write the code, put yourself in my position: I want to read and understand it, so make as few changes as possible (especially architectural changes) and make your purpose clear. Break big changes into a series of smaller logical changes.

Replying to dcf:

Replying to jct:

The attached fac-onionoo2.zip is more or less the same that fac-onionoo.zip, with some improvements:

The query to the Onionoo server was 'factored out' in the new module query_onionoo.py

The function query_onionoo.query_onionoo_server has a new flag to detect if is running in a test mode:

This is implemented cause the Onionoo server protocol at the moment is very unstable, so the new flag it is useful to test the Onionoo server protocol.

The class fac-onionoo.Handler that is implementing the 'fac-onionoo' protocol with its clients was improved in order to give more information.

The function fac-onionoo.update_local_db was improved in order to be more robust to errors.

The module fac-onionoo-test has new tests and has improved the old tests with the goal of covering as much as possible of the implemented functionality.

You have the right idea about verifying the TLS connection to onionoo. However we already solve this problem in other flash proxy programs in a different way, so you should use the same way. This certifi module is not present in Debian testing, so I can't try it easily. Check flashproxy-email-poller for an example. Use the M2Crypto library. Don't do certificate pinning. Use the default trusted cert list built into OpenSSL through M2Crypto.

I don't like the factoring of base_server.py. I would prefer cut-and-paste code to premature generalization. There's no reason to make the exit daemon use the same protocol as the facilitator. It should take just an IP address on a single line as input, not a CHECK_IP command.

Please get rid of the ip_ver function. I think that function is misguided. The patch doesn't use the function of returning the address family, which is what distinguishes it from parse_addr_spec.

Don't make random network connections in tests: {{{ response = opener.open("https://www.google.es") self.assertRaises(custom_ssl.InvalidCertificateException, opener.open, "https://74.125.232.50") }}}

I don't want a dependency on stem. I don't think you need to parse the exit policy anyway. Just consider a relay an exit if its exit policy is not exactly ["reject :"]. I much prefer simplicity over elimination of false positives.

In general, you should seek to simplify your changes. There really should be only one added file: the source code of the onionoo-querying daemon. Other files will need to be patched. Add tests to facilitator-test rather than making new test files.

I think you should start by writing only the onionoo-querying daemon. That should be able to stand alone without any other changes. The whole program should only be about 200 lines, I estimate; if it's much more than that you might be doing something wrong. The database should not be a complicated data structure: only a set with a lock around it. As you write the code, put yourself in my position: I want to read and understand it, so make as few changes as possible (especially architectural changes) and make your purpose clear. Break big changes into a series of smaller logical changes.

Working on that!

Trac:
Username: jct

Replying to dcf:

Replying to jct:

The attached fac-onionoo2.zip is more or less the same that fac-onionoo.zip, with some improvements:

The query to the Onionoo server was 'factored out' in the new module query_onionoo.py

The function query_onionoo.query_onionoo_server has a new flag to detect if is running in a test mode:

This is implemented cause the Onionoo server protocol at the moment is very unstable, so the new flag it is useful to test the Onionoo server protocol.

The class fac-onionoo.Handler that is implementing the 'fac-onionoo' protocol with its clients was improved in order to give more information.

The function fac-onionoo.update_local_db was improved in order to be more robust to errors.

The module fac-onionoo-test has new tests and has improved the old tests with the goal of covering as much as possible of the implemented functionality.

You have the right idea about verifying the TLS connection to onionoo. However we already solve this problem in other flash proxy programs in a different way, so you should use the same way. This certifi module is not present in Debian testing, so I can't try it easily. Check flashproxy-email-poller for an example. Use the M2Crypto library. Don't do certificate pinning. Use the default trusted cert list built into OpenSSL through M2Crypto.

I don't like the factoring of base_server.py. I would prefer cut-and-paste code to premature generalization. There's no reason to make the exit daemon use the same protocol as the facilitator. It should take just an IP address on a single line as input, not a CHECK_IP command.

Please get rid of the ip_ver function. I think that function is misguided. The patch doesn't use the function of returning the address family, which is what distinguishes it from parse_addr_spec.

Don't make random network connections in tests: {{{ response = opener.open("https://www.google.es") self.assertRaises(custom_ssl.InvalidCertificateException, opener.open, "https://74.125.232.50") }}}

I don't want a dependency on stem. I don't think you need to parse the exit policy anyway. Just consider a relay an exit if its exit policy is not exactly ["reject :"]. I much prefer simplicity over elimination of false positives.

In general, you should seek to simplify your changes. There really should be only one added file: the source code of the onionoo-querying daemon. Other files will need to be patched. Add tests to facilitator-test rather than making new test files.

I think you should start by writing only the onionoo-querying daemon. That should be able to stand alone without any other changes. The whole program should only be about 200 lines, I estimate; if it's much more than that you might be doing something wrong. The database should not be a complicated data structure: only a set with a lock around it. As you write the code, put yourself in my position: I want to read and understand it, so make as few changes as possible (especially architectural changes) and make your purpose clear. Break big changes into a series of smaller logical changes.

I'm attaching a prototype for the onionoo-querying daemon that is working standalone. It is following more or less your previous suggestions. There is also an independent test for the standalone server. The test is using a random network connection (to an Exit Node), but it is there only at the prototype stage.

Trac:
Username: jct

onionoo-querying.zip

Protototype for the standalone onionoo-querying daemon

Trac:
Username: jct

Replying to jct:

Replying to dcf:

Replying to jct:

The attached fac-onionoo2.zip is more or less the same that fac-onionoo.zip, with some improvements:

The query to the Onionoo server was 'factored out' in the new module query_onionoo.py

The function query_onionoo.query_onionoo_server has a new flag to detect if is running in a test mode:

This is implemented cause the Onionoo server protocol at the moment is very unstable, so the new flag it is useful to test the Onionoo server protocol.

The class fac-onionoo.Handler that is implementing the 'fac-onionoo' protocol with its clients was improved in order to give more information.

The function fac-onionoo.update_local_db was improved in order to be more robust to errors.

The module fac-onionoo-test has new tests and has improved the old tests with the goal of covering as much as possible of the implemented functionality.

You have the right idea about verifying the TLS connection to onionoo. However we already solve this problem in other flash proxy programs in a different way, so you should use the same way. This certifi module is not present in Debian testing, so I can't try it easily. Check flashproxy-email-poller for an example. Use the M2Crypto library. Don't do certificate pinning. Use the default trusted cert list built into OpenSSL through M2Crypto.

I don't like the factoring of base_server.py. I would prefer cut-and-paste code to premature generalization. There's no reason to make the exit daemon use the same protocol as the facilitator. It should take just an IP address on a single line as input, not a CHECK_IP command.

Please get rid of the ip_ver function. I think that function is misguided. The patch doesn't use the function of returning the address family, which is what distinguishes it from parse_addr_spec.

Don't make random network connections in tests: {{{ response = opener.open("https://www.google.es") self.assertRaises(custom_ssl.InvalidCertificateException, opener.open, "https://74.125.232.50") }}}

I don't want a dependency on stem. I don't think you need to parse the exit policy anyway. Just consider a relay an exit if its exit policy is not exactly ["reject :"]. I much prefer simplicity over elimination of false positives.

In general, you should seek to simplify your changes. There really should be only one added file: the source code of the onionoo-querying daemon. Other files will need to be patched. Add tests to facilitator-test rather than making new test files.

I think you should start by writing only the onionoo-querying daemon. That should be able to stand alone without any other changes. The whole program should only be about 200 lines, I estimate; if it's much more than that you might be doing something wrong. The database should not be a complicated data structure: only a set with a lock around it. As you write the code, put yourself in my position: I want to read and understand it, so make as few changes as possible (especially architectural changes) and make your purpose clear. Break big changes into a series of smaller logical changes.

I'm attaching a prototype for the onionoo-querying daemon that is working standalone. It is following more or less your previous suggestions. There is also an independent test for the standalone server. The test is using a random network connection (to an Exit Node), but it is there only at the prototype stage.

I'm attaching a new version with two changes:

In onionoo-querying the internal structure for TorExitNodesSet was changed from a list to a dictionary. In this way the IP lookup 's time complexity has changed from O(n) to O(1)
In onionoo-querying-test was incremented the time in order to wait for the onionoo-querying daemon initialization, and thus avoid an error message of "connection refused".

Trac:
Username: jct

onionoo-querying-ver2.zip

Protototype for the standalone onionoo-querying daemon

Trac:
Username: jct

Replying to jct:

In onionoo-querying the internal structure for TorExitNodesSet was changed from a list to a dictionary. In this way the IP lookup 's time complexity has changed from O(n) to O(1)

In onionoo-querying-test was incremented the time in order to wait for the onionoo-querying daemon initialization, and thus avoid an error message of "connection refused".

Note to self about what this program does. It periodically refreshes a local database of possible exits (IP addresses of relays whose exit policy is not reject *:*). It listens on localhost port 9003. On receiving a connection, it reads an IP address followed by a newline. If the IP address is a possible exit, it sends the string "EXIT". If not, it sends "OK_num", where num is the number of exits in the database.

$ ncat localhost 9003
1.2.3.4
OK_1074$ ncat localhost 9003
jfsdkl
OK_1074$ ncat localhost 9003
217.115.10.133
EXIT

Karsten's comments on onionoo-querying-ver2.zip. https://lists.torproject.org/pipermail/tor-dev/2013-May/004825.html

I've decided that it's not worth running a whole separate onionoo cache for this. We do a best effort of avoiding running within TBB; the rest of this looks like a lot of maintenance cost for small gain.

Trac:
Resolution: N/A to wontfix
Status: needs_revision to closed

closed

Facilitator should not give client registrations to Tor exits

Child items ...

Activity