Opened 5 years ago

Closed 4 years ago

#7549 closed enhancement (wontfix)

Facilitator should not give client registrations to Tor exits

Reported by: dcf Owned by: jct
Priority: Medium Milestone:
Component: Archived/Flashproxy Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Like in #6293, in order to avoid a Tor-in-Tor situation, the facilitator should not give client registrations to any proxy that is requesting from a Tor exit. This should work similarly to how BridgeDB treats exits specially.

This ideally will use a locally cached database of exits. (Not an on-demand DNS lookup.) It should continue to work (perhaps with some classification errors) even if the database can't be refreshed for some time.

Roger offered this command, which is used to update the exit database for BridgeDB:

cat $HOME/auto-naming/moria1/cached-des* | python $HOME/git/contrib/exitlist <ip>:<port> > exitlist

The facilitator should not return a useful client registration in any case. An additional question is whether it should send a signal requesting that the proxy disable itself. (Knowing that a malicious proxy may ignore it.)

Child Tickets

Attachments (6)

detect_proxy_within_tor_in_tor_situation.patch (7.4 KB) - added by jct 5 years ago.
A patch with the candidate code in order to detect that the public address of the Proxy belongs to a TOR Exit node
test.py (4.2 KB) - added by jct 5 years ago.
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.
fac-onionoo.zip (11.7 KB) - added by jct 5 years ago.
Threaded TCP server to answer about IP's belonging to Tor Exit nodes.
fac-onionoo2.zip (14.4 KB) - added by jct 5 years ago.
Second generation of a Threaded TCP server to answer about IP's belonging to Tor Exit nodes.
onionoo-querying.zip (6.1 KB) - added by jct 5 years ago.
Protototype for the standalone onionoo-querying daemon
onionoo-querying-ver2.zip (6.1 KB) - added by jct 5 years ago.
Protototype for the standalone onionoo-querying daemon

Download all attachments as: .zip

Change History (22)

Changed 5 years ago by jct

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

comment:1 Changed 5 years ago by 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)

comment:2 in reply to:  1 Changed 5 years ago by dcf

Status: newneeds_revision

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.

comment:3 Changed 5 years ago by jct

Working on that!

Changed 5 years ago by jct

Attachment: test.py added

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.

comment:4 Changed 5 years ago by 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.

comment:5 Changed 5 years ago by dcf

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.

comment:6 Changed 5 years ago by cypherpunks

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

Changed 5 years ago by jct

Attachment: fac-onionoo.zip added

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

comment:8 Changed 5 years ago by 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:
    • 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.).

Changed 5 years ago by jct

Attachment: fac-onionoo2.zip added

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

comment:9 Changed 5 years ago by 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.

comment:10 in reply to:  9 ; Changed 5 years ago by 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.

comment:11 in reply to:  10 Changed 5 years ago by 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.

Working on that!

comment:12 in reply to:  10 ; Changed 5 years ago by 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.

Changed 5 years ago by jct

Attachment: onionoo-querying.zip added

Protototype for the standalone onionoo-querying daemon

comment:13 in reply to:  12 ; Changed 5 years ago by 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:

1) 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)

2) 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".

Changed 5 years ago by jct

Attachment: onionoo-querying-ver2.zip added

Protototype for the standalone onionoo-querying daemon

comment:14 in reply to:  13 ; Changed 4 years ago by dcf

Replying to jct:

1) 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)

2) 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

comment:15 in reply to:  14 Changed 4 years ago by dcf

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

comment:16 Changed 4 years ago by dcf

Resolution: wontfix
Status: needs_revisionclosed

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.

Note: See TracTickets for help on using tickets.