[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #7549 [Flashproxy]: Facilitator should not give client registrations to Tor exits
#7549: Facilitator should not give client registrations to Tor exits
-------------------------+--------------------------------------------------
Reporter: dcf | Owner: jct
Type: enhancement | Status: needs_revision
Priority: normal | Milestone:
Component: Flashproxy | Version:
Keywords: | Parent:
Points: | Actualpoints:
-------------------------+--------------------------------------------------
Comment(by dcf):
Replying to [comment:9 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.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7549#comment:10>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs