Re: [Tails-dev] Announcing control-port-filter-python - a fo…

Borrar esta mensaxe

Responder a esta mensaxe
Autor: intrigeri
Data:  
Para: The Tails public development discussion list
Asunto: Re: [Tails-dev] Announcing control-port-filter-python - a fork of tor-controlport-filter by Tails
Hi,

Patrick Schleizer wrote (09 Mar 2015 01:47:23 GMT) :
> I would like to inform you about the existence of
> control-port-filter-python, a fork of tor-controlport-filter by Tails.


Thanks!

> Improvements:


> * Supports parallel connections.


Looks useful, e.g. once we want Tor Monitor to stop connecting to the
Tor control port directly :)

> * Configurable by dropping .d-style configuration snippets into
> /etc/cpfpy.d. I.e. whitelist can be extended by dropping snippets.


Looks useful too, for easier code sharing between distros!

> * Supports logging.


It feels strange to me to add direct log files handling (as opposed to
using the syslog interface or the native systemd journal's one) to
a system-wide daemon in 2015: that's one more log file that won't be
aggregated in the systemd journal, so one has to look in one more
place when trying to debug something. Anyway, that's probably a matter
of taste :)

The good news is that the logging module supports syslog, so it seems
easy enough to adjust according to one's taste. Perhaps that could be
supported with a command-line option? (I guess the answer will be
"patches welcome" -- fair enough :)

> * Support to answer 'getinfo net/listeners/socks' with the lie
> '250-net/listeners/socks="127.0.0.1:9150"'.


The idea is to reply to Torbutton whatever it wants to hear when it
builds about:tor and its green onion icon, right ?

> * Honors signals sigterm, sigint.


Do you mean that Tails' current version ignores such signals?

> * Complete sysvinit script.


The great news is that the boilerplate that makes that script 159
lines long will soon be unnecessary, and be replaced by something
a little bit nicer (to my eye at least), such as:

https://git-tails.immerda.ch/tails/tree/config/chroot_local-includes/lib/systemd/system/tor-controlport-filter.service?h=feature/jessie

:)

> * Lintian clean /debian packaging folder.


Great.

> We would like to hear your feedback, comments, etc.


I've got a few design/style comments (keep in mind that I'm no Python
expert at all, so don't take it as a proper code review):

* The amount of global variables usage is troubling.
* Some exception catching feels a bit too wide.
* The list of "if line.startswith" that follows "for line in c" wants
to be replaced by a dispatch table or something more readable.

Cheers,
--
intrigeri